[Mesa-dev] [PATCH v3 12/14] nv50/ir: make OP_SELP a compare instruction

[Ilia Mirkin]

Sooo.... I know I'm the one who suggested this change in the first
place. But having slept on it, I think this is wrong. The point of a
CmpInstruction is when you're comparing two things and you need a
comparison operator and stuff like that. This is a much simpler
situation -- you can only have a NOT. Or not have a NOT :) This is
already well-expressed using a modifier. Furthermore I think there are
opt passes which might do clever things with Cmp instructions... not
sure.

So what I propose is to:
(a) flip it back to a regular instruction
(b) remove the i->setCond bit of the emission condition, only use the
modifier [the original problem was it was using i->cc which is also
wrong]
(c) have the creator of the operation set the NOT modifier as necessary.

[And again, this is totally my bad... I told you to do this, and then
gave you a R-b on it...]

  -ilia

On Wed, Feb 17, 2016 at 4:27 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
> This OP_SELP insn will be used to handle compare and swap subops.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp | 8 ++++----
>  src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h     | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
> index a7c49a2..6566d24 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
> @@ -120,7 +120,7 @@ private:
>
>     void emitSET(const CmpInstruction *);
>     void emitSLCT(const CmpInstruction *);
> -   void emitSELP(const Instruction *);
> +   void emitSELP(const CmpInstruction *);
>
>     void emitTEXBAR(const Instruction *);
>     void emitTEX(const TexInstruction *);
> @@ -1170,11 +1170,11 @@ CodeEmitterNVC0::emitSLCT(const CmpInstruction *i)
>        code[0] |= 1 << 5;
>  }
>
> -void CodeEmitterNVC0::emitSELP(const Instruction *i)
> +void CodeEmitterNVC0::emitSELP(const CmpInstruction *i)
>  {
>     emitForm_A(i, HEX64(20000000, 00000004));
>
> -   if (i->cc == CC_NOT_P || i->src(2).mod & Modifier(NV50_IR_MOD_NOT))
> +   if (i->setCond == CC_NOT_P || i->src(2).mod & Modifier(NV50_IR_MOD_NOT))
>        code[1] |= 1 << 20;
>  }
>
> @@ -2433,7 +2433,7 @@ CodeEmitterNVC0::emitInstruction(Instruction *insn)
>        emitSET(insn->asCmp());
>        break;
>     case OP_SELP:
> -      emitSELP(insn);
> +      emitSELP(insn->asCmp());
>        break;
>     case OP_SLCT:
>        emitSLCT(insn->asCmp());
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h b/src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h
> index e465f24..02e6157 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_inlines.h
> @@ -281,14 +281,14 @@ Value *TexInstruction::getIndirectS() const
>
>  CmpInstruction *Instruction::asCmp()
>  {
> -   if (op >= OP_SET_AND && op <= OP_SLCT && op != OP_SELP)
> +   if (op >= OP_SET_AND && op <= OP_SLCT)
>        return static_cast<CmpInstruction *>(this);
>     return NULL;
>  }
>
>  const CmpInstruction *Instruction::asCmp() const
>  {
> -   if (op >= OP_SET_AND && op <= OP_SLCT && op != OP_SELP)
> +   if (op >= OP_SET_AND && op <= OP_SLCT)
>        return static_cast<const CmpInstruction *>(this);
>     return NULL;
>  }
> --
> 2.6.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev