[Mesa-dev] [PATCH 2/3] nv50/ir: optimize slct(b, c, set(a, 0)) to slct(b, c, a)

[Karol Herbst]

 for
On Sat, Dec 15, 2018 at 7:58 PM Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>
> On Fri, Dec 14, 2018 at 6:12 PM Karol Herbst <kherbst at redhat.com> wrote:
> >
> > From: Karol Herbst <karolherbst at gmail.com>
> >
> > helps mainly feral ported games
> >
> > shader-db changes:
> > total instructions in shared programs     : 7614782 -> 7565661 (-0.65%)
> > total gprs used in shared programs        : 798045 -> 797213 (-0.10%)
> > total shared used in shared programs      : 639636 -> 639636 (0.00%)
> > total local used in shared programs       : 24648 -> 24648 (0.00%)
> > total bytes used in shared programs       : 81330696 -> 80806160 (-0.64%)
> >
> >                 local     shared        gpr       inst      bytes
> >     helped          0          0        701       7940       7940
> >       hurt          0          0         44          4          4
> >
> > Signed-off-by: Karol Herbst <kherbst at redhat.com>
> > ---
> >  .../nouveau/codegen/nv50_ir_peephole.cpp      | 47 +++++++++++++++++--
> >  1 file changed, 44 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> > index a8bd4f868bf..37e9edc49f4 100644
> > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> > @@ -1763,7 +1763,8 @@ private:
> >     bool tryADDToMADOrSAD(Instruction *, operation toOp);
> >     void handleMINMAX(Instruction *);
> >     void handleRCP(Instruction *);
> > -   void handleSLCT(Instruction *);
> > +   void handleSLCT(CmpInstruction *);
> > +   bool tryMergeSLCTSET(CmpInstruction *slct, CmpInstruction *set);
>
> SLCT_SET or SET_SLCT. Otherwise it all runs together.
>
> >     void handleLOGOP(Instruction *);
> >     void handleCVT_NEG(Instruction *);
> >     void handleCVT_CVT(Instruction *);
> > @@ -1956,8 +1957,12 @@ AlgebraicOpt::handleRCP(Instruction *rcp)
> >  }
> >
> >  void
> > -AlgebraicOpt::handleSLCT(Instruction *slct)
> > +AlgebraicOpt::handleSLCT(CmpInstruction *slct)
> >  {
> > +   Instruction *insn = slct->getSrc(2)->getInsn();
> > +   while(insn && insn->op == OP_SET && tryMergeSLCTSET(slct, insn->asCmp())) {
>
> while (
>
> > +      insn = slct->getSrc(2)->getInsn();
> > +   }
> >     if (slct->getSrc(2)->reg.file == FILE_IMMEDIATE) {
> >        if (slct->getSrc(2)->asImm()->compare(slct->asCmp()->setCond, 0.0f))
> >           slct->setSrc(0, slct->getSrc(1));
> > @@ -1970,6 +1975,42 @@ AlgebraicOpt::handleSLCT(Instruction *slct)
> >     slct->setSrc(2, NULL);
> >  }
> >
> > +bool
> > +AlgebraicOpt::tryMergeSLCTSET(CmpInstruction *slct, CmpInstruction *set)
> > +{
> > +   assert(slct->op == OP_SLCT && set->op == OP_SET);
> > +
> > +   if (typeSizeof(set->sType) != 4)
> > +      return false;
> > +
> > +   CondCode setCC = set->getCondition();
> > +   CondCode slctCC = slct->getCondition();
> > +   CondCode newCC = setCC;
> > +
> > +   if (slctCC != CC_NE && slctCC != CC_EQ)
> > +      return false;
> > +
> > +   ImmediateValue imm0;
> > +   int s;
> > +
> > +   if (set->src(0).getImmediate(imm0) && imm0.isInteger(0))
> > +      s = 1;
> > +   else if (set->src(1).getImmediate(imm0) && imm0.isInteger(0))
> > +      s = 0;
> > +   else
> > +      return false;
> > +
> > +   slct->setSrc(2, set->getSrc(s));
> > +   if (s)
> > +      newCC = reverseCondCode(newCC);
> > +   if (slctCC == CC_EQ)
> > +      newCC = inverseCondCode(newCC);
>
> While not wrong, this is all pretty confusing. I won't require it, but
> I'd like to encourage you to write a paragraph about the theory about
> wtf is going on here.

I think this isn't directly correct though. We can't just flip a CC_EQ
over to a CC_NE or vice versa for floats, it has to be CC_EQU <=>
CC_NE and CC_EQ <=> CC_NEU. Need to think about this a bit more
carefully. Which might also mean that whenever we use inverseCondCode
for fp operations could be incorrect.

>
> > +
> > +   slct->sType = set->sType;
> > +   slct->setCondition(newCC);
> > +   return true;
> > +}
> > +
> >  void
> >  AlgebraicOpt::handleLOGOP(Instruction *logop)
> >  {
> > @@ -2340,7 +2381,7 @@ AlgebraicOpt::visit(BasicBlock *bb)
> >           handleMINMAX(i);
> >           break;
> >        case OP_SLCT:
> > -         handleSLCT(i);
> > +         handleSLCT(i->asCmp());
> >           break;
> >        case OP_AND:
> >        case OP_OR:
> > --
> > 2.19.2
> >