[PATCH] D115699: [llvm-profgen] Skip disassembling for PLT section

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/test/tools/llvm-profgen/inline-noprobe2.test:76
 ;CHECK:   3: 5
-;CHECK: main:820:0
+quick_sort:903:25
+ 1: 24
----------------
wlei wrote:
> wlei wrote:
> > hoy wrote:
> > > wenlei wrote:
> > > > wlei wrote:
> > > > > wenlei wrote:
> > > > > > are these test changes expected?
> > > > > Oh, sorry, it's the same to https://reviews.llvm.org/D115538, previously I removed by mistake. Let me removed this here.
> > > > If you put them in a stack, these earlier/unrelated changes won't show up. 
> > > > 
> > > > But still.. why are the remaining test changes, are they all due to PLT?
> > > Good question. Do the counters changed refer to PLT calls?
> > This patch is in the bottom of the patch stack. It appeared that those counter changes are all fixed by the following two patches. I guess it's due to the wrong leading external addr processing. let me confirm what's wrong.
> OK, it turned out that the PLT address is just the leading LBR's target, remembering before the two external fix, the whole samples will be ignored. 
> 
> ```
> 0x400540 is plt
> 400870 0x400870/0x400540/P/-/-/1  0x4008bf/0x400870/P/-/-/7  0x7f7448e88cc1/0x400875/P/-/-/1  
> ```
> 
> 
> 
I see. So the counters 5.1, 5.3 ... correspond to a call to the PLT entries right?


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:557
 
+    if (SectionName == ".plt")
+      continue;
----------------
wlei wrote:
> hoy wrote:
> > Move this above the `ShowDisassemblyOnly` check?
> `objdump` also print out ".plt" section, I was thinking to print a section name as a hint we have the ".plt" section there(the size is not empty, we just doesn't disassembly it's body). Otherwise people might misunderstand there is no ".plt" section
Makes sense, though I was thinking we'll probably use objdump to look at real disasm, and use llvm-profgen to dump only functions of interest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115699