https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/79381
>From d4caa0997799b712edb11d90c5be79d0aab3c312 Mon Sep 17 00:00:00 2001 From: mingmingl <mingmi...@google.com> Date: Thu, 25 Jan 2024 13:59:03 -0800 Subject: [PATCH 1/4] Introduce an opton to control the number of vtables to annotate and use it when generating function summaries. Created using spr 1.3.4 --- .../IndirectCallPromotionAnalysis.cpp | 3 +++ llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 12 ++++----- .../thinlto-func-summary-vtableref-pgo.ll | 25 ++++++++++++------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp b/llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp index ebfa1c8fc08e1c..18cb6a220e3bd0 100644 --- a/llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp +++ b/llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp @@ -45,6 +45,9 @@ static cl::opt<unsigned> cl::desc("Max number of promotions for a single indirect " "call callsite")); +cl::opt<unsigned> MaxNumVTableAnnotations("icp-max-num-vtables", cl::init(6), cl::Hidden, + cl::desc("Max number of vtables annotated for a vtable load instruction.")); + ICallPromotionAnalysis::ICallPromotionAnalysis() { ValueDataArray = std::make_unique<InstrProfValueData[]>(MaxNumPromotions); } diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp index fc8c31de0f4501..0f0085025cc56b 100644 --- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -82,6 +82,8 @@ static cl::opt<std::string> ModuleSummaryDotFile( extern cl::opt<bool> ScalePartialSampleProfileWorkingSetSize; +extern cl::opt<unsigned> MaxNumVTableAnnotations; + // Walk through the operands of a given User via worklist iteration and populate // the set of GlobalValue references encountered. Invoked either on an // Instruction or a GlobalVariable (which walks its initializer). @@ -129,14 +131,10 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser, if (I) { uint32_t ActualNumValueData = 0; uint64_t TotalCount = 0; - // 24 is the maximum number of values preserved for one instrumented site, - // defined by INSTR_PROF_DEFAULT_NUM_VAL_PER_SITE in - // compiler-rt/lib/profile/InstrProfilingValue.c; passing 24 as - // `MaxNumValueData` controls the max number of elements in the returned - // array. The actual number of values is gated by the number of ops in !prof - // metadata. + // MaxNumVTableAnnotations is the maximum number of vtables annotated on + // the instruction. auto ValueDataArray = getValueProfDataFromInst( - *I, IPVK_VTableTarget, 24 /* MaxNumValueData */, ActualNumValueData, + *I, IPVK_VTableTarget, MaxNumVTableAnnotations /* MaxNumValueData */, ActualNumValueData, TotalCount); if (ValueDataArray.get()) { diff --git a/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll b/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll index 28e4b1d19aef72..ba3ce9a75ee832 100644 --- a/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll +++ b/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll @@ -1,4 +1,8 @@ -; RUN: opt -module-summary %s -o %t.o +; Promote at most one function and annotate at most one vtable. +; As a result, only one value (of each relevant kind) shows up in the function +; summary. + +; RUN: opt -module-summary -icp-max-num-vtables=1 -icp-max-prom=1 %s -o %t.o ; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s @@ -11,15 +15,17 @@ ; CHECK-NEXT: <FLAGS op0=0/> ; The `VALUE_GUID` below represents the "_ZTV4Base" referenced by the instruction ; that loads vtable pointers. -; CHECK-NEXT: <VALUE_GUID op0=18 op1=1960855528937986108/> +; CHECK-NEXT: <VALUE_GUID op0=21 op1=1960855528937986108/> ; The `VALUE_GUID` below represents the "_ZN4Base4funcEv" referenced by the ; indirect call instruction. -; CHECK-NEXT: <VALUE_GUID op0=17 op1=5459407273543877811/> +; CHECK-NEXT: <VALUE_GUID op0=20 op1=5459407273543877811/> +; NOTE vtables and functions from Derived class is dropped because +; `-icp-max-num-vtables` and `-icp-max-prom` are both set to one. ; <PERMODULE_PROFILE> has the format [valueid, flags, instcount, funcflags, ; numrefs, rorefcnt, worefcnt, ; m x valueid, ; n x (valueid, hotness+tailcall)] -; CHECK-NEXT: <PERMODULE_PROFILE abbrevid=4 op0=0 op1=0 op2=4 op3=256 op4=1 op5=1 op6=0 op7=18 op8=17 op9=3/> +; CHECK-NEXT: <PERMODULE_PROFILE abbrevid=4 op0=0 op1=0 op2=4 op3=256 op4=1 op5=1 op6=0 op7=21 op8=20 op9=3/> ; CHECK-NEXT: </GLOBALVAL_SUMMARY_BLOCK> target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" @@ -36,7 +42,6 @@ define i32 @_Z4testP4Base(ptr %0) !prof !15 { !llvm.module.flags = !{!1} - !1 = !{i32 1, !"ProfileSummary", !2} !2 = !{!3, !4, !5, !6, !7, !8, !9, !10} !3 = !{!"ProfileFormat", !"InstrProf"} @@ -53,10 +58,12 @@ define i32 @_Z4testP4Base(ptr %0) !prof !15 { !14 = !{i32 999999, i64 1, i32 2} !15 = !{!"function_entry_count", i32 150} -; 1960855528937986108 is the MD5 hash of _ZTV4Base -!16 = !{!"VP", i32 2, i64 1600, i64 1960855528937986108, i64 1600} -; 5459407273543877811 is the MD5 hash of _ZN4Base4funcEv -!17 = !{!"VP", i32 0, i64 1600, i64 5459407273543877811, i64 1600} +; 1960855528937986108 is the MD5 hash of _ZTV4Base, and +; 13870436605473471591 is the MD5 hash of _ZTV7Derived +!16 = !{!"VP", i32 2, i64 150, i64 1960855528937986108, i64 100, i64 13870436605473471591, i64 50} +; 5459407273543877811 is the MD5 hash of _ZN4Base4funcEv, and +; 6174874150489409711 is the MD5 hash of _ZN7Derived4funcEv +!17 = !{!"VP", i32 0, i64 150, i64 5459407273543877811, i64 100, i64 6174874150489409711, i64 50} ; ModuleSummaryIndex stores <guid, global-value summary> map in std::map; so ; global value summares are printed out in the order that gv's guid increases. >From 3e5b1d234a85cc19ccb98fd0a83ea11d93d78a4e Mon Sep 17 00:00:00 2001 From: mingmingl <mingmi...@google.com> Date: Thu, 25 Jan 2024 14:01:46 -0800 Subject: [PATCH 2/4] run git clang-format Created using spr 1.3.4 --- llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp | 5 +++-- llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp b/llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp index 18cb6a220e3bd0..ab53717eb889a0 100644 --- a/llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp +++ b/llvm/lib/Analysis/IndirectCallPromotionAnalysis.cpp @@ -45,8 +45,9 @@ static cl::opt<unsigned> cl::desc("Max number of promotions for a single indirect " "call callsite")); -cl::opt<unsigned> MaxNumVTableAnnotations("icp-max-num-vtables", cl::init(6), cl::Hidden, - cl::desc("Max number of vtables annotated for a vtable load instruction.")); +cl::opt<unsigned> MaxNumVTableAnnotations( + "icp-max-num-vtables", cl::init(6), cl::Hidden, + cl::desc("Max number of vtables annotated for a vtable load instruction.")); ICallPromotionAnalysis::ICallPromotionAnalysis() { ValueDataArray = std::make_unique<InstrProfValueData[]>(MaxNumPromotions); diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp index 0f0085025cc56b..24f0569db9d2ed 100644 --- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -134,8 +134,8 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser, // MaxNumVTableAnnotations is the maximum number of vtables annotated on // the instruction. auto ValueDataArray = getValueProfDataFromInst( - *I, IPVK_VTableTarget, MaxNumVTableAnnotations /* MaxNumValueData */, ActualNumValueData, - TotalCount); + *I, IPVK_VTableTarget, MaxNumVTableAnnotations /* MaxNumValueData */, + ActualNumValueData, TotalCount); if (ValueDataArray.get()) { for (uint32_t j = 0; j < ActualNumValueData; j++) { >From 10fa0177beb559c1faa60f7f4d5927bc9101fa82 Mon Sep 17 00:00:00 2001 From: mingmingl <mingmi...@google.com> Date: Thu, 25 Jan 2024 16:42:04 -0800 Subject: [PATCH 3/4] resolve review feedback Created using spr 1.3.4 --- llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 6 +++--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 2 +- llvm/lib/ProfileData/InstrProf.cpp | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp index 24f0569db9d2ed..02eeec853ed7a7 100644 --- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -134,13 +134,13 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser, // MaxNumVTableAnnotations is the maximum number of vtables annotated on // the instruction. auto ValueDataArray = getValueProfDataFromInst( - *I, IPVK_VTableTarget, MaxNumVTableAnnotations /* MaxNumValueData */, + *I, IPVK_VTableTarget, MaxNumVTableAnnotations, ActualNumValueData, TotalCount); if (ValueDataArray.get()) { for (uint32_t j = 0; j < ActualNumValueData; j++) { - RefEdges.insert(Index.getOrInsertValueInfo( - ValueDataArray[j].Value /* VTableGUID */)); + RefEdges.insert(Index.getOrInsertValueInfo(/* VTableGUID = */ + ValueDataArray[j].Value)); } } } diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index 432a99d35efeab..2a4edeb7ce2123 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -210,7 +210,7 @@ class ModuleBitcodeWriterBase : public BitcodeWriterBase { // For each referenced variables in the function summary, see if the // variable is represented by a GUID (as opposed to a symbol to - // declarations or definitions in the module). If so, sythesize a + // declarations or definitions in the module). If so, synthesize a // value id. for (auto &RefEdge : FS->refs()) if ((!RefEdge.haveGVs() || !RefEdge.getValue())) diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 925e0fb67e1ee2..e0943ec2ecd461 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -1339,6 +1339,7 @@ static bool getValueProfDataFromInst(const MDNode *const MD, return true; } + std::unique_ptr<InstrProfValueData[]> getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind, uint32_t MaxNumValueData, uint32_t &ActualNumValueData, @@ -1353,6 +1354,8 @@ getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind, return ValueDataArray; } +// FIXME: Migrate existing callers to the function above that returns an +// array. bool getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind, uint32_t MaxNumValueData, >From 5ed910ab2a754978b858bde1e3323ca399cad7ff Mon Sep 17 00:00:00 2001 From: mingmingl <mingmi...@google.com> Date: Fri, 26 Jan 2024 16:16:28 -0800 Subject: [PATCH 4/4] Rename static helper function with "Impl" suffix Created using spr 1.3.4 --- llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 8 ++++---- llvm/lib/ProfileData/InstrProf.cpp | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp index 02eeec853ed7a7..3ad0bab827a512 100644 --- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -133,14 +133,14 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser, uint64_t TotalCount = 0; // MaxNumVTableAnnotations is the maximum number of vtables annotated on // the instruction. - auto ValueDataArray = getValueProfDataFromInst( - *I, IPVK_VTableTarget, MaxNumVTableAnnotations, - ActualNumValueData, TotalCount); + auto ValueDataArray = + getValueProfDataFromInst(*I, IPVK_VTableTarget, MaxNumVTableAnnotations, + ActualNumValueData, TotalCount); if (ValueDataArray.get()) { for (uint32_t j = 0; j < ActualNumValueData; j++) { RefEdges.insert(Index.getOrInsertValueInfo(/* VTableGUID = */ - ValueDataArray[j].Value)); + ValueDataArray[j].Value)); } } } diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index e0943ec2ecd461..1d8b0a1aca95f2 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -1308,11 +1308,11 @@ MDNode *mayHaveValueProfileOfKind(const Instruction &Inst, return MD; } -static bool getValueProfDataFromInst(const MDNode *const MD, - const uint32_t MaxNumDataWant, - InstrProfValueData ValueData[], - uint32_t &ActualNumValueData, - uint64_t &TotalC, bool GetNoICPValue) { +static bool getValueProfDataFromInstImpl(const MDNode *const MD, + const uint32_t MaxNumDataWant, + InstrProfValueData ValueData[], + uint32_t &ActualNumValueData, + uint64_t &TotalC, bool GetNoICPValue) { const unsigned NOps = MD->getNumOperands(); // Get total count ConstantInt *TotalCInt = mdconst::dyn_extract<ConstantInt>(MD->getOperand(2)); @@ -1339,7 +1339,6 @@ static bool getValueProfDataFromInst(const MDNode *const MD, return true; } - std::unique_ptr<InstrProfValueData[]> getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind, uint32_t MaxNumValueData, uint32_t &ActualNumValueData, @@ -1348,8 +1347,8 @@ getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind, if (!MD) return nullptr; auto ValueDataArray = std::make_unique<InstrProfValueData[]>(MaxNumValueData); - if (!getValueProfDataFromInst(MD, MaxNumValueData, ValueDataArray.get(), - ActualNumValueData, TotalC, GetNoICPValue)) + if (!getValueProfDataFromInstImpl(MD, MaxNumValueData, ValueDataArray.get(), + ActualNumValueData, TotalC, GetNoICPValue)) return nullptr; return ValueDataArray; } @@ -1365,8 +1364,9 @@ bool getValueProfDataFromInst(const Instruction &Inst, MDNode *MD = mayHaveValueProfileOfKind(Inst, ValueKind); if (!MD) return false; - return getValueProfDataFromInst(MD, MaxNumValueData, ValueData, - ActualNumValueData, TotalC, GetNoICPValue); + return getValueProfDataFromInstImpl(MD, MaxNumValueData, ValueData, + ActualNumValueData, TotalC, + GetNoICPValue); } MDNode *getPGOFuncNameMetadata(const Function &F) { _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits