[PATCH] D113727: [llvm-profgen] Add switch to allow use of first loadable segment for calculating offset

[Lei Wang via Phabricator via llvm-commits]

wlei accepted this revision.
wlei added a comment.

LGTM, thanks for the change!



================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:712
+
+      if (!UseOffset || (UseOffset && UseLoadableSegmentAsBase)) {
+        Start = Binary->offsetToVirtualAddr(Start);
----------------
wlei wrote:
> Wondering if we can avoid changing the code here.
> 
> I'm thinking like if we can refactor all `getPreferredBaseAddress` to `getBaseAddress()` or a new function.
> 
> Then we have code early in the binary to `setBaseAddress` like:
> ```
> if (UseLoadableSegmentAsBase)
>    setBaseAddress(getFirstLoadableAddress())
> else
>    setBaseAddress(getPreferredBaseAddress())
> ```
> 
> Then here `Binary->offsetToVirtualAddr(..);` will cover all the offset cases.
> 
> 
> 
> 
> 
> 
> 
> 
> 
I realized we might not change `getPreferredBaseAddress ` to `getFirstLoadableAddress` for disassembling, all the CallOffsets, RetOffsets,.. are based on that.  Here It's only for the unsymbolized profile, should be fine. Feel free to ignore this comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113727