[PATCH] D90125: [CSSPGO] Infrastructure for context-sensitive Sample PGO and Inlining
David Li via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 29 14:33:32 PDT 2020
davidxl added inline comments.
================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:347
+// State of context associated with FunctionSamples
+enum ContextState {
+ UnknownContext = 0x0, // Profile without context
----------------
Since these states are not mutually exclusive, perhaps name it ContextStateMask?
================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:364
+
+ void promoteOnPath(StringRef ContextStrToRemove) {
+ assert(FullContext.startswith(ContextStrToRemove));
----------------
document this method.
================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:377
+
+ static void DecodeContextString(StringRef ContextStr, StringRef &FName,
+ LineLocation &LineLoc) {
----------------
Document the context string format here.
================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:409
+private:
+ void setContext(StringRef ContextStr, ContextState CState) {
+ assert(!ContextStr.empty());
----------------
what is the input format? document it here.
================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:504
+ if (ret == BodySamples.end()) {
+ // For CSFDO, in order to conserve profile size, we no longer write out
+ // locations profile for those not hit during training, so we need to
----------------
CSFDO ==> CSSPGO
================
Comment at: llvm/include/llvm/Transforms/IPO/SampleContextTracker.h:94
+ SampleContextTracker(StringMap<FunctionSamples> &Profiles);
+ FunctionSamples *getCalleeContextSamplesFor(const CallBase &Inst,
+ StringRef CalleeName);
----------------
Document the public APIs.
================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:225
}
- Profiles[FName] = FunctionSamples();
- FunctionSamples &FProfile = Profiles[FName];
- FProfile.setName(FName);
+ SampleContext FContext(FName);
+ if (FContext.hasContext())
----------------
so FName is also the context String?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90125/new/
https://reviews.llvm.org/D90125
More information about the llvm-commits
mailing list