Undo unrelated change.

Author: Snehasish Kumar

llvm/lib/Transforms/Instrumentation/MemProfiler.cpp

@@ -95,7 +95,7 @@ static cl::opt<bool> ClInsertVersionCheck(
 // This flag may need to be replaced with -f[no-]memprof-reads.
 static cl::opt<bool> ClInstrumentReads("memprof-instrument-reads",
                                        cl::desc("instrument read instructions"),
-                                       cl::Hidden, cl::init(false));
+                                       cl::Hidden, cl::init(true));
 
 static cl::opt<bool>
     ClInstrumentWrites("memprof-instrument-writes",
@@ -177,8 +177,11 @@ static cl::opt<bool>
                         cl::desc("Salvage stale MemProf profile"),
                         cl::init(false), cl::Hidden);
 
-static cl::opt<bool> ClMemProfAttachCalleeGuids("memprof-attach-calleeguids",
-                                                cl::init(false));
+static cl::opt<bool> ClMemProfAttachCalleeGuids(
+    "memprof-attach-calleeguids",
+    cl::desc(
+        "Attach calleeguids as value profile metadata for indirect calls."),
+    cl::init(true), cl::Hidden);
 
 extern cl::opt<bool> MemProfReportHintedSizes;
 extern cl::opt<unsigned> MinClonedColdBytePercent;
@@ -974,8 +977,6 @@ static void addVPMetadata(Module &M, Instruction &I,
     return;
 
   if (I.getMetadata(LLVMContext::MD_prof)) {
-    // Use the existing helper function to check for indirect call
-    // target value profiling metadata
     uint64_t Unused;
     auto ExistingVD =
         getValueProfDataFromInst(I, IPVK_IndirectCallTarget, ~0U, Unused);
@@ -988,20 +989,17 @@ static void addVPMetadata(Module &M, Instruction &I,
   SmallVector<InstrProfValueData, 4> VDs;
   uint64_t TotalCount = 0;
 
-  for (GlobalValue::GUID CalleeGUID : CalleeGuids) {
-    // For MemProf, we don't have actual call counts, so we assign
-    // a weight of 1 to each potential target. This provides the
-    // information needed for indirect call promotion without
-    // specific count data.
+  for (const GlobalValue::GUID CalleeGUID : CalleeGuids) {
     InstrProfValueData VD;
     VD.Value = CalleeGUID;
-    VD.Count = 1; // Weight for ICP decision making
+    // For MemProf, we don't have actual call counts, so we assign
+    // a weight of 1 to each potential target.
+    VD.Count = 1;
     VDs.push_back(VD);
     TotalCount += VD.Count;
   }
 
   if (!VDs.empty()) {
-    // Attach value profile metadata for indirect call targets
     annotateValueSite(M, I, VDs, TotalCount, IPVK_IndirectCallTarget,
                       VDs.size());
   }
@@ -1075,12 +1073,16 @@ readMemprof(Module &M, Function &F, IndexedInstrProfReader *MemProfReader,
   // (allocation info and the callsites).
   std::map<uint64_t, std::set<const AllocationInfo *>> LocHashToAllocInfo;
 
+  // Helper struct for maintaining refs to callsite data. As an alternative we
+  // could store a pointer to the CallSiteInfo struct but we also need the frame
+  // index. Using ArrayRefs instead makes it a little easier to read.
   struct CallSiteEntry {
     // Subset of frames for the corresponding CallSiteInfo.
     ArrayRef<Frame> Frames;
     // Potential targets for indirect calls.
     ArrayRef<GlobalValue::GUID> CalleeGuids;
 
+    // Only compare Frame contents.
     bool operator==(const CallSiteEntry &Other) const {
       return Frames.data() == Other.Frames.data() &&
              Frames.size() == Other.Frames.size();

llvm/test/Transforms/PGOProfile/memprof_annotate_indirect_call_fdo_conflict.test

@@ -25,18 +25,17 @@ entry:
   %fp = alloca ptr, align 8
   %0 = load ptr, ptr %fp, align 8
   ; This call already has FDO value profile metadata - should NOT be modified
+  ; CHECK: call void %0(), {{.*}} !prof !6
+  ; CHECK-NOT: call void %0(), {{.*}} !prof ![[NEWPROF:[0-9]+]]
   call void %0(), !dbg !5, !prof !6
-; CHECK: call void %0(), {{.*}} !prof !6
-; CHECK-NOT: call void %0(), {{.*}} !prof ![[NEWPROF:[0-9]+]]
 
-  ; This call does NOT have existing metadata - should get MemProf annotation
   %1 = load ptr, ptr %fp, align 8
+  ; This call does NOT have existing metadata - should get MemProf annotation
+  ; CHECK: call void %1(), {{.*}} !prof ![[NEWPROF:[0-9]+]]
   call void %1(), !dbg !7
-; CHECK: call void %1(), {{.*}} !prof ![[NEWPROF:[0-9]+]]
   ret void
 }
 
-; Existing FDO value profile metadata (should be preserved)
 !6 = !{!"VP", i32 0, i64 100, i64 9191153033785521275, i64 80, i64 -1069303473483922844, i64 20}
 
 ; CHECK: !6 = !{!"VP", i32 0, i64 100, i64 9191153033785521275, i64 80, i64 -1069303473483922844, i64 20}

llvm/test/Transforms/PGOProfile/memprof_annotate_indirect_call_yaml.test

@@ -3,7 +3,6 @@
 
 ; RUN: split-file %s %t
 ; RUN: llvm-profdata merge --memprof-version=4 %t/memprof_annotate_indirect_call_yaml.yaml -o %t/memprof_annotate_indirect_call_yaml.memprofdata
-; RUN: opt < %t/memprof_annotate_indirect_call_yaml.ll -passes='memprof-use<profile-filename=%t/memprof_annotate_indirect_call_yaml.memprofdata>' -S 2>&1 | FileCheck %s
 ; RUN: opt < %t/memprof_annotate_indirect_call_yaml.ll -passes='memprof-use<profile-filename=%t/memprof_annotate_indirect_call_yaml.memprofdata>' -memprof-attach-calleeguids=false -S 2>&1 | FileCheck %s --check-prefix=CHECK-DISABLE
 ; RUN: opt < %t/memprof_annotate_indirect_call_yaml.ll -passes='memprof-use<profile-filename=%t/memprof_annotate_indirect_call_yaml.memprofdata>' -memprof-attach-calleeguids=true -S 2>&1 | FileCheck %s --check-prefix=CHECK-ENABLE
 
@@ -23,7 +22,6 @@ entry:
   %fp = alloca ptr, align 8
   %0 = load ptr, ptr %fp, align 8
   call void %0(), !dbg !5
-; CHECK-NOT: !prof
 ; CHECK-ENABLE: call void %0(), {{.*}} !prof ![[PROF:[0-9]+]]
 ; CHECK-DISABLE-NOT: !prof
   ret void