[Mesa-dev] [PATCH 7/9] nir: Change bfm's semantics to match Intel/AMD/SM5.

[Ian Romanick]

On 01/11/2016 04:14 PM, Matt Turner wrote:
> On Mon, Jan 11, 2016 at 4:01 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 01/11/2016 02:48 PM, Matt Turner wrote:
>>> Intel/AMD's hardware instructions do not handle arguments of 32.
>>> Constant evaluation should not produce a result different from the
>>> hardware instruction.
>>> ---
>>>  src/glsl/nir/nir_opcodes.py | 9 ++++++---
>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/src/glsl/nir/nir_opcodes.py b/src/glsl/nir/nir_opcodes.py
>>> index 9e92fab..ccf9498 100644
>>> --- a/src/glsl/nir/nir_opcodes.py
>>> +++ b/src/glsl/nir/nir_opcodes.py
>>> @@ -511,12 +511,15 @@ binop("fpow", tfloat, "", "powf(src0, src1)")
>>>  binop_horiz("pack_half_2x16_split", 1, tuint, 1, tfloat, 1, tfloat,
>>>              "pack_half_1x16(src0.x) | (pack_half_1x16(src1.x) << 16)")
>>>
>>> +# bfm implements the behavior of the first operation of the SM5 "bfi" assembly
>>> +# and that of the "bfi1" i965 instruction. That is, it has undefined behavior
>>> +# if either of its arguments are 32.
>>>  binop_convert("bfm", tuint, tint, "", """
>>>  int bits = src0, offset = src1;
>>> -if (offset < 0 || bits < 0 || offset + bits > 32)
>>> -   dst = 0; /* undefined per the spec */
>>> +if (offset < 0 || bits < 0 || offset > 31 || bits > 31 || offset + bits > 32)
>>> +   dst = 0; /* undefined */
>>>  else
>>> -   dst = ((1ul << bits) - 1) << offset;
>>> +   dst = ((1u << bits) - 1) << offset;
>>
>> Was s/1ul/1u/ intentional?
> 
> Yes -- previously we wanted to handle the "1 << 32" case so we needed
> to use 1ul (actually, 1ull I suppose?) but now we're okay with that
> being undefined so by changing s/1ul/1ull/ we save a little bit by not
> needing a 64-bit operation.

That makes sense.  Maybe mention that in the commit message?  Either
way, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>