{PGO] Profile annotation for MemOPSize value profile

[xur]

Summary

This patch annotates the valuesites profile to memory intrinsic.
This is suggested in code review of D28966.

This patch has the dependence on https://reviews.llvm.org/D28965.

Note that I change the interface for annotateValueSite() InstrProf.h to write out all the available records the MaxMDCount is passed as 0.

[davidxl]

lib/ProfileData/InstrProf.cpp

791

more readable with

bool AllValues = (MDCount == 0);

..

if (!AllValues && --MDCount == 0)

break;

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

1202

the logic should be other way around:

if Kind == mem_op, do special thing.

On the other hand, why do we need to annotate all values? Shouldn't the transformation only care about top-N (say 3) values which pass certain threshold? All other values should be lumped together into the default value case.

[xur]

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

1202

I was thinking indirect-call-promtion was special as other value profiles won't truncate the profile.

I could do the top-N values. But
(1) the range options was defined in another file.
(2) this slightly change the profile semantic: the default is for (-inf, PreciseRangeStart) and (PreciseRangeLast, LargeRangeValue). Inserting PreciseValue changes this.
(3) right now, the optimization generates a switch stmt. we might need more values for better switch optimization.

[xur]

Integrate David's comments. Using a separated option to control the number of annotation for memop_size.

[xur]

Fix the testcase.

[davidxl]

test/Transforms/PGOProfile/Inputs/memop_size_annotation.proftext

19 ↗

