[Mesa-dev] [PATCH v2 4/6] nv50/ir: restructure postraconstantfolding pass

[Samuel Pitoiset]

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

One minor comment below.

On 10/09/2016 11:04 AM, Karol Herbst wrote:
> we might want to add more folding passes here, so make it a bit more generic
>
> v2: leave the comment and reword commit message
>
> Signed-off-by: Karol Herbst <karolherbst at gmail.com>
> ---
>  .../drivers/nouveau/codegen/nv50_ir_peephole.cpp   | 120 +++++++++++----------
>  1 file changed, 62 insertions(+), 58 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> index fd1fec6..1f47ba2 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
> @@ -2956,75 +2956,79 @@ FlatteningPass::tryPredicateConditional(BasicBlock *bb)
>  // constraint SDST == SSRC2
>  // TODO:
>  // Does NVC0+ have other situations where this pass makes sense?
> -class NV50PostRaConstantFolding : public Pass
> +class PostRaConstantFolding : public Pass
>  {
>  private:
> -   virtual bool visit(BasicBlock *);
> +   virtual bool visit(Instruction *);

Usually we add a blank like between visit() and handleXXX().

> +   void handleMAD(Instruction *);
>  };
>
> -bool
> -NV50PostRaConstantFolding::visit(BasicBlock *bb)
> +// Fold Immediate into MAD; must be done after register allocation due to
> +// constraint SDST == SSRC2
> +void
> +PostRaConstantFolding::handleMAD(Instruction *i)
>  {
> -   Value *vtmp;
> -   Instruction *def;
> -
> -   for (Instruction *i = bb->getFirst(); i; i = i->next) {
> -      switch (i->op) {
> -      case OP_MAD:
> -         if (i->def(0).getFile() != FILE_GPR ||
> -             i->src(0).getFile() != FILE_GPR ||
> -             i->src(1).getFile() != FILE_GPR ||
> -             i->src(2).getFile() != FILE_GPR ||
> -             i->getDef(0)->reg.data.id != i->getSrc(2)->reg.data.id)
> -            break;
> -
> -         if (i->getDef(0)->reg.data.id >= 64 ||
> -             i->getSrc(0)->reg.data.id >= 64)
> -            break;
> +   if (i->def(0).getFile() != FILE_GPR ||
> +       i->src(0).getFile() != FILE_GPR ||
> +       i->src(1).getFile() != FILE_GPR ||
> +       i->src(2).getFile() != FILE_GPR ||
> +       i->getDef(0)->reg.data.id != i->getSrc(2)->reg.data.id)
> +      return;
>
> -         if (i->flagsSrc >= 0 && i->getSrc(i->flagsSrc)->reg.data.id != 0)
> -            break;
> +   if (i->getDef(0)->reg.data.id >= 64 ||
> +       i->getSrc(0)->reg.data.id >= 64)
> +      return;
>
> -         if (i->getPredicate())
> -            break;
> +   if (i->flagsSrc >= 0 && i->getSrc(i->flagsSrc)->reg.data.id != 0)
> +      return;
>
> -         def = i->getSrc(1)->getInsn();
> -         if (def && def->op == OP_SPLIT && typeSizeof(def->sType) == 4)
> -            def = def->getSrc(0)->getInsn();
> -         if (def && def->op == OP_MOV && def->src(0).getFile() == FILE_IMMEDIATE) {
> -            vtmp = i->getSrc(1);
> -            if (isFloatType(i->sType)) {
> -               i->setSrc(1, def->getSrc(0));
> -            } else {
> -               ImmediateValue val;
> -               bool ret = def->src(0).getImmediate(val);
> -               assert(ret);
> -               if (i->getSrc(1)->reg.data.id & 1)
> -                  val.reg.data.u32 >>= 16;
> -               val.reg.data.u32 &= 0xffff;
> -               i->setSrc(1, new_ImmediateValue(bb->getProgram(), val.reg.data.u32));
> -            }
> +   if (i->getPredicate())
> +      return;
>
> -            /* There's no post-RA dead code elimination, so do it here
> -             * XXX: if we add more code-removing post-RA passes, we might
> -             *      want to create a post-RA dead-code elim pass */
> -            if (vtmp->getInsn()->isDead(true)) {
> -               Value *src = vtmp->getInsn()->getSrc(0);
> -               // Careful -- splits will have already been removed from the
> -               // functions. Don't double-delete.
> -               if (vtmp->getInsn()->bb)
> -                  delete_Instruction(prog, vtmp->getInsn());
> -               if (src->getInsn() && src->getInsn()->isDead(true))
> -                  delete_Instruction(prog, src->getInsn());
> -            }
> +   Value *vtmp;
> +   Instruction *def = i->getSrc(1)->getInsn();
> +
> +   if (def && def->op == OP_SPLIT && typeSizeof(def->sType) == 4)
> +      def = def->getSrc(0)->getInsn();
> +   if (def && def->op == OP_MOV && def->src(0).getFile() == FILE_IMMEDIATE) {
> +      vtmp = i->getSrc(1);
> +      if (isFloatType(i->sType)) {
> +         i->setSrc(1, def->getSrc(0));
> +      } else {
> +         ImmediateValue val;
> +         bool ret = def->src(0).getImmediate(val);
> +         assert(ret);
> +         if (i->getSrc(1)->reg.data.id & 1)
> +            val.reg.data.u32 >>= 16;
> +         val.reg.data.u32 &= 0xffff;
> +         i->setSrc(1, new_ImmediateValue(prog, val.reg.data.u32));
> +      }
>
> -            break;
> -         }
> -         break;
> -      default:
> -         break;
> +      /* There's no post-RA dead code elimination, so do it here
> +       * XXX: if we add more code-removing post-RA passes, we might
> +       *      want to create a post-RA dead-code elim pass */
> +      if (vtmp->getInsn()->isDead(true)) {
> +         Value *src = vtmp->getInsn()->getSrc(0);
> +         // Careful -- splits will have already been removed from the
> +         // functions. Don't double-delete.
> +         if (vtmp->getInsn()->bb)
> +            delete_Instruction(prog, vtmp->getInsn());
> +         if (src->getInsn() && src->getInsn()->isDead(true))
> +            delete_Instruction(prog, src->getInsn());
>        }
>     }
> +}
> +
> +bool
> +PostRaConstantFolding::visit(Instruction *i)
> +{
> +   switch (i->op) {
> +   case OP_MAD:
> +      handleMAD(i);
> +      break;
> +   default:
> +      break;
> +   }
>
>     return true;
>  }
> @@ -3444,7 +3448,7 @@ Program::optimizePostRA(int level)
>  {
>     RUN_PASS(2, FlatteningPass, run);
>     if (getTarget()->getChipset() < 0xc0)
> -      RUN_PASS(2, NV50PostRaConstantFolding, run);
> +      RUN_PASS(2, PostRaConstantFolding, run);
>
>     return true;
>  }
>