[PATCH] D110365: [llvm][profile] Add padding after binary IDs

Leonard Chan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 23 17:13:08 PDT 2021


leonardchan added a comment.

D110188 <https://reviews.llvm.org/D110188> which originally added the padding was reverted. I'm merging that change into this since they're both tightly coupled. Rather than adding padding after all the build IDs, we instead add padding after *each* build ID such that the build ID sizes are naturally aligned.



================
Comment at: llvm/lib/ProfileData/InstrProfReader.cpp:558
     // Increment by binary id data length.
     BI += BinaryIdLen;
     if (BI > (const uint8_t *)DataBuffer->getBufferEnd())
----------------
mcgrathr wrote:
> What is the packing protocol?  It seems wise to pad after *each* ID to make the next size field naturally-aligned, rather than having the second size field be misaligned if the first build ID length is not a multiple of 8.
> 
Right now there's no alignment and the size just seems to be added after the previous build ID. I updated `__llvm_write_build_ids` to take this suggestion of adding padding after each ID. This should also prevent us from needing to add padding after all the build IDs if there's padding in between each.

I'm wondering though if maybe adding padding after each ID would lead to some bloat compared to just adding padding once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110365



More information about the llvm-commits mailing list