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

[Matthew Voss via llvm-commits]

Author: Matthew Voss
Date: 2022-11-04T17:08:54-07:00
New Revision: a4b543a5a541eeff5a7ba92ada4b0d809a2c8482

URL: https://github.com/llvm/llvm-project/commit/a4b543a5a541eeff5a7ba92ada4b0d809a2c8482
DIFF: https://github.com/llvm/llvm-project/commit/a4b543a5a541eeff5a7ba92ada4b0d809a2c8482.diff

LOG: [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

Added: 
    

Modified: 
    llvm/lib/ProfileData/InstrProfWriter.cpp
    llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
    llvm/test/Transforms/PGOProfile/consecutive-zeros.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index cd4e8900c9637..de632695ca499 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/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);
     }
   }
 

diff  --git a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp b/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
index 07c03ee2049ac..267446bddcf5f 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
+++ b/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)

diff  --git a/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll b/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll
index a388cbc37d5a6..6634838d21e1a 100644
--- a/llvm/test/Transforms/PGOProfile/consecutive-zeros.ll
+++ b/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"