[llvm-profdata] Check for all duplicate entries in MemOpSize table

Previously, we only checked for duplicate zero entries when merging a
MemOPSize table (see D92074), but a user recently provided a reproducer
demonstrating that other entries can also be duplicated. As demonstrated
by the test in this patch, PGOMemOPSizeOpt can potentially generate
invalid IR for non-zero, non-consecutive duplicate entries. This seems
to be a rare case, since the duplicate entry is often below the
threshold, but possible. This patch extends the existing warning to
check for any duplicate values in the table, both in the optimization
and in llvm-profdata.

Differential Revision: https://reviews.llvm.org/D136211

Author: ormris

llvm/lib/ProfileData/InstrProfWriter.cpp

@@ -530,13 +530,10 @@ Error InstrProfWriter::validateRecord(const InstrProfRecord &Func) {
     for (uint32_t S = 0; S < NS; S++) {
       uint32_t ND = Func.getNumValueDataForSite(VK, S);
       std::unique_ptr<InstrProfValueData[]> VD = Func.getValueForSite(VK, S);
-      bool WasZero = false;
+      DenseSet<uint64_t> SeenValues;
       for (uint32_t I = 0; I < ND; I++)
-        if ((VK != IPVK_IndirectCallTarget) && (VD[I].Value == 0)) {
-          if (WasZero)
-            return make_error<InstrProfError>(instrprof_error::invalid_prof);
-          WasZero = true;
-        }
+        if ((VK != IPVK_IndirectCallTarget) && !SeenValues.insert(VD[I].Value).second)
+          return make_error<InstrProfError>(instrprof_error::invalid_prof);
     }
   }
 

llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp

@@ -291,9 +291,9 @@ bool MemOPSizeOpt::perform(MemOp MO) {
   uint64_t SavedRemainCount = SavedTotalCount;
   SmallVector<uint64_t, 16> SizeIds;
   SmallVector<uint64_t, 16> CaseCounts;
+  SmallDenseSet<uint64_t, 16> SeenSizeId;
   uint64_t MaxCount = 0;
   unsigned Version = 0;
-  int64_t LastV = -1;
   // Default case is in the front -- save the slot here.
   CaseCounts.push_back(0);
   SmallVector<InstrProfValueData, 24> RemainingVDs;
@@ -316,15 +316,12 @@ bool MemOPSizeOpt::perform(MemOp MO) {
       break;
     }
 
-    if (V == LastV) {
-      LLVM_DEBUG(dbgs() << "Invalid Profile Data in Function " << Func.getName()
-                        << ": Two consecutive, identical values in MemOp value"
-                           "counts.\n");
+    if (!SeenSizeId.insert(V).second) {
+      errs() << "Invalid Profile Data in Function " << Func.getName()
+             << ": Two identical values in MemOp value counts.\n";
       return false;
     }
 
-    LastV = V;
-
     SizeIds.push_back(V);
     CaseCounts.push_back(C);
     if (C > MaxCount)

llvm/test/Transforms/PGOProfile/consecutive-zeros.ll

@@ -1,6 +1,5 @@
-; REQUIRES: asserts
 ; RUN: llvm-profdata merge %S/Inputs/consecutive-zeros.proftext -o %t.profdata
-; RUN: opt < %s -debug -passes=pgo-instr-use,pgo-memop-opt -pgo-memop-count-threshold=0 -pgo-memop-percent-threshold=0 -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s
+; RUN: opt < %s -passes=pgo-instr-use,pgo-memop-opt -pgo-memop-count-threshold=0 -pgo-memop-percent-threshold=0 -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"