[PATCH] D100528: [CSSPGO][llvm-profdata] Support trimming cold context when merging profiles

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profdata/CMakeLists.txt:2
 set(LLVM_LINK_COMPONENTS
+  Analysis
   Core
----------------
wenlei wrote:
> wmi wrote:
> > wenlei wrote:
> > > This is because we now use ProfileSummaryInfo.cpp, so we can get all consistent hot/cold threshold computation and related switches, though pulling in entire Analysis lib is less than ideal. I'm thinking about moving ProfileSummaryInfo into ProfileData/ProfileSummaryBuildder.cpo from Analysis/ProfileSummaryInfo.cpp. But I'd like to get feedback before I move things around. 
> > Extract ProfileSummaryInfo from Analysis/ directory sounds reasonable to me. How about move it to Core -- lib/IR/ProfileSummary.cpp for example? The reason is ProfileSummaryInfo interface is used widely. If someday we want to use the interface in Core or Support which ProfileData depends on, it will introduce cyclic dependence. Putting it in Core will clear the problem. 
> Sounds good. Thanks for the suggestion.
Looks like extracting ProfileSummaryInfo to Core isn't a trivial move. ProfileSummaryInfo has a bunch of interfaces taking BFI as parameter, and also offers invalidation, so it depends on Analysis. It's fine when ProfileSummaryInfo itself is part of Analysis, but if we move it to Core, we will have cyclic dependency. For the same reason, we can't move it to ProfileData either. 

If we want to move it, we will need to break it up, so the part that does the threshold computation can be moved, and the part that does the query (relies on Analysis) stays.  

Now what I'm thinking about is only moving the threshold/cutoff switches in ProfileSummaryInfo.cpp along with part of ProfileSummaryInfo::computeThresholds into ProfileData/ProfileSummaryBuilder. That won't change dependencies as ProfileSummaryInfo uses ProfileSummaryBuilder already. But it lets tools that only uses ProfileData access consistent threshold computation too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100528