[PATCH] D122602: [CSSPGO] Rename ProfileIsCSNested and ProfileIsCSFlat

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 28 22:55:52 PDT 2022


hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:200
+      // is a preinliner-computed profile.
+      FunctionSamples::ProfileIsPreInlined = true;
       FuncFinalSize += Candidate.SizeCost;
----------------
wenlei wrote:
> hoy wrote:
> > wenlei wrote:
> > > I think we should set this in `CSPreInliner::run()`? In theory even if preinliner did nothing to the input profile, as long as we run it, the profile should be considered preinlined. 
> > > 
> > > That also avoids setting it in the loop, for each function.
> > Sounds good.
> For the case I mentioned, how do we mark a profile as preinlined for text profile though, if no context actually has the preinline bit, but preinliner was run? 
> 
> Admittedly this is a very corner case.. 
We don't have a way to tell if a text profile is preinlined for that case. But on the other hand, such a text profile would not be considered as preinlined with the previous version. The "shouldInline" flag per context is the only way to tell if a text profile is preinlined or not. 

The new iteration creates a discrepancy between text and extbin profile when there isn't any context with "shouldInline" flag. Such extbin profile will not lead to any inlining in the compiler, but the text counterpart would. I think the behavior of the extbin profile is what we want. Unfortunately that cannot be achieved with text profile. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122602



More information about the llvm-commits mailing list