Nouveau - May 2015 - [PATCH 3/4] nvc0/ir: optimize set & 1.0 to produce boolean-float sets

On 09.05.2015 07:35, Ilia Mirkin wrote:
> This has started to happen more now that the backend is producing
> KILL_IF more often.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>   .../drivers/nouveau/codegen/nv50_ir_peephole.cpp   | 29
++++++++++++++++++++++
>   .../nouveau/codegen/nv50_ir_target_nv50.cpp        |  2 ++
>   2 files changed, 31 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> index 14446b6..d8af19a 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> @@ -973,6 +973,35 @@ ConstantFolding::opnd(Instruction *i, ImmediateValue
&imm0, int s)
>      }
>         break;
>   
> +   case OP_AND:
> +   {
> +      CmpInstruction *cmp = i->getSrc(t)->getInsn()->asCmp();
> +      if (!cmp || cmp->op == OP_SLCT)
how about if (cmp == NULL || ...) and kill the same condition later?
> +         return;
> +      if (!prog->getTarget()->isOpSupported(cmp->op, TYPE_F32))
> +         return;
> +      if (imm0.reg.data.f32 != 1.0)
> +         return;
> +      if (cmp == NULL)
> +         return;
> +      if (i->getSrc(t)->getInsn()->dType != TYPE_U32)
> +         return;
> +
> +      i->getSrc(t)->getInsn()->dType = TYPE_F32;
> +      if (i->src(t).mod != Modifier(0)) {
> +         assert(i->src(t).mod == Modifier(NV50_IR_MOD_NOT));
> +         i->src(t).mod = Modifier(0);
> +         cmp->setCond = reverseCondCode(cmp->setCond);
> +      }
> +      i->op = OP_MOV;
> +      i->setSrc(s, NULL);
> +      if (t) {
> +         i->setSrc(0, i->getSrc(t));
> +         i->setSrc(t, NULL);
> +      }
> +   }
> +      break;
> +
>      case OP_SHL:
>      {
>         if (s != 1 || i->src(0).mod != Modifier(0))
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
> index 178a167..70180eb 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
> @@ -413,6 +413,8 @@ TargetNV50::isOpSupported(operation op, DataType ty)
const
>         return false;
>      case OP_SAD:
>         return ty == TYPE_S32;
> +   case OP_SET:
> +      return !isFloatType(ty);
>      default:
>         return true;
>      }

On Sat, May 9, 2015 at 11:27 AM, Tobias Klausmann
<tobias.johannes.klausmann at mni.thm.de> wrote:
>
>
> On 09.05.2015 07:35, Ilia Mirkin wrote:
>>
>> This has started to happen more now that the backend is producing
>> KILL_IF more often.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>   .../drivers/nouveau/codegen/nv50_ir_peephole.cpp   | 29
>> ++++++++++++++++++++++
>>   .../nouveau/codegen/nv50_ir_target_nv50.cpp        |  2 ++
>>   2 files changed, 31 insertions(+)
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>> index 14446b6..d8af19a 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>> @@ -973,6 +973,35 @@ ConstantFolding::opnd(Instruction *i,
ImmediateValue
>> &imm0, int s)
>>      }
>>         break;
>>   +   case OP_AND:
>> +   {
>> +      CmpInstruction *cmp = i->getSrc(t)->getInsn()->asCmp();
>> +      if (!cmp || cmp->op == OP_SLCT)
>
>
> how about if (cmp == NULL || ...) and kill the same condition later?
I just killed the other one. I think the usual style tends to be if
(!ptr) rather than if (ptr == NULL) in codegen. Both are acceptable
though.
>
>
>> +         return;
>> +      if (!prog->getTarget()->isOpSupported(cmp->op,
TYPE_F32))
>> +         return;
>> +      if (imm0.reg.data.f32 != 1.0)
>> +         return;
>> +      if (cmp == NULL)
>> +         return;
>> +      if (i->getSrc(t)->getInsn()->dType != TYPE_U32)
>> +         return;
>> +
>> +      i->getSrc(t)->getInsn()->dType = TYPE_F32;
>> +      if (i->src(t).mod != Modifier(0)) {
>> +         assert(i->src(t).mod == Modifier(NV50_IR_MOD_NOT));
>> +         i->src(t).mod = Modifier(0);
>> +         cmp->setCond = reverseCondCode(cmp->setCond);
>> +      }
>> +      i->op = OP_MOV;
>> +      i->setSrc(s, NULL);
>> +      if (t) {
>> +         i->setSrc(0, i->getSrc(t));
>> +         i->setSrc(t, NULL);
>> +      }
>> +   }
>> +      break;
>> +
>>      case OP_SHL:
>>      {
>>         if (s != 1 || i->src(0).mod != Modifier(0))
>> diff --git
a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
>> index 178a167..70180eb 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
>> @@ -413,6 +413,8 @@ TargetNV50::isOpSupported(operation op, DataType
ty)
>> const
>>         return false;
>>      case OP_SAD:
>>         return ty == TYPE_S32;
>> +   case OP_SET:
>> +      return !isFloatType(ty);
>>      default:
>>         return true;
>>      }
>
>

On 09.05.2015 19:53, Ilia Mirkin wrote:
> On Sat, May 9, 2015 at 11:27 AM, Tobias Klausmann
> <tobias.johannes.klausmann at mni.thm.de> wrote:
>>
>> On 09.05.2015 07:35, Ilia Mirkin wrote:
>>> This has started to happen more now that the backend is producing
>>> KILL_IF more often.
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>    .../drivers/nouveau/codegen/nv50_ir_peephole.cpp   | 29
>>> ++++++++++++++++++++++
>>>    .../nouveau/codegen/nv50_ir_target_nv50.cpp        |  2 ++
>>>    2 files changed, 31 insertions(+)
>>>
>>> diff --git
a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>>> index 14446b6..d8af19a 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
>>> @@ -973,6 +973,35 @@ ConstantFolding::opnd(Instruction *i,
ImmediateValue
>>> &imm0, int s)
>>>       }
>>>          break;
>>>    +   case OP_AND:
>>> +   {
>>> +      CmpInstruction *cmp =
i->getSrc(t)->getInsn()->asCmp();
>>> +      if (!cmp || cmp->op == OP_SLCT)
>>
>> how about if (cmp == NULL || ...) and kill the same condition later?
> I just killed the other one. I think the usual style tends to be if
> (!ptr) rather than if (ptr == NULL) in codegen. Both are acceptable
> though.
it was mainly about the dead code you killed now. With this it looks 
fine to me, so feel free to add

Reviewed-by: Tobias Klausmann <tobias.johannes.klausmann at mni.thm.de>

>>
>>> +         return;
>>> +      if (!prog->getTarget()->isOpSupported(cmp->op,
TYPE_F32))
>>> +         return;
>>> +      if (imm0.reg.data.f32 != 1.0)
>>> +         return;
>>> +      if (cmp == NULL)
>>> +         return;
>>> +      if (i->getSrc(t)->getInsn()->dType != TYPE_U32)
>>> +         return;
>>> +
>>> +      i->getSrc(t)->getInsn()->dType = TYPE_F32;
>>> +      if (i->src(t).mod != Modifier(0)) {
>>> +         assert(i->src(t).mod == Modifier(NV50_IR_MOD_NOT));
>>> +         i->src(t).mod = Modifier(0);
>>> +         cmp->setCond = reverseCondCode(cmp->setCond);
>>> +      }
>>> +      i->op = OP_MOV;
>>> +      i->setSrc(s, NULL);
>>> +      if (t) {
>>> +         i->setSrc(0, i->getSrc(t));
>>> +         i->setSrc(t, NULL);
>>> +      }
>>> +   }
>>> +      break;
>>> +
>>>       case OP_SHL:
>>>       {
>>>          if (s != 1 || i->src(0).mod != Modifier(0))
>>> diff --git
a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
>>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
>>> index 178a167..70180eb 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_target_nv50.cpp
>>> @@ -413,6 +413,8 @@ TargetNV50::isOpSupported(operation op,
DataType ty)
>>> const
>>>          return false;
>>>       case OP_SAD:
>>>          return ty == TYPE_S32;
>>> +   case OP_SET:
>>> +      return !isFloatType(ty);
>>>       default:
>>>          return true;
>>>       }
>>