[PATCH] D86957: [Bitstream] Use alignTo to make code more readable. NFC

stephan.yichao.zhao via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 1 14:17:49 PDT 2020


stephan.yichao.zhao added a comment.

In D86957#2250270 <https://reviews.llvm.org/D86957#2250270>, @craig.topper wrote:

> In D86957#2250245 <https://reviews.llvm.org/D86957#2250245>, @stephan.yichao.zhao wrote:
>
>> In D86957#2250225 <https://reviews.llvm.org/D86957#2250225>, @craig.topper wrote:
>>
>>> In D86957#2250193 <https://reviews.llvm.org/D86957#2250193>, @stephan.yichao.zhao wrote:
>>>
>>>> So this does both 64bit cast and alignment. Thank you.
>>>>
>>>> Hi Craig, in your case, will NumElts be actually larger than 2^32? NumElts is read from https://llvm.org/docs/BitCodeFormat.html#enter-subblock-encoding that defines the blocklen to be 32bit. So when it gets larger, the overflow can happen at the writer side (https://llvm.org/doxygen/BitstreamWriter_8h_source.html#l00384).
>>>
>>> My specific case was a blob for metadata strings that was ~1GB in size. The multipy by 8 to convert its size to bits was overflowing. I do worry that it might break again if the blob of metadata strings exceed 4GB.
>>
>> The case I fixed is similar. One way to address is to extend that blocklen field to 64bit. imo this does not introduce any back-compatibility issue because 32 is not a fixed width, but VBR.
>>
>> 1. when an old reader reads a bitcode written by a new writer, it works if blocklen is <= 2^32. Although it gets broken if blocklen is > 2^32, this case it does not work anyway.
>> 2. when a new reader reads a bitcode written by an old writer, it works fine since blocklen is <= 2^32.
>>
>> So it is possible to extend it to 64bit.
>
> The blocklen field in ENTER_SUBBLOCK isn't a VBR from what I could see. Its just a 32 bit value allowing a maximum block size of 16GB. There is a VBR6 to store the size of the blob. That one we could change to use uint64_t to allow blobs larger than 4GB, but we'd still be limited by the 16GB limit.

Right. blocklen isnt VBR... 16GB is the limit if the bitcode format is not upgraded into a new version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86957



More information about the llvm-commits mailing list