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

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

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
>

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
>>