[Nouveau] [PATCH 2/4] nvc0/ir: Handle reverse subop for OP_EXTBF when folding constant expressions

[Ilia Mirkin]

I think you misunderstood my suggestion. This code won't actually get
executed when running that test. What I was suggesting was to change
the shift/width argument passed to extbf when converting BREV, which
should in turn cause the test to start failing. The way in which you
need to modify the expected value the test generates will dictate
whether the bit reverse happens before or after the bitfield
extraction.

On Thu, May 29, 2014 at 3:53 PM, Tobias Klausmann
<tobias.johannes.klausmann at mni.thm.de> wrote:
> Tested with:
> MESA_EXTENSION_OVERRIDE=GL_ARB_gpu_shader5 ./shader_runner
> ../tests/spec/arb_gpu_shader5/execution/built-in-functions/fs-bitfieldReverse.shader_test
> -> green output, so this should be ok
>
> the test was not change though...
>
>
>
> On 29.05.2014 21:47, Ilia Mirkin wrote:
>>
>> Can you verify that you tested how the HW handles this, as well as
>> exactly how you did it (i.e. how did you modify the code + piglit
>> test, what the results were, etc)
>>
>> On Thu, May 29, 2014 at 3:43 PM, Tobias Klausmann
>> <tobias.johannes.klausmann at mni.thm.de> wrote:
>>>
>>> Signed-off-by: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de>
>>> ---
>>>   src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp | 14
>>> ++++++++++++--
>>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>>> index 58092f4..93f7c2a 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>>> @@ -529,8 +529,18 @@ ConstantFolding::expr(Instruction *i,
>>>            lshift = 32 - width - offset;
>>>         }
>>>         switch (i->dType) {
>>> -      case TYPE_S32: res.data.s32 = (a->data.s32 << lshift) >> rshift;
>>> break;
>>> -      case TYPE_U32: res.data.u32 = (a->data.u32 << lshift) >> rshift;
>>> break;
>>> +      case TYPE_S32: {
>>> +         res.data.s32 = (res.data.s32 << lshift) >> rshift;
>>> +         if (i->subOp == NV50_IR_SUBOP_EXTBF_REV)
>>> +            res.data.s32 = util_bitreverse(res.data.s32);
>>> +         break;
>>> +      }
>>> +      case TYPE_U32: {
>>> +         res.data.u32 = (res.data.u32 << lshift) >> rshift;
>>> +         if (i->subOp == NV50_IR_SUBOP_EXTBF_REV)
>>> +            res.data.u32 = util_bitreverse(res.data.u32);
>>> +         break;
>>> +      }
>>>         default:
>>>            return;
>>>         }
>>> --
>>> 1.8.4.5
>>>
>