[PATCH] D103178: [CSSPGO][llvm-profgen] Allow multiple executable load segments.

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:349
+        assert(*It < Event.Offset);
+        if (Event.Offset - *It != Event.BaseAddress - Binary.getBaseAddress())
+          exitWithError("Segment not loaded by consecutive mmaps");
----------------
hoy wrote:
> wenlei wrote:
> > another nit: we named MMapEvent::BaseAddress based on the assumption of single text segment and single load, in which case the address is always the base load address of the binary. now it turns out that's not always the case, perhaps Event::MappedAddress is more appropriate? 
> `Event::MappedAddress` sounds good to me, or how about name it just `Address`?
> 
> 
> ```
>   struct MMapEvent {
>     uint64_t PID = 0;
>     uint64_t BaseAddress = 0;
>     uint64_t Size = 0;
>     uint64_t Offset = 0;
>     StringRef BinaryPath;
>   };
> ```
> 
Address is good too. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103178