(On Diff #92076)

can you make the data not sorted in the profile data ?

[xur]

randomize the order the VP profile entry in proftext, as suggested by David.

[davidxl]

lgtm

[vsk]

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

128

Please add a space after "memop".

269

I don't expect a function named "find" to annotate anything. Consider renaming this (maybe annotateMemIntrinsics)?

include/llvm/ProfileData/InstrProf.h

 enum InstrProfValueKind : uint32_t {
 #define VALUE_PROF_KIND(Enumerator, Value) Enumerator = Value,
 #include "llvm/ProfileData/InstrProfData.inc"
 };
 
 /// Get the value profile data for value site \p SiteIdx from \p InstrProfR
 /// and annotate the instruction \p Inst with the value profile meta data.
 /// Annotate up to \p MaxMDCount (default 3) number of records per value site.
+/// 0 value of \p MaxMDCount indicates to annotate all the available records.
 void annotateValueSite(Module &M, Instruction &Inst,
                        const InstrProfRecord &InstrProfR,
                        InstrProfValueKind ValueKind, uint32_t SiteIndx,
                        uint32_t MaxMDCount = 3);
 
 /// Same as the above interface but using an ArrayRef, as well as \p Sum.
 void annotateValueSite(Module &M, Instruction &Inst,
                        ArrayRef<InstrProfValueData> VDs,

lib/ProfileData/InstrProf.cpp

void annotateValueSite(Module &M, Instruction &Inst,
   Vals.push_back(MDHelper.createConstant(
       ConstantInt::get(Type::getInt32Ty(Ctx), ValueKind)));
   // Total Count
   Vals.push_back(
       MDHelper.createConstant(ConstantInt::get(Type::getInt64Ty(Ctx), Sum)));
 
   // Value Profile Data
   uint32_t MDCount = MaxMDCount;
+  bool CheckMaxCount = MDCount;
   for (auto &VD : VDs) {
     Vals.push_back(MDHelper.createConstant(
         ConstantInt::get(Type::getInt64Ty(Ctx), VD.Value)));
     Vals.push_back(MDHelper.createConstant(
         ConstantInt::get(Type::getInt64Ty(Ctx), VD.Count)));
-    if (--MDCount == 0)
+    if (CheckMaxCount && --MDCount == 0)
       break;
   }
   Inst.setMetadata(LLVMContext::MD_prof, MDNode::get(Ctx, Vals));
 }
 
 bool getValueProfDataFromInst(const Instruction &Inst,
                               InstrProfValueKind ValueKind,
                               uint32_t MaxNumValueData,

lib/Transforms/Instrumentation/PGOInstrumentation.cpp

static cl::opt<unsigned> MaxNumAnnotations(
     "icp-max-annotations", cl::init(3), cl::Hidden, cl::ZeroOrMore,
     cl::desc("Max number of annotations for a single indirect "
              "call callsite"));
 
 // Command line option to control appending FunctionHash to the name of a COMDAT
 // function. This is to avoid the hash mismatch caused by the preinliner.
 static cl::opt<bool> DoComdatRenaming(
     "do-comdat-renaming", cl::init(false), cl::Hidden,
     cl::desc("Append function hash to the name of COMDAT function to avoid "
              "function hash mismatch due to the preinliner"));
 
 // Command line option to enable/disable the warning about missing profile
 // information.
 static cl::opt<bool>
     PGOWarnMissing("pgo-warn-missing-function", cl::init(false), cl::Hidden,
                    cl::desc("Use this option to turn on/off "
                             "warnings about missing profile data for "
struct MemIntrinsicVisitor : public InstVisitor<MemIntrinsicVisitor> {
   Function &F;
   unsigned NMemIs = 0;          // Number of memIntrinsics instrumented.
   VisitMode Mode = VM_counting; // Visiting mode.
   unsigned CurCtrId = 0;        // Current counter index.
   unsigned TotalNumCtrs = 0;    // Total number of counters
   GlobalVariable *FuncNameVar = nullptr;
   uint64_t FuncHash = 0;
   PGOUseFunc *UseFunc = nullptr;
+  std::vector<Instruction *> Candidates;
 
   MemIntrinsicVisitor(Function &Func) : F(Func) {}
 
   void countMemIntrinsics(Function &Func) {
     NMemIs = 0;
     Mode = VM_counting;
     visit(Func);
   }
+
   void instrumentMemIntrinsics(Function &Func, unsigned TotalNC,
                                GlobalVariable *FNV, uint64_t FHash) {
     Mode = VM_instrument;
     TotalNumCtrs = TotalNC;
     FuncHash = FHash;
     FuncNameVar = FNV;
     visit(Func);
   }
 
+  std::vector<Instruction *> findMemIntrinsics(Function &Func) {
+    Candidates.clear();
+    Mode = VM_annotate;
+    visit(Func);
+    return Candidates;
+  }
+
   // Visit the IR stream and annotate all mem intrinsic call instructions.
   void instrumentOneMemIntrinsic(MemIntrinsic &MI);
   // Visit \p MI instruction and perform tasks according to visit mode.
   void visitMemIntrinsic(MemIntrinsic &SI);
   unsigned getNumOfMemIntrinsics() const { return NMemIs; }
 };
 
 class PGOInstrumentationGenLegacyPass : public ModulePass {
FuncPGOInstrumentation(
         SIVisitor(Func), MIVisitor(Func), FunctionHash(0), MST(F, BPI, BFI) {
 
     // This should be done before CFG hash computation.
     SIVisitor.countSelects(Func);
     MIVisitor.countMemIntrinsics(Func);
     NumOfPGOSelectInsts += SIVisitor.getNumOfSelectInsts();
     NumOfPGOMemIntrinsics += MIVisitor.getNumOfMemIntrinsics();
     ValueSites[IPVK_IndirectCallTarget] = findIndirectCallSites(Func);
+    ValueSites[IPVK_MemOPSize] = MIVisitor.findMemIntrinsics(Func);
 
     FuncName = getPGOFuncName(F);
     computeCFGHash();
     if (ComdatMembers.size())
       renameComdatFunction();
     DEBUG(dumpInfo("after CFGMST"));
 
     NumOfPGOBB += MST.BBInfos.size();
void MemIntrinsicVisitor::visitMemIntrinsic(MemIntrinsic &MI) {
   NMemIs++;
   switch (Mode) {
   case VM_counting:
     return;
   case VM_instrument:
     instrumentOneMemIntrinsic(MI);
     return;
   case VM_annotate:
-    break;
+    Candidates.push_back(&MI);
+    return;
   }
   llvm_unreachable("Unknown visiting mode");
 }
 
 // Traverse all valuesites and annotate the instructions for all value kind.
 void PGOUseFunc::annotateValueSites() {
   if (DisableValueProfiling)
     return;
void PGOUseFunc::annotateValueSites(uint32_t Kind) {
   }
 
   for (auto &I : ValueSites) {
     DEBUG(dbgs() << "Read one value site profile (kind = " << Kind
                  << "): Index = " << ValueSiteIndex << " out of "
                  << NumValueSites << "\n");
     annotateValueSite(*M, *I, ProfileRecord,
                       static_cast<InstrProfValueKind>(Kind), ValueSiteIndex,
-                      MaxNumAnnotations);
+                      Kind == IPVK_IndirectCallTarget ? MaxNumAnnotations : 0);
     ValueSiteIndex++;
   }
 }
 } // end anonymous namespace
 
 // Create a COMDAT variable INSTR_PROF_RAW_VERSION_VAR to make the runtime
 // aware this is an ir_level profile so it can set the version flag.
 static void createIRLevelProfileFlagVariable(Module &M) {

test/Transforms/PGOProfile/Inputs/memop_size.proftext

+# IR level Instrumentation Flag
+:ir
+foo
+# Func Hash:
[phone]
+# Num Counters:
+3
+# Counter Values:
+556
+20
+1
+# Num Value Kinds:
+1
+# ValueKind = IPVK_MemOPSize:
+1
+# NumValueSites:
+1
+9
+1:99
+2:88
+3:77
+9:72
+4:66
+5:55
+6:44
+7:33
+8:22

test/Transforms/PGOProfile/memop_size.ll

+; RUN: llvm-profdata merge %S/Inputs/memop_size.proftext -o %t.profdata
+; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s --check-prefix=MEMOP_ANNOTATION
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s --check-prefix=MEMOP_ANNOTATION
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @foo(i8* %dst, i8* %src, i32* %a, i32 %n) {
+entry:
+  br label %for.cond
+
+for.cond:
+  %i.0 = phi i32 [ 0, %entry ], [ %inc5, %for.inc4 ]
+  %cmp = icmp slt i32 %i.0, %n
+  br i1 %cmp, label %for.body, label %for.end6
+
+for.body:
+  br label %for.cond1
+
+for.cond1:
+  %j.0 = phi i32 [ 0, %for.body ], [ %inc, %for.inc ]
+  %idx.ext = sext i32 %i.0 to i64
+  %add.ptr = getelementptr inbounds i32, i32* %a, i64 %idx.ext
+  %0 = load i32, i32* %add.ptr, align 4
+  %cmp2 = icmp slt i32 %j.0, %0
+  br i1 %cmp2, label %for.body3, label %for.end
+
+for.body3:
+  %add = add nsw i32 %i.0, 1
+  %conv = sext i32 %add to i64
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %conv, i32 1, i1 false)
+; MEMOP_ANNOTATION: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %dst, i8* %src, i64 %conv, i32 1, i1 false)
+; MEMOP_ANNOTATION-SAME: !prof ![[MEMOP_VALUESITE:[0-9]+]]
+; MEMOP_ANNOTATION: ![[MEMOP_VALUESITE]] = !{!"VP", i32 1, i64 556, i64 1, i64 99, i64 2, i64 88, i64 3, i64 77, i64 9, i64 72, i64 4, i64 66, i64 5, i64 55, i64 6, i64 44, i64 7, i64 33, i64 8, i64 22}
+  br label %for.inc
+
+for.inc:
+  %inc = add nsw i32 %j.0, 1
+  br label %for.cond1
+
+for.end:
+  br label %for.inc4
+
+for.inc4:
+  %inc5 = add nsw i32 %i.0, 1
+  br label %for.cond
+
+for.end6:
+  ret void
+}
+
+declare void @llvm.lifetime.start(i64, i8* nocapture)
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1)
+
+declare void @llvm.lifetime.end(i64, i8* nocapture)