[PATCH] D112282: [llvm-profgen] Switch to DWARF-based symbol and ranges

[Hongtao Yu via Phabricator via llvm-commits]

hoy accepted this revision.
hoy added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:362-363
+    auto *Symbol = Binary->findDWARFSymbolForOffset(RangeBegin);
+    if (!Symbol)
+      continue;
+
----------------
wlei wrote:
> wenlei wrote:
> > wlei wrote:
> > > wenlei wrote:
> > > > Just double check, the warnings are not being removed, but moved and covered by a separate patch, right?
> > > > 
> > > > Not a big deal, but it would be good for review if you can either 1) still keep it for review, 2) make a stack on top of the other patch so the warning doesn't show up on left side at all.. 
> > > Good point. I actually intend to remove the warning here because I found some code in plt section which don't match any DWARF symbol will trigger this warning and it's a false positive. Added some comments to this.
> > > 
> > > 
> > > 
> > > because I found some code in plt section which don't match any DWARF symbol  
> > 
> > But we're looking for elf symbol here, not dwarf symbol? 
> > 
> > Regardless I'm hoping that we still have some sanity check for profiles so users would get a hint when they use mismatched profile and binary. 
> > 
> > But we're looking for elf symbol here, not dwarf symbol?
>  I think the name is misleading, here it's not the elf symbol. Sorry for the confusing. Changed to `FuncRange` as you suggested.
> 
> > Regardless I'm hoping that we still have some sanity check for profiles so users would get a hint when they use mismatched profile and binary.
> Yeah, I will pick up https://reviews.llvm.org/D111902 for the sanity check.
Perhaps the sanity check can be done during the parsing of LBR traces so that errors will not be propagated too far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112282