Base address can be loaded from MMP event, but pseudo probe address is always based on preferred loading address. If the base address is not equal to the preferred loading address, the pseudo probe address query will be wrong. Recover the base address to preferred loading address in order to match pseudo probe address correctly.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
1199 | 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. | |
llvm/tools/llvm-profgen/PerfReader.h | ||
374 ↗ | (On Diff #433547) | Perhaps AddrBasedCtxKey as name is just fine? This is in contrast with StringBasedCtxKey, so Addr as a general term isn't a big deal, I don't have strong opinion though.. |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
1199 | 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
#2 sounds more practical. WDYT? |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
1199 | Hmm, I found it's very tricky case.
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 | |
llvm/tools/llvm-profgen/PerfReader.h | ||
374 ↗ | (On Diff #433547) | Sounds good. |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
1199 | 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 |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
1199 | I see. This is complicated. How about we always use preferred address when doing this offset to virtual addr conversion, and we rebase all LBR and stack sample addresses to be on the preferred load address in the the perf reader ? |
Thanks for the work! It should be more reliable and also looks cleaner.
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
745 | I'm a bit confused here. What is the semantics of using both offset and LoadableSegmentAsBase? |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
745 | I think this is to be compatible to our internal old tool where some services use offset and the FirstLoadableAddress as the output of unsymbolized profile. The original patch is https://reviews.llvm.org/D113727. |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
745 | Do we still need this now? The offset here is computed by subtracting the runtime base address from preferred-based virtual address. This seems different from what previous computed, ie., subtracting the runtime base address from runtime virtual address. |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
745 | It should be the same behavior as previous offset computation. This is about how we define the "offset": Assuming MMAP loading address is missing or just equal to preferred-based-address, then the virtual address = offset + preferred-based-address. and for this two branches, if it's UseLoadableSegmentAsBase is False, then the offset is virtual address - preferred-based-address --> offset + preferred-based-address - preferred-based-address which is the original "offset". but when UseLoadableSegmentAsBase is True, the offset is virtual address - FirstLoadableAddress -->offset + preferred-based-address - FirstLoadableAddress Yeah, but if we won't use the old tool anymore, I think we can remove all the UseLoadableSegmentAsBase code. |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
745 |
Could the "wrong" offsets now cause trouble for pseudo probes which always have the "correct" offset? It should work fine previously?
Agreed. |
llvm/tools/llvm-profgen/PerfReader.cpp | ||
---|---|---|
745 | I looked a bit deeper. The "wrong" offsets should work. getFirstLoadableAddress stands for the preferred first load address, not the runtime one where I got the impression from its definition. // The runtime base address that the first loadabe segment is loaded at. uint64_t FirstLoadableAddress = 0; Then we no longer need to remove the feature in this change. |