[Mesa-dev] [PATCH 1/9] nir: Fix constant evaluation of bfm.

[Connor Abbott]

On Mon, Jan 11, 2016 at 6:52 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 01/11/2016 02:48 PM, Matt Turner wrote:
>> NIR's bfm, like Intel/AMD's hardware instructions and GLSL IR's
>> ir_binop_bfm takes <bits> as src0 and <offset> as src1.
>
> All the questions...
>
> Is the ordering of the operands documented anywhere?  I was only able to
> deduce this by looking at glsl_to_nir.cpp (and then
> ir_constant_expression.cpp).  Also notice that ir_binop_bfm is also
> woefully underdocumented. :(

FWIW, the idea is that these constant-folding expressions are supposed
to *be* the documentation for the opcode, with the additional benefit
that they actually can be tested. It's been a little difficult to put
that advantage into practice with GLSL IR doing its own constant
folding, resulting in bugs like this, but with SPIR-V it should get
easier. At least in this case, a testcase showed that it was wrong,
rather than some documentation silently sitting there and bitrotting.

>
> Is there a test case that hits this?
>
> It looks like this code has existed since January 2015.  Should this be
> tagged for stable?
>
> Either way, this patch is
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>
>> ---
>>  src/glsl/nir/nir_opcodes.py | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/nir/nir_opcodes.py b/src/glsl/nir/nir_opcodes.py
>> index d31507f..398ae50 100644
>> --- a/src/glsl/nir/nir_opcodes.py
>> +++ b/src/glsl/nir/nir_opcodes.py
>> @@ -512,7 +512,7 @@ binop_horiz("pack_half_2x16_split", 1, tuint, 1, tfloat, 1, tfloat,
>>              "pack_half_1x16(src0.x) | (pack_half_1x16(src1.x) << 16)")
>>
>>  binop_convert("bfm", tuint, tint, "", """
>> -int offset = src0, bits = src1;
>> +int bits = src0, offset = src1;
>>  if (offset < 0 || bits < 0 || offset + bits > 32)
>>     dst = 0; /* undefined per the spec */
>>  else
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev