Emil Velikov
2014-May-21 19:01 UTC
[Nouveau] [Mesa-dev] [PATCH 04/12] nv50/ir/tgsi: TGSI_OPCODE_POW replicates its result
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 >
Ilia Mirkin
2014-May-21 19:03 UTC
[Nouveau] [Mesa-dev] [PATCH 04/12] nv50/ir/tgsi: TGSI_OPCODE_POW replicates its result
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.)
Ilia Mirkin
2014-May-21 19:04 UTC
[Nouveau] [Mesa-dev] [PATCH 04/12] nv50/ir/tgsi: TGSI_OPCODE_POW replicates its result
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?
Reasonably Related Threads
- [Mesa-dev] [PATCH 04/12] nv50/ir/tgsi: TGSI_OPCODE_POW replicates its result
- [Mesa-dev] [PATCH 04/12] nv50/ir/tgsi: TGSI_OPCODE_POW replicates its result
- [Mesa-dev] [PATCH 04/12] nv50/ir/tgsi: TGSI_OPCODE_POW replicates its result
- [PATCH 04/12] nv50/ir/tgsi: TGSI_OPCODE_POW replicates its result
- [PATCH mesa 6/6] nouveau: codegen: Disable more old resource handling code