On Thu, 2013-12-12 at 11:06 -0800, Josh Stone wrote:
On 12/12/2013 04:13 AM, Petr Machata wrote:
> Josh Stone <jistone(a)redhat.com> writes:
>> +#define get_sleb128_step(var, addr, nth) \
>> do { \
>> + unsigned char __b = *(addr)++; \
>> + if (likely ((__b & 0x80) == 0)) \
>> { \
>> + struct { signed int i:7; } __s = { .i = __b }; \
>> + (var) |= (typeof (var)) __s.i << ((nth) * 7); \
>
> Oh, the bitfield trick is clever!
I should give credit, I found that trick here:
http://graphics.stanford.edu/~seander/bithacks.html#FixedSignExtend
The former code was trying to sign-extend after the value was shifted
and combined, which as a variable width is harder. I really like that
this way the compiler is fully aware that this is a sign extension,
rather than being a side effect of ORing bits or left-right shifts.
Sadly the neat trick triggers undefined behavior since we are trying to
left shift a negative value. Even though it appears to work currently I
am slightly afraid a compiler optimization might take advantage of this
in the future (since it is undefined behavior it could just assume
negative values won't occur) especially since this code is inlined in a
lot of places, causing hard to diagnose errors.
The attached patch is very much not clever, but does what is intended in
a well-defined way (it is basically what the DWARF spec gives as pseudo
code). Does anybody see a better way?
Thanks,
Mark