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

[Ilia Mirkin]

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