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

Fri Mar 26 16:19:53 PDT 2021

wenlei 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
----------------
wmi wrote:
> wenlei wrote:
> > wenlei wrote:
> > > wmi wrote:
> > > > 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?
> > > The merge is done within getBaseSamplesFor on-demand, same as how it's done in compiler.
> > Good point, let me check and adjust.
> Ah, I see. markContextSamplesInlined mark those contexts which will be inlined. For the rest, they are non-inlined and will be merged when getBaseSamplesFor is called. 
Seem the assignment is treated as a use, and we don't have warning in release. But we do have one for printProfileNames, now wrapped in macro.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99146