[Mesa-dev] [PATCH 3/9] i965: fix 64-bit immediates in brw_inst(_set)_bits

[Ilia Mirkin]

On Thu, Nov 19, 2015 at 12:30 PM, Kristian Høgsberg <krh at bitplanet.net> wrote:
> On Thu, Nov 19, 2015 at 2:05 AM, Iago Toral Quiroga <itoral at igalia.com> wrote:
>> From: Connor Abbott <connor.w.abbott at intel.com>
>>
>> If we tried to get/set something that was exactly 64 bits, we would
>> try to do (1 << 64) - 1 to calculate the mask which doesn't give us all
>> 1's like we want.
>>
>> v2 (Iago)
>>  - Replace ~0 by ~0ull
>>  - Removed unnecessary parenthesis
>>
>> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_inst.h | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_inst.h b/src/mesa/drivers/dri/i965/brw_inst.h
>> index 4ed95c4..ec08194 100644
>> --- a/src/mesa/drivers/dri/i965/brw_inst.h
>> +++ b/src/mesa/drivers/dri/i965/brw_inst.h
>> @@ -694,7 +694,8 @@ brw_inst_bits(const brw_inst *inst, unsigned high, unsigned low)
>>     high %= 64;
>>     low %= 64;
>>
>> -   const uint64_t mask = (1ull << (high - low + 1)) - 1;
>> +   const uint64_t mask = (high - low == 63) ? ~0ull :
>> +      (1ull << (high - low + 1)) - 1;
>
> Can we do
>
> const uint64_t mask = (~0ul >> (64 - (high - low + 1)));
>
> instead?
>
> Kristian
>
>>     return (inst->data[word] >> low) & mask;
>>  }
>> @@ -713,7 +714,8 @@ brw_inst_set_bits(brw_inst *inst, unsigned high, unsigned low, uint64_t value)
>>     high %= 64;
>>     low %= 64;
>>
>> -   const uint64_t mask = ((1ull << (high - low + 1)) - 1) << low;
>> +   const uint64_t mask = (high - low == 63) ? ~0ull :
>> +      ((1ull << (high - low + 1)) - 1) << low;

Or if you feel like reusing exising logic, there's
BITFIELD64_MASK/BITFIELD64_RANGE in mtypes.h [which does it the  ? :
way... could be fixed though].

  -ilia