[PATCH] D126827: [llvm-profgen] Fix a loading address bug for pseudo probe profile

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:1195
+  // preferred loading address in order to match pseduo probe address correctly.
+  Binary->setBaseAddress(Binary->getPreferredBaseAddress());
+
----------------
wlei wrote:
> hoy wrote:
> > wenlei wrote:
> > > Where do we use getBaseAddress in probe queries? Can we use getPreferredBaseAddress directly there? Ideally there should be one definition for base address which is the actual executable segment load address, and changing it here makes it inconsistent. 
> > Agreed that `setBaseAddress` should be called once to be consistent. Pseudo probe decoding is based on the preferred address. I think a reasonable fix could be 
> > 
> > 1) make the decoding offset-based. This will require a change to the bolt code. base; or
> > 2) do an offset to preferred addr translation for every probe look up in llvm-profgen. This is being done in some places, eg, `extractPrefixContextStack`. 
> > 
> > #2 sounds more practical. WDYT?
> Hmm, I found it's very tricky case.
> 
> > Where do we use getBaseAddress in probe queries? 
> 
> We call it in `Binary->offsetToVirtualAddr`.
> 
> However, we call `IP.advance()` to iterate all the address in the range, in `IP` it will convert the `Address` back to `offset` using the base address. like  `Offset + preferred Address - Mmap loading address`. That's another inconsistency..  Then we need to change code inside of the IP. but IP also used in `PerfReader`
> 
> 
> 
> 
>  
Or can we use virtual address(`Offset + preferred Address`) for all the code(PerfReader and ProfileGenerator)? to do this, we can convert the address at the very beginning of PerfReader. like `Address = PhysicalAddress - Mmap loading address + PreferredAddress`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126827