-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ThinLTO][TypeProf] Implement vtable def import #79381
Conversation
Created using spr 1.3.4 [skip ci]
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4 [skip ci]
Created using spr 1.3.4
@llvm/pr-subscribers-llvm-analysis Author: Mingming Liu (minglotus-6) ChangesAdd annotated vtable GUID as referenced variables in per function summary, and update bitcode writer to create value-ids for these referenced vtables.
Full diff: https://github.com/llvm/llvm-project/pull/79381.diff 5 Files Affected:
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index d936f38741e1e3a..248f62c7a810592 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -283,7 +283,7 @@ void annotateValueSite(Module &M, Instruction &Inst,
/// Extract the value profile data from \p Inst which is annotated with
/// value profile meta data. Return false if there is no value data annotated,
-/// otherwise return true.
+/// otherwise return true.
bool getValueProfDataFromInst(const Instruction &Inst,
InstrProfValueKind ValueKind,
uint32_t MaxNumValueData,
@@ -291,6 +291,16 @@ bool getValueProfDataFromInst(const Instruction &Inst,
uint32_t &ActualNumValueData, uint64_t &TotalC,
bool GetNoICPValue = false);
+/// Extract the value profile data from \p Inst and returns them if \p Inst is
+/// annotated with value profile data. Returns nullptr otherwise. It's similar
+/// to `getValueProfDataFromInst` above except that an array is allocated only
+/// after a preliminary checking that the value profiles of kind `ValueKind`
+/// exist.
+std::unique_ptr<InstrProfValueData[]>
+getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind,
+ uint32_t MaxNumValueData, uint32_t &ActualNumValueData,
+ uint64_t &TotalC, bool GetNoICPValue = false);
+
inline StringRef getPGOFuncNameMetadataName() { return "PGOFuncName"; }
/// Return the PGOFuncName meta data associated with a function.
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 1f15e94783240a7..fc8c31de0f4501f 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -124,6 +124,28 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
Worklist.push_back(Operand);
}
}
+
+ const Instruction *I = dyn_cast<Instruction>(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.
+ auto ValueDataArray = getValueProfDataFromInst(
+ *I, IPVK_VTableTarget, 24 /* MaxNumValueData */, ActualNumValueData,
+ TotalCount);
+
+ if (ValueDataArray.get()) {
+ for (uint32_t j = 0; j < ActualNumValueData; j++) {
+ RefEdges.insert(Index.getOrInsertValueInfo(
+ ValueDataArray[j].Value /* VTableGUID */));
+ }
+ }
+ }
return HasBlockAddress;
}
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index a5fc267b1883bfe..432a99d35efeab4 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -199,7 +199,7 @@ class ModuleBitcodeWriterBase : public BitcodeWriterBase {
for (const auto &GUIDSummaryLists : *Index)
// Examine all summaries for this GUID.
for (auto &Summary : GUIDSummaryLists.second.SummaryList)
- if (auto FS = dyn_cast<FunctionSummary>(Summary.get()))
+ if (auto FS = dyn_cast<FunctionSummary>(Summary.get())) {
// For each call in the function summary, see if the call
// is to a GUID (which means it is for an indirect call,
// otherwise we would have a Value for it). If so, synthesize
@@ -207,6 +207,15 @@ class ModuleBitcodeWriterBase : public BitcodeWriterBase {
for (auto &CallEdge : FS->calls())
if (!CallEdge.first.haveGVs() || !CallEdge.first.getValue())
assignValueId(CallEdge.first.getGUID());
+
+ // 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
+ // value id.
+ for (auto &RefEdge : FS->refs())
+ if ((!RefEdge.haveGVs() || !RefEdge.getValue()))
+ assignValueId(RefEdge.getGUID());
+ }
}
protected:
@@ -4071,7 +4080,7 @@ void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
NameVals.push_back(SpecialRefCnts.second); // worefcnt
for (auto &RI : FS->refs())
- NameVals.push_back(VE.getValueID(RI.getValue()));
+ NameVals.push_back(getValueId(RI));
const bool UseRelBFRecord =
WriteRelBFToSummary && !F.hasProfileData() &&
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 598f3fafb2bd90f..925e0fb67e1ee20 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1285,46 +1285,44 @@ void annotateValueSite(Module &M, Instruction &Inst,
Inst.setMetadata(LLVMContext::MD_prof, MDNode::get(Ctx, Vals));
}
-bool getValueProfDataFromInst(const Instruction &Inst,
- InstrProfValueKind ValueKind,
- uint32_t MaxNumValueData,
- InstrProfValueData ValueData[],
- uint32_t &ActualNumValueData, uint64_t &TotalC,
- bool GetNoICPValue) {
+MDNode *mayHaveValueProfileOfKind(const Instruction &Inst,
+ InstrProfValueKind ValueKind) {
MDNode *MD = Inst.getMetadata(LLVMContext::MD_prof);
if (!MD)
- return false;
+ return nullptr;
- unsigned NOps = MD->getNumOperands();
+ if (MD->getNumOperands() < 5)
+ return nullptr;
- if (NOps < 5)
- return false;
-
- // Operand 0 is a string tag "VP":
MDString *Tag = cast<MDString>(MD->getOperand(0));
- if (!Tag)
- return false;
-
- if (!Tag->getString().equals("VP"))
- return false;
+ if (!Tag || !Tag->getString().equals("VP"))
+ return nullptr;
// Now check kind:
ConstantInt *KindInt = mdconst::dyn_extract<ConstantInt>(MD->getOperand(1));
if (!KindInt)
- return false;
+ return nullptr;
if (KindInt->getZExtValue() != ValueKind)
- return false;
+ return nullptr;
+
+ return MD;
+}
+static bool getValueProfDataFromInst(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));
if (!TotalCInt)
return false;
TotalC = TotalCInt->getZExtValue();
-
ActualNumValueData = 0;
for (unsigned I = 3; I < NOps; I += 2) {
- if (ActualNumValueData >= MaxNumValueData)
+ if (ActualNumValueData >= MaxNumDataWant)
break;
ConstantInt *Value = mdconst::dyn_extract<ConstantInt>(MD->getOperand(I));
ConstantInt *Count =
@@ -1341,6 +1339,33 @@ bool getValueProfDataFromInst(const Instruction &Inst,
return true;
}
+std::unique_ptr<InstrProfValueData[]>
+getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind,
+ uint32_t MaxNumValueData, uint32_t &ActualNumValueData,
+ uint64_t &TotalC, bool GetNoICPValue) {
+ MDNode *MD = mayHaveValueProfileOfKind(Inst, ValueKind);
+ if (!MD)
+ return nullptr;
+ auto ValueDataArray = std::make_unique<InstrProfValueData[]>(MaxNumValueData);
+ if (!getValueProfDataFromInst(MD, MaxNumValueData, ValueDataArray.get(),
+ ActualNumValueData, TotalC, GetNoICPValue))
+ return nullptr;
+ return ValueDataArray;
+}
+
+bool getValueProfDataFromInst(const Instruction &Inst,
+ InstrProfValueKind ValueKind,
+ uint32_t MaxNumValueData,
+ InstrProfValueData ValueData[],
+ uint32_t &ActualNumValueData, uint64_t &TotalC,
+ bool GetNoICPValue) {
+ MDNode *MD = mayHaveValueProfileOfKind(Inst, ValueKind);
+ if (!MD)
+ return false;
+ return getValueProfDataFromInst(MD, MaxNumValueData, ValueData,
+ ActualNumValueData, TotalC, GetNoICPValue);
+}
+
MDNode *getPGOFuncNameMetadata(const Function &F) {
return F.getMetadata(getPGOFuncNameMetadataName());
}
diff --git a/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll b/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll
index 78b175caca85f01..28e4b1d19aef72c 100644
--- a/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll
+++ b/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll
@@ -3,18 +3,23 @@
; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
; RUN: llvm-dis -o - %t.o | FileCheck %s --check-prefix=DIS
-
+; Round trip it through llvm-as
+; RUN: llvm-dis -o - %t.o | llvm-as -o - | llvm-dis -o - | FileCheck %s --check-prefix=DIS
; CHECK: <GLOBALVAL_SUMMARY_BLOCK
; CHECK-NEXT: <VERSION op0=9/>
; 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/>
; The `VALUE_GUID` below represents the "_ZN4Base4funcEv" referenced by the
; indirect call instruction.
; CHECK-NEXT: <VALUE_GUID op0=17 op1=5459407273543877811/>
; <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=0 op5=0 op6=0 op7=17 op8=3/>
+; 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: </GLOBALVAL_SUMMARY_BLOCK>
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
@@ -56,6 +61,7 @@ define i32 @_Z4testP4Base(ptr %0) !prof !15 {
; 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.
; DIS: ^0 = module: (path: "{{.*}}", hash: (0, 0, 0, 0, 0))
-; DIS: ^1 = gv: (guid: 5459407273543877811)
-; DIS: ^2 = gv: (name: "_Z4testP4Base", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 4, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 0, hasUnknownCall: 1, mustBeUnreachable: 0), calls: ((callee: ^1, hotness: hot))))) ; guid = 15857150948103218965
-; DIS: ^3 = blockcount: 0
+; DIS: ^1 = gv: (guid: 1960855528937986108)
+; DIS: ^2 = gv: (guid: 5459407273543877811)
+; DIS: ^3 = gv: (name: "_Z4testP4Base", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 4, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 0, hasUnknownCall: 1, mustBeUnreachable: 0), calls: ((callee: ^2, hotness: hot)), refs: (readonly ^1)))) ; guid = 15857150948103218965
+; DIS: ^4 = blockcount: 0
|
@llvm/pr-subscribers-lto Author: Mingming Liu (minglotus-6) ChangesAdd annotated vtable GUID as referenced variables in per function summary, and update bitcode writer to create value-ids for these referenced vtables.
Full diff: https://github.com/llvm/llvm-project/pull/79381.diff 5 Files Affected:
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index d936f38741e1e3a..248f62c7a810592 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -283,7 +283,7 @@ void annotateValueSite(Module &M, Instruction &Inst,
/// Extract the value profile data from \p Inst which is annotated with
/// value profile meta data. Return false if there is no value data annotated,
-/// otherwise return true.
+/// otherwise return true.
bool getValueProfDataFromInst(const Instruction &Inst,
InstrProfValueKind ValueKind,
uint32_t MaxNumValueData,
@@ -291,6 +291,16 @@ bool getValueProfDataFromInst(const Instruction &Inst,
uint32_t &ActualNumValueData, uint64_t &TotalC,
bool GetNoICPValue = false);
+/// Extract the value profile data from \p Inst and returns them if \p Inst is
+/// annotated with value profile data. Returns nullptr otherwise. It's similar
+/// to `getValueProfDataFromInst` above except that an array is allocated only
+/// after a preliminary checking that the value profiles of kind `ValueKind`
+/// exist.
+std::unique_ptr<InstrProfValueData[]>
+getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind,
+ uint32_t MaxNumValueData, uint32_t &ActualNumValueData,
+ uint64_t &TotalC, bool GetNoICPValue = false);
+
inline StringRef getPGOFuncNameMetadataName() { return "PGOFuncName"; }
/// Return the PGOFuncName meta data associated with a function.
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 1f15e94783240a7..fc8c31de0f4501f 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -124,6 +124,28 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
Worklist.push_back(Operand);
}
}
+
+ const Instruction *I = dyn_cast<Instruction>(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.
+ auto ValueDataArray = getValueProfDataFromInst(
+ *I, IPVK_VTableTarget, 24 /* MaxNumValueData */, ActualNumValueData,
+ TotalCount);
+
+ if (ValueDataArray.get()) {
+ for (uint32_t j = 0; j < ActualNumValueData; j++) {
+ RefEdges.insert(Index.getOrInsertValueInfo(
+ ValueDataArray[j].Value /* VTableGUID */));
+ }
+ }
+ }
return HasBlockAddress;
}
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index a5fc267b1883bfe..432a99d35efeab4 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -199,7 +199,7 @@ class ModuleBitcodeWriterBase : public BitcodeWriterBase {
for (const auto &GUIDSummaryLists : *Index)
// Examine all summaries for this GUID.
for (auto &Summary : GUIDSummaryLists.second.SummaryList)
- if (auto FS = dyn_cast<FunctionSummary>(Summary.get()))
+ if (auto FS = dyn_cast<FunctionSummary>(Summary.get())) {
// For each call in the function summary, see if the call
// is to a GUID (which means it is for an indirect call,
// otherwise we would have a Value for it). If so, synthesize
@@ -207,6 +207,15 @@ class ModuleBitcodeWriterBase : public BitcodeWriterBase {
for (auto &CallEdge : FS->calls())
if (!CallEdge.first.haveGVs() || !CallEdge.first.getValue())
assignValueId(CallEdge.first.getGUID());
+
+ // 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
+ // value id.
+ for (auto &RefEdge : FS->refs())
+ if ((!RefEdge.haveGVs() || !RefEdge.getValue()))
+ assignValueId(RefEdge.getGUID());
+ }
}
protected:
@@ -4071,7 +4080,7 @@ void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
NameVals.push_back(SpecialRefCnts.second); // worefcnt
for (auto &RI : FS->refs())
- NameVals.push_back(VE.getValueID(RI.getValue()));
+ NameVals.push_back(getValueId(RI));
const bool UseRelBFRecord =
WriteRelBFToSummary && !F.hasProfileData() &&
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 598f3fafb2bd90f..925e0fb67e1ee20 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1285,46 +1285,44 @@ void annotateValueSite(Module &M, Instruction &Inst,
Inst.setMetadata(LLVMContext::MD_prof, MDNode::get(Ctx, Vals));
}
-bool getValueProfDataFromInst(const Instruction &Inst,
- InstrProfValueKind ValueKind,
- uint32_t MaxNumValueData,
- InstrProfValueData ValueData[],
- uint32_t &ActualNumValueData, uint64_t &TotalC,
- bool GetNoICPValue) {
+MDNode *mayHaveValueProfileOfKind(const Instruction &Inst,
+ InstrProfValueKind ValueKind) {
MDNode *MD = Inst.getMetadata(LLVMContext::MD_prof);
if (!MD)
- return false;
+ return nullptr;
- unsigned NOps = MD->getNumOperands();
+ if (MD->getNumOperands() < 5)
+ return nullptr;
- if (NOps < 5)
- return false;
-
- // Operand 0 is a string tag "VP":
MDString *Tag = cast<MDString>(MD->getOperand(0));
- if (!Tag)
- return false;
-
- if (!Tag->getString().equals("VP"))
- return false;
+ if (!Tag || !Tag->getString().equals("VP"))
+ return nullptr;
// Now check kind:
ConstantInt *KindInt = mdconst::dyn_extract<ConstantInt>(MD->getOperand(1));
if (!KindInt)
- return false;
+ return nullptr;
if (KindInt->getZExtValue() != ValueKind)
- return false;
+ return nullptr;
+
+ return MD;
+}
+static bool getValueProfDataFromInst(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));
if (!TotalCInt)
return false;
TotalC = TotalCInt->getZExtValue();
-
ActualNumValueData = 0;
for (unsigned I = 3; I < NOps; I += 2) {
- if (ActualNumValueData >= MaxNumValueData)
+ if (ActualNumValueData >= MaxNumDataWant)
break;
ConstantInt *Value = mdconst::dyn_extract<ConstantInt>(MD->getOperand(I));
ConstantInt *Count =
@@ -1341,6 +1339,33 @@ bool getValueProfDataFromInst(const Instruction &Inst,
return true;
}
+std::unique_ptr<InstrProfValueData[]>
+getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind,
+ uint32_t MaxNumValueData, uint32_t &ActualNumValueData,
+ uint64_t &TotalC, bool GetNoICPValue) {
+ MDNode *MD = mayHaveValueProfileOfKind(Inst, ValueKind);
+ if (!MD)
+ return nullptr;
+ auto ValueDataArray = std::make_unique<InstrProfValueData[]>(MaxNumValueData);
+ if (!getValueProfDataFromInst(MD, MaxNumValueData, ValueDataArray.get(),
+ ActualNumValueData, TotalC, GetNoICPValue))
+ return nullptr;
+ return ValueDataArray;
+}
+
+bool getValueProfDataFromInst(const Instruction &Inst,
+ InstrProfValueKind ValueKind,
+ uint32_t MaxNumValueData,
+ InstrProfValueData ValueData[],
+ uint32_t &ActualNumValueData, uint64_t &TotalC,
+ bool GetNoICPValue) {
+ MDNode *MD = mayHaveValueProfileOfKind(Inst, ValueKind);
+ if (!MD)
+ return false;
+ return getValueProfDataFromInst(MD, MaxNumValueData, ValueData,
+ ActualNumValueData, TotalC, GetNoICPValue);
+}
+
MDNode *getPGOFuncNameMetadata(const Function &F) {
return F.getMetadata(getPGOFuncNameMetadataName());
}
diff --git a/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll b/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll
index 78b175caca85f01..28e4b1d19aef72c 100644
--- a/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll
+++ b/llvm/test/Bitcode/thinlto-func-summary-vtableref-pgo.ll
@@ -3,18 +3,23 @@
; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
; RUN: llvm-dis -o - %t.o | FileCheck %s --check-prefix=DIS
-
+; Round trip it through llvm-as
+; RUN: llvm-dis -o - %t.o | llvm-as -o - | llvm-dis -o - | FileCheck %s --check-prefix=DIS
; CHECK: <GLOBALVAL_SUMMARY_BLOCK
; CHECK-NEXT: <VERSION op0=9/>
; 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/>
; The `VALUE_GUID` below represents the "_ZN4Base4funcEv" referenced by the
; indirect call instruction.
; CHECK-NEXT: <VALUE_GUID op0=17 op1=5459407273543877811/>
; <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=0 op5=0 op6=0 op7=17 op8=3/>
+; 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: </GLOBALVAL_SUMMARY_BLOCK>
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
@@ -56,6 +61,7 @@ define i32 @_Z4testP4Base(ptr %0) !prof !15 {
; 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.
; DIS: ^0 = module: (path: "{{.*}}", hash: (0, 0, 0, 0, 0))
-; DIS: ^1 = gv: (guid: 5459407273543877811)
-; DIS: ^2 = gv: (name: "_Z4testP4Base", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 4, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 0, hasUnknownCall: 1, mustBeUnreachable: 0), calls: ((callee: ^1, hotness: hot))))) ; guid = 15857150948103218965
-; DIS: ^3 = blockcount: 0
+; DIS: ^1 = gv: (guid: 1960855528937986108)
+; DIS: ^2 = gv: (guid: 5459407273543877811)
+; DIS: ^3 = gv: (name: "_Z4testP4Base", summaries: (function: (module: ^0, flags: (linkage: external, visibility: default, notEligibleToImport: 0, live: 0, dsoLocal: 0, canAutoHide: 0), insts: 4, funcFlags: (readNone: 0, readOnly: 0, noRecurse: 0, returnDoesNotAlias: 0, noInline: 0, alwaysInline: 0, noUnwind: 0, mayThrow: 0, hasUnknownCall: 1, mustBeUnreachable: 0), calls: ((callee: ^2, hotness: hot)), refs: (readonly ^1)))) ; guid = 15857150948103218965
+; DIS: ^4 = blockcount: 0
|
// array. The actual number of values is gated by the number of ops in !prof | ||
// metadata. | ||
auto ValueDataArray = getValueProfDataFromInst( | ||
*I, IPVK_VTableTarget, 24 /* MaxNumValueData */, ActualNumValueData, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really want 24? Or should we limit to something smaller (and tunable), like we do for virtual function pointer profiles in ICP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the number of vtables is too many compared with the number of functions (i.e., 3 indirect call candidates from 10 vtables, ICP transformation heuristic should choose function comparison based on my experiment). I'll introduce a cl::opt
option and limits the default value as 6 (2x of function's default)
…se it when generating function summaries. Created using spr 1.3.4
✅ With the latest revision this PR passed the C/C++ code formatter. |
Created using spr 1.3.4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with 2 minor comment nits
auto ValueDataArray = getValueProfDataFromInst( | ||
*I, IPVK_VTableTarget, 24 /* MaxNumValueData */, ActualNumValueData, | ||
TotalCount); | ||
*I, IPVK_VTableTarget, MaxNumVTableAnnotations /* MaxNumValueData */, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove the "/* MaxNumValueData */" now that this is not a constant. Or if you want to keep it use the format shown in my other comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
if (ValueDataArray.get()) { | ||
for (uint32_t j = 0; j < ActualNumValueData; j++) { | ||
RefEdges.insert(Index.getOrInsertValueInfo( | ||
ValueDataArray[j].Value /* VTableGUID */)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use format "/* VTableGUID = */ ValueDataArray[j].Value" for consistency with other code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
||
// 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// declarations or definitions in the module). If so, sythesize a | |
// declarations or definitions in the module). If so, synthesize a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
llvm/lib/ProfileData/InstrProf.cpp
Outdated
|
||
static bool getValueProfDataFromInst(const MDNode *const MD, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a plan to consolidate this function so that it's no longer a stub for existing callsites? AFAICT the std::unique_ptr<InstrProfValueData[]>
version is functionally identical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, existing callers of the bool
version no longer get the checking from mayHaveValueProfileOfKind
which seems to be an unintended change of functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a plan to consolidate this function so that it's no longer a stub for existing callsites?
I could do the refactoring this before major implementation changes are in, so this pr picks up unique_ptr<Array>
interface soon. Will leave this PR as it is for now, and update it after refactoring one is in the main branch.
Also, existing callers of the bool version no longer get the checking from mayHaveValueProfileOfKind which seems to be an unintended change of functionality
There are two bool
version in this pr, one is this static function and the other one is non-static function (called by existing callers of bool
version). The non-static bool
version gets the checking from mayHaveValueProfileOfKind
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could do the refactoring this before major implementation changes are in, so this pr picks up unique_ptr interface soon. Will leave this PR as it is for now, and update it after refactoring one is in the main branch.
Definitely don't need to change this PR, just making sure it's something that's on your radar so we can keep the code clean.
There are two bool version in this pr, one is this static function and the other one is non-static function (called by existing callers of bool version). The non-static bool version gets the checking from mayHaveValueProfileOfKind.
I see, or rather I see now that you told me 😆. Given that consider renaming the static function (getValueProfDataFromInstImpl
?) to make it easier to distinguish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good! Done.
Created using spr 1.3.4
Created using spr 1.3.4
A precommit test case to show function summary and global values when a function has instructions annotated with vtable profiles and indirect call profiles. - This is a precommit test for #79381
…ization and indirect call value profile update into one helper function (#80762) * The motivation is to move indirect callee profile update inside the function-based speculative indirect-call promotion, so that there are fewer diffs the vtable-based transformation and profile update is implemented in a follow-up patch. * The Parent patch is #79381
Add annotated vtable GUID as referenced variables in per function summary, and update bitcode writer to create value-ids for these referenced vtables.