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

[Ilia Mirkin]

On Wed, May 21, 2014 at 3:03 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Wed, May 21, 2014 at 3:01 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> 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.
>
> OK -- if you'd like, you can send a patch to convert all of them over
> to mkMov. (Personally, I actually have a mild preference towards
> nuking it since it implies that mov is somehow special compared to
> other ops, and it isn't.)

Oh, and for situations like this, mkMov(a, b) would actually be wrong.
You'd need to use mkMov(a, b, TYPE_F32).

   Instruction *mkMov(Value *, Value *, DataType = TYPE_U32);

Fun, eh?