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

[Wenlei He via Phabricator via llvm-commits]

wenlei 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());
+
----------------
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. 


================
Comment at: llvm/tools/llvm-profgen/PerfReader.h:374
+// Offset-based context id
+struct OffsetBasedCtxKey : public ContextKey {
   SmallVector<uint64_t, 16> Context;
----------------
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..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126827