[PATCH] D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:181
+    bool ShouldInline = false;
+    if ((ShouldInline = shouldInline(Candidate))) {
+      // We mark context as inined as the corresponding context profile
----------------
The definition of ShouldInline is only used in LLVM_DEBUG. It may trigger warning in release mode.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:181-189
+    if ((ShouldInline = shouldInline(Candidate))) {
+      // We mark context as inined as the corresponding context profile
+      // won't be merged into that function's base profile.
+      ContextTracker.markContextSamplesInlined(Candidate.CalleeSamples);
+      Candidate.CalleeSamples->getContext().setAttribute(
+          ContextShouldBeInlined);
+      FuncFinalSize += Candidate.SizeCost;
----------------
wmi wrote:
> The definition of ShouldInline is only used in LLVM_DEBUG. It may trigger warning in release mode.
I didn't find if the candidate should not be inlined, where the context profile is merged into the base profile. Could you show me?


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:199
+    if (!CQueue.empty())
+      dbgs() << "  Inline candidates ignored due to size limit (inliner "
+                "original size: "
----------------
Included in LLVM_DEBUG.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:221
+
+  auto printProfileNames = [](StringMap<FunctionSamples> &Profiles,
+                              bool IsInput) {
----------------
printProfileNames is only used in LLVM_DEBUG. Need to include it in #ifndef NDEBUG.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99146/new/

https://reviews.llvm.org/D99146