[PATCH] D94001: [CSSPGO] Call site prioritized BFS inlining for sample PGO

[Wei Mi via Phabricator via llvm-commits]

wmi added a comment.

Sorry for the delay in review. It makes a lot of sense to have a priority based inliner for CSSPGO since its profile annotation doesn't rely on replaying the inlining. But I don't understand why we rely on BFS/DFS strategy to expose the hottest callsite for priority based inliner. In my mind, CSSPGO can know the hotness of each callsite with full context information. Can we just sort all the callsites  based on CSSPGO profile then try to inline the hottest callsite under a specific context first in a top down manner? We may actually need to inline some parent callsites before we can inline the hottest callsite, but it is all driven by the target to inline the hottest callsite first. If we worry some callsite is too deep and we need to inline through a deep path before we can inline the specific callsite, we may add the depth into priority computation.  What do you think?



================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:426
+  bool getInlineCandidate(InlineCandidate *NewCandidate, CallBase *CB);
+  bool inlineCandidate(InlineCandidate &Candidate,
+                       SmallVector<CallBase *, 8> &InlinedCallSites);
----------------
Since there is another Struct named as InlineCandidate, can we rename it to doInline or executeInline?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94001