[PATCH] D102039: [profile] WIP Add binary id into profiles

[Gulfem Savrun Yeniceri via Phabricator via llvm-commits]

gulfem added inline comments.


================
Comment at: compiler-rt/include/profile/InstrProfData.inc:132
 INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version())
+INSTR_PROF_RAW_HEADER(uint64_t, HasBuildId, 0) // TODO: Can use a bool?
 INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize)
----------------
gulfem wrote:
> phosek wrote:
> > Instead of `HasBuildId` as a boolean, I think it might be better to use `BuildIdSize` to allow multiple ids. While raw profiles should have either 0 or 1 in practice, indexed profiles created by merging multiple raw profiles could have multiple ids, so using `BuildIdSize` would allow using the same header format for both  raw and indexed format.
> Can a raw profile **with** build id and another raw profile **without** a build id be merged and create indexed profiled?
@phosek, it seems to me like `raw` and `indexed` profile do not share the same profile format.
For ex, `raw` profile header is defined in:
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ProfileData/InstrProfData.inc#L130
 
Whereas,  `indexed` profile header is defined in:
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ProfileData/InstrProf.h#L999

I think, we might need to extend both formats with binary id then.
Please let me know if I'm missing anything. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102039