[Mesa-dev] [PATCH v2 3/7] nv50/ir: optimize neg(and(set, 1)) to set

[Ilia Mirkin]

On Wed, Jan 27, 2016 at 12:25 PM, Karol Herbst <nouveau at karolherbst.de> wrote:
> From: Karol Herbst <git at karolherbst.de>
>
> helps shaders in saints row IV, bioshock infinite and shadow warrior
>
> total instructions in shared programs : 1921966 -> 1910935 (-0.57%)
> total gprs used in shared programs    : 251863 -> 251728 (-0.05%)
> total local used in shared programs   : 5673 -> 5673 (0.00%)
> total bytes used in shared programs   : 17622728 -> 17521824 (-0.57%)
>
>                 local        gpr       inst      bytes
>     helped           0         137         719         719
>       hurt           0          12           0           0
>
> v2: remove this opt for OP_SLCT and check against float for OP_SET
> Signed-off-by: Karol Herbst <nouveau at karolherbst.de>
> ---
>  .../drivers/nouveau/codegen/nv50_ir_peephole.cpp   | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> index 7b51ce0..eb43f6c 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> @@ -1510,6 +1510,7 @@ private:
>     void handleCVT_CVT(Instruction *);
>     void handleCVT_EXTBF(Instruction *);
>     void handleSUCLAMP(Instruction *);
> +   void handleNEG(Instruction *);
>
>     BuildUtil bld;
>  };
> @@ -1982,6 +1983,31 @@ AlgebraicOpt::handleSUCLAMP(Instruction *insn)
>     insn->setSrc(0, add->getSrc(s));
>  }
>
> +// NEG(AND(SET, 1)) -> SET
> +void
> +AlgebraicOpt::handleNEG(Instruction *i) {
> +   Instruction *src = i->getSrc(0)->getInsn();

if (isFloatType(i->sType))
  return;

NEG can be both for integers and floats. Need to make sure we're doing
integer negation here.

> +   if (src->op == OP_AND) {

It'd reduce nesting both here and below if instead you just bailed if
you saw something you didn't like. So here just do

if (src->op != OP_AND)
  return;

and similarly below.

> +      ImmediateValue imm;
> +      int b = 1;
> +      if (src->src(0).getImmediate(imm))
> +         b = 1;
> +      else if (src->src(1).getImmediate(imm))
> +         b = 0;
> +      else
> +         return;
> +
> +      if (imm.isInteger(1)) {
> +         Value *srcAnd = src->getSrc(b);
> +         Instruction *set = srcAnd->getInsn();
> +         if (set->op == OP_SET && !isFloatType(set->dType)) {

Please include the other SET variants here (SET_AND/SET_OR/SET_XOR).

> +            i->def(0).replace(srcAnd->getInsn()->getDef(0), false);

Uhhhh... shouldn't this be set->getDef(0)? Oh, it is. But it'd be
clearer if you actually wrote it that way :) And drop srcAnd entirely
-- IMHO it's a confusing name.

> +            return;
> +         }
> +      }
> +   }
> +}
> +
>  bool
>  AlgebraicOpt::visit(BasicBlock *bb)
>  {
> @@ -2019,6 +2045,9 @@ AlgebraicOpt::visit(BasicBlock *bb)
>        case OP_SUCLAMP:
>           handleSUCLAMP(i);
>           break;
> +      case OP_NEG:
> +         handleNEG(i);
> +         break;
>        default:
>           break;
>        }
> --
> 2.7.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev