Nouveau - May 2014 - [Mesa-dev] [PATCH 04/12] nv50/ir/tgsi: TGSI_OPCODE_POW replicates its result

On 21/05/14 00:39, Ilia Mirkin wrote:
> From: Christoph Bumiller <christoph.bumiller at speed.at>
> 
> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: "10.2" <mesa-stable at lists.freedesktop.org>
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> index 3e44bf9..e24be51 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -2199,7 +2199,6 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>     case TGSI_OPCODE_IMUL_HI:
>     case TGSI_OPCODE_UMUL_HI:
>     case TGSI_OPCODE_OR:
> -   case TGSI_OPCODE_POW:
>     case TGSI_OPCODE_SHL:
>     case TGSI_OPCODE_ISHR:
>     case TGSI_OPCODE_USHR:
> @@ -2254,6 +2253,11 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>        FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
>           mkOp1(OP_MOV, TYPE_U32, dst0[c], fetchSrc(0, c));
>        break;
> +   case TGSI_OPCODE_POW:
> +      val0 = mkOp2v(op, TYPE_F32, getScratch(), fetchSrc(0, 0),
fetchSrc(1, 0));
> +      FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
> +         mkOp1(OP_MOV, TYPE_F32, dst0[c], val0);
> +      break;
Can you use mkMov ? Pretty please :)

-Emil
>     case TGSI_OPCODE_EX2:
>     case TGSI_OPCODE_LG2:
>        val0 = mkOp1(op, TYPE_F32, getScratch(), fetchSrc(0,
0))->getDef(0);
>

On Wed, May 21, 2014 at 2:51 PM, Emil Velikov <emil.l.velikov at
gmail.com> wrote:
> On 21/05/14 00:39, Ilia Mirkin wrote:
>> From: Christoph Bumiller <christoph.bumiller at speed.at>
>>
>> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> Cc: "10.2" <mesa-stable at lists.freedesktop.org>
>> ---
>>  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> index 3e44bf9..e24be51 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> @@ -2199,7 +2199,6 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>>     case TGSI_OPCODE_IMUL_HI:
>>     case TGSI_OPCODE_UMUL_HI:
>>     case TGSI_OPCODE_OR:
>> -   case TGSI_OPCODE_POW:
>>     case TGSI_OPCODE_SHL:
>>     case TGSI_OPCODE_ISHR:
>>     case TGSI_OPCODE_USHR:
>> @@ -2254,6 +2253,11 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>>        FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
>>           mkOp1(OP_MOV, TYPE_U32, dst0[c], fetchSrc(0, c));
>>        break;
>> +   case TGSI_OPCODE_POW:
>> +      val0 = mkOp2v(op, TYPE_F32, getScratch(), fetchSrc(0, 0),
fetchSrc(1, 0));
>> +      FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
>> +         mkOp1(OP_MOV, TYPE_F32, dst0[c], val0);
>> +      break;
> Can you use mkMov ? Pretty please :)
The two are used fairly interchangeably, and a lot of the surrounding
code uses this style (as you can see in the context lines). I'd rather
keep it as-is.

  -ilia

On 21/05/14 19:53, Ilia Mirkin wrote:
> On Wed, May 21, 2014 at 2:51 PM, Emil Velikov <emil.l.velikov at
gmail.com> wrote:
>> On 21/05/14 00:39, Ilia Mirkin wrote:
>>> From: Christoph Bumiller <christoph.bumiller at speed.at>
>>>
>>> Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> Cc: "10.2" <mesa-stable at lists.freedesktop.org>
>>> ---
>>>  src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 6
+++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git
a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>> index 3e44bf9..e24be51 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>>> @@ -2199,7 +2199,6 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>>>     case TGSI_OPCODE_IMUL_HI:
>>>     case TGSI_OPCODE_UMUL_HI:
>>>     case TGSI_OPCODE_OR:
>>> -   case TGSI_OPCODE_POW:
>>>     case TGSI_OPCODE_SHL:
>>>     case TGSI_OPCODE_ISHR:
>>>     case TGSI_OPCODE_USHR:
>>> @@ -2254,6 +2253,11 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
>>>        FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
>>>           mkOp1(OP_MOV, TYPE_U32, dst0[c], fetchSrc(0, c));
>>>        break;
>>> +   case TGSI_OPCODE_POW:
>>> +      val0 = mkOp2v(op, TYPE_F32, getScratch(), fetchSrc(0, 0),
fetchSrc(1, 0));
>>> +      FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
>>> +         mkOp1(OP_MOV, TYPE_F32, dst0[c], val0);
>>> +      break;
>> Can you use mkMov ? Pretty please :)
> 
> The two are used fairly interchangeably, and a lot of the surrounding
> code uses this style (as you can see in the context lines). I'd rather
> keep it as-is.
> 
IMHO using mkMov makes the code easier to read for noobs like me. There are
only two cases of mkOp1(OP_MOV) and more than a dozen mkMov's. Either way
it's
up-to you.

-Emil
>   -ilia
>