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

[Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 21/05/14 19:53, Ilia Mirkin wrote:
> On Wed, May 21, 2014 at 2:51 PM, Emil Velikov <emil.l.velikov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 21/05/14 00:39, Ilia Mirkin wrote:
>>> From: Christoph Bumiller <christoph.bumiller-D4IDLmZPRBc@public.gmane.org>
>>>
>>> Reviewed-by: Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
>>> Cc: "10.2" <mesa-stable-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.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
>