[PATCH] D94334: [InlineAdvisor] Allow replay of inline decisions for the CGSCC inliner from optimization remarks

[Di Mo via Phabricator via llvm-commits]

modimo added inline comments.


================
Comment at: llvm/lib/Analysis/ReplayInlineAdvisor.cpp:58
+namespace {
+class ReplayInlineAdvice : public InlineAdvice {
+public:
----------------
mtrofin wrote:
> wenlei wrote:
> > This looks redundant/similar to `DefaultInlineAdvice`, is that just for controlling `EmitRemarks`? ORE should be able to handle remark printing (or not) correctly without extra guard. 
> Along the same lines as @wenlei 's comment - if the Advisor can both generate and digest the trace of decisions, why rely on ORE and not, instead, use a more structured format that wouldn't need parsing like ReplayInlineAdvisor.cpp:43?
> 
> Also (if using ORE is desirable, case in which I share @wenlei's question), I think there's a yaml output format ORE generates, perhaps requiring that as input would also simplify ingestion?
I've moved this section to D94333 since the replay mechanism change to consume the new `line:col.discriminator` format needed to be together with format change. I've folded `ReplayInlineAdvice` back into `DefaultInlineAdvice` with the additional features I need. The extra guard is needed because the SampleProfile inliner uses the "legacy PM" mechanism of inline printing rather than bundling it with InlineAdvice calls. Since the use of InlineAdvice in SampleProfile is purely to support replay right now I'm leaving that refactoring (if we want to go after it) for the future.

As far as using yaml I like how condensed the format is in remarks form. Something that's a single line in remarks ends up as 24 lines (like in llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll) which makes manual reading and modification tedious especially on larger binaries. The current text processing is also fairly simple as is which makes a change here less pressing.

That being said I'm not against using the yaml file as the official/supported format. A nice advantage there is that if we wanted to add more replay data (say negative inline decisions) it'll be smoother in yaml than adding new parsing of the text remark.


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

https://reviews.llvm.org/D94334