Address comments.

Author: Snehasish Kumar

llvm/lib/Transforms/Instrumentation/MemProfiler.cpp

@@ -978,8 +978,9 @@ static void addVPMetadata(Module &M, Instruction &I,
 
   if (I.getMetadata(LLVMContext::MD_prof)) {
     uint64_t Unused;
-    auto ExistingVD =
-        getValueProfDataFromInst(I, IPVK_IndirectCallTarget, ~0U, Unused);
+    // ~0U means get all available value profile data without any count limit
+    auto ExistingVD = getValueProfDataFromInst(I, IPVK_IndirectCallTarget,
+                                               /*MaxNumValueData=*/~0U, Unused);
     // We don't know how to merge value profile data yet.
     if (!ExistingVD.empty()) {
       return;
@@ -994,6 +995,8 @@ static void addVPMetadata(Module &M, Instruction &I,
     VD.Value = CalleeGUID;
     // For MemProf, we don't have actual call counts, so we assign
     // a weight of 1 to each potential target.
+    // TODO: Consider making this weight configurable or increasing it to
+    // improve effectiveness for ICP.
     VD.Count = 1;
     VDs.push_back(VD);
     TotalCount += VD.Count;
@@ -1083,6 +1086,9 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
     ArrayRef<GlobalValue::GUID> CalleeGuids;
 
     // Only compare Frame contents.
+    // Use pointer-based equality instead of ArrayRef's operator== which does
+    // element-wise comparison. We want to check if it's the same slice of the
+    // underlying array, not just equivalent content.
     bool operator==(const CallSiteEntry &Other) const {
       return Frames.data() == Other.Frames.data() &&
              Frames.size() == Other.Frames.size();
@@ -1271,9 +1277,8 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
           addCallsiteMetadata(I, InlinedCallStack, Ctx);
 
           // Try to attach indirect call metadata if possible.
-          if (!CalledFunction) {
+          if (!CalledFunction)
             addVPMetadata(M, I, CallSiteEntry.CalleeGuids);
-          }
 
           // Only need to find one with a matching call stack and add a single
           // callsite metadata.

llvm/test/Transforms/PGOProfile/memprof_annotate_indirect_call.test

@@ -1,11 +1,11 @@
 ; RUN: split-file %s %t
 
-; COM: Basic functionality with flag toggle
+;; Basic functionality with flag toggle
 ; RUN: llvm-profdata merge --memprof-version=4 %t/basic.yaml -o %t/basic.memprofdata
 ; RUN: opt < %t/basic.ll -passes='memprof-use<profile-filename=%t/basic.memprofdata>' -memprof-attach-calleeguids=false -S 2>&1 | FileCheck %s --check-prefix=CHECK-DISABLE
 ; RUN: opt < %t/basic.ll -passes='memprof-use<profile-filename=%t/basic.memprofdata>' -memprof-attach-calleeguids=true -S 2>&1 | FileCheck %s --check-prefix=CHECK-ENABLE
 
-; COM: FDO conflict handling
+;; FDO conflict handling
 ; RUN: llvm-profdata merge --memprof-version=4 %t/fdo_conflict.yaml -o %t/fdo_conflict.memprofdata
 ; RUN: opt < %t/fdo_conflict.ll -passes='memprof-use<profile-filename=%t/fdo_conflict.memprofdata>' -memprof-attach-calleeguids=true -S 2>&1 | FileCheck %s --check-prefix=CHECK-FDO
 
@@ -63,7 +63,6 @@ entry:
   %0 = load ptr, ptr %fp, align 8
   ; This call already has FDO value profile metadata - should NOT be modified
   ; CHECK-FDO: call void %0(), {{.*}} !prof !6
-  ; CHECK-FDO-NOT: call void %0(), {{.*}} !prof !9
   call void %0(), !dbg !15, !prof !16
 
   %1 = load ptr, ptr %fp, align 8