On 04/22/2014 07:55 AM, Mark Wielaard wrote: > On Thu, 2013-12-12 at 11:06 -0800, Josh Stone wrote: >> On 12/12/2013 04:13 AM, Petr Machata wrote: >>> Josh Stone <[email protected]> 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?
Multiply instead of shift. I.e. (typeof (var)) (__b & 0x7f) * (1 << ((nth) * 7)); With any luck the compiler will even notice it's a multiply by a value with one bit set, and convert it back to a shift during optimization. r~
