modimo added inline comments.
================
Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:412
+
+ Remark << ";";
}
----------------
wenlei wrote:
> modimo wrote:
> > wenlei wrote:
> > > nit: any special reason for adding this? doesn't seem consistent with
> > > other remarks we have.
> > If you grab the remark outputs via `-Rpass=inline` you'll get additional
> > suffix information:
> > ```
> > inline.cpp:8:12: remark: _Z3foov inlined into main with (cost=0,
> > threshold=375) at callsite main:2:12; [-Rpass=inline]
> > return foo();
> > ```
> >
> > The semicolon is to separate the remark from any additional output at the
> > end so when replaying we can match the correct callsite. Something like
> > this would be unneeded for yaml replay but for the current implementation
> > it's necessary for correctness.
> >
> By correctness, did you mean the fact that you rely on `split(";")` in
> parsing, or something else?
>
> This is not a big deal, but if no other remarks end with `;`, it would be
> good to be consistent. Using `split(";")` for parsing is just one way of
> implementing it, and IMO could be changed to favor consistency in remarks
> output.
> By correctness, did you mean the fact that you rely on `split(";")` in
> parsing, or something else?
Yeah, without that we would store the callsite from remarks as `main:2:12
[-Rpass=inline]` which would not match the actual callsite string `main:2:12`
that we query the map with which causes replay to never inline.
> This is not a big deal, but if no other remarks end with `;`, it would be
> good to be consistent. Using `split(";")` for parsing is just one way of
> implementing it, and IMO could be changed to favor consistency in remarks
> output.
Doing a search query for `OptimizationRemarkAnalysis` I see vectorizer ORE uses
"." for their terminator so switching to that is better consistency. I'll make
the change in an upcoming patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94333/new/
https://reviews.llvm.org/D94333
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
