-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[ProfileData] Add a variant of getValueProfDataFromInst #95993
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
[ProfileData] Add a variant of getValueProfDataFromInst #95993
Conversation
This patch adds a variant of getValueProfDataFromInst that returns std::vector<InstrProfValueData> instead of std::unique<InstrProfValueData[]>. The new return type carries the length with it, so we can drop out parameter ActualNumValueData. Also, the caller can directly feed the return value into a range-based for loop as shown in the patch. I'm planning to migrate other callers of getValueProfDataFromInst to the new variant in follow-up patches.
@llvm/pr-subscribers-pgo @llvm/pr-subscribers-llvm-analysis Author: Kazu Hirata (kazutakahirata) ChangesThis patch adds a variant of getValueProfDataFromInst that returns I'm planning to migrate other callers of getValueProfDataFromInst to Full diff: https://github.com/llvm/llvm-project/pull/95993.diff 3 Files Affected:
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index df79073da5b50..2150a5ec8dc21 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -294,6 +294,13 @@ getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind,
uint32_t MaxNumValueData, 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 an empty vector otherwise.
+std::vector<InstrProfValueData>
+getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind,
+ uint32_t MaxNumValueData, 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 c6934f55ee114..94ac0484f5ec7 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -143,20 +143,15 @@ static bool findRefEdges(ModuleSummaryIndex &Index, const User *CurUser,
const Instruction *I = dyn_cast<Instruction>(CurUser);
if (I) {
- uint32_t ActualNumValueData = 0;
uint64_t TotalCount = 0;
// MaxNumVTableAnnotations is the maximum number of vtables annotated on
// the instruction.
- 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));
- }
- }
+ auto ValueDataArray = getValueProfDataFromInst(
+ *I, IPVK_VTableTarget, MaxNumVTableAnnotations, TotalCount);
+
+ for (const auto &V : ValueDataArray)
+ RefEdges.insert(Index.getOrInsertValueInfo(/* VTableGUID = */
+ V.Value));
}
return HasBlockAddress;
}
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index 8cc1625e1c05c..e94cdc3b3235e 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1381,6 +1381,43 @@ getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind,
return ValueDataArray;
}
+std::vector<InstrProfValueData>
+getValueProfDataFromInst(const Instruction &Inst, InstrProfValueKind ValueKind,
+ uint32_t MaxNumValueData, uint64_t &TotalC,
+ bool GetNoICPValue) {
+ std::vector<InstrProfValueData> ValueData;
+ MDNode *MD = mayHaveValueProfileOfKind(Inst, ValueKind);
+ if (!MD)
+ return ValueData;
+ const unsigned NOps = MD->getNumOperands();
+ // Get total count
+ ConstantInt *TotalCInt = mdconst::dyn_extract<ConstantInt>(MD->getOperand(2));
+ if (!TotalCInt)
+ return ValueData;
+ TotalC = TotalCInt->getZExtValue();
+
+ ValueData.reserve(MaxNumValueData);
+ for (unsigned I = 3; I < NOps; I += 2) {
+ if (ValueData.size() >= MaxNumValueData)
+ break;
+ ConstantInt *Value = mdconst::dyn_extract<ConstantInt>(MD->getOperand(I));
+ ConstantInt *Count =
+ mdconst::dyn_extract<ConstantInt>(MD->getOperand(I + 1));
+ if (!Value || !Count) {
+ ValueData.clear();
+ return ValueData;
+ }
+ uint64_t CntValue = Count->getZExtValue();
+ if (!GetNoICPValue && (CntValue == NOMORE_ICP_MAGICNUM))
+ continue;
+ InstrProfValueData V;
+ V.Value = Value->getZExtValue();
+ V.Count = CntValue;
+ ValueData.push_back(V);
+ }
+ return ValueData;
+}
+
MDNode *getPGOFuncNameMetadata(const Function &F) {
return F.getMetadata(getPGOFuncNameMetadataName());
}
|
I've switched to |
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 but it'd be good to wait a little bit more for Ellis's comments
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/139/builds/348 Here is the relevant piece of the build log for the reference:
|
This patch adds a variant of getValueProfDataFromInst that returns std::vector<InstrProfValueData> instead of std::unique<InstrProfValueData[]>. The new return type carries the length with it, so we can drop out parameter ActualNumValueData. Also, the caller can directly feed the return value into a range-based for loop as shown in the patch. I'm planning to migrate other callers of getValueProfDataFromInst to the new variant in follow-up patches.
This patch adds a variant of getValueProfDataFromInst that returns
std::vector instead of
std::unique<InstrProfValueData[]>. The new return type carries the
length with it, so we can drop out parameter ActualNumValueData.
Also, the caller can directly feed the return value into a range-based
for loop as shown in the patch.
I'm planning to migrate other callers of getValueProfDataFromInst to
the new variant in follow-up patches.