[cfe-dev] BitstreamReader::GetCurrentBitNo() counterintuitive
Ted Kremenek
kremenek at apple.com
Tue Dec 23 15:29:57 PST 2008
Part of the problem is that "Deserialize.cpp" (in llvm) uses
GetCurrentBitNo() in a whole bunch of places. AST serialization/
deserialization is the only client of the serialization library, and
there aren't that many test cases in the clang test suite for AST
serialization because it just hasn't been completed yet. I have a
feeling that changing the behavior of GetCurrentBitNo() will break
things.
I'm actually a little surprised that GetCurrentBitNo() needs to be
changed. While the AST serialization work is incomplete, it has
worked for non-trivial cases. Since it uses GetCurrentBitNo() to jump
around the bitcode file, I'm suspicious that anything needs to be
changed.
Also, what happens if NextChar == FirstChar? Wouldn't (NextChar-
FirstChar-4)*8 be a negative number?
On Dec 23, 2008, at 11:39 AM, Chris Lattner wrote:
>
> On Dec 23, 2008, at 12:36 AM, Zhongxing Xu wrote:
>
>> As the comments said, BitstreamReader::GetCurrentBitNo() should
>> return the bit # of the bit we are reading. But the current
>> implementation actually return 4 more bytes than the bit we are
>> reading. It calculate the bit number by
>>
>> (NextChar-FirstChar)*8 + ((32-BitsInCurWord) & 31);
>>
>> But 'NextChar' is already after the word that we are reading. I
>> propose to fix this by the following patch.
>
> Hi Zhongxing,
>
> This is ok as long as it doesn't cause any regressions in the *llvm*
> dejagnu regression tests, thanks!
>
> -Chris
>
>>
>>
>> Index: include/llvm/Bitcode/BitstreamReader.h
>> ===================================================================
>> --- include/llvm/Bitcode/BitstreamReader.h (版本 61317)
>> +++ include/llvm/Bitcode/BitstreamReader.h (工作副本)
>> @@ -114,7 +114,8 @@
>>
>> /// GetCurrentBitNo - Return the bit # of the bit we are reading.
>> uint64_t GetCurrentBitNo() const {
>> - return (NextChar-FirstChar)*8 + ((32-BitsInCurWord) & 31);
>> + return (NextChar-FirstChar-4)*8 + (BitsInCurWord ? (32-
>> BitsInCurWord) & 31
>> + : 32);
>> }
>>
>> /// JumpToBit - Reset the stream to the specified bit number.
>> @@ -130,7 +131,6 @@
>>
>> // Skip over any bits that are already consumed.
>> if (WordBitNo) {
>> - NextChar -= 4;
>> Read(static_cast<unsigned>(WordBitNo));
>> }
>> }
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
More information about the cfe-dev
mailing list