Re: [PATCH 2/3] Simplify and inline get_uleb128 and get_sleb128

[Richard Henderson <rth@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]

On 04/23/2014 11:32 AM, Petr Machata wrote:
> Richard Henderson <rth@redhat.com> writes:
> 
>> On 04/23/2014 03:17 AM, Petr Machata wrote:
>>> Wouldn't something like this get us off the hook as well?
>>>
>>> -	(var) |= (typeof (var)) __s.i << ((nth) * 7);			      \
>>> +	(var) |= (typeof (var))						      \
>>> +	  (((uint64_t) (typeof (var)) __s.i) << ((nth) * 7));		      \
>>>
>>> We are really only using the bitfield trick to avoid having to
>>> sign-extend by hand, but we can shift unsigned without losing anything.
>>
>> It gets us off the hook, but might introduce a 64-bit shift where
>> only a 32-bit shift was required.
> 
> Good point, but get_sleb128_step is only used from __libdw_get_sleb128,
> where the type is int64_t.  This macro is not safe for outside use
> anyway, as it uses its parameters more than once.
> 
> Hmm, should we maybe #undef it after __libdw_get_sleb128, so that it's
> clear that it's for local use only?

In that case, why play around with the typeof at all?  Just merge the macro
into its only user.  At which point using uint64_t makes total sense in context.

Similarly with get_uleb128_step, I assume, at least wrt to the typeof.


r~