nv50/ir: fix Instruction::isActionEqual for PHI instructions

phi instructions don't have the same results by simply having the same sources.
They need to be inside the same BasicBlock or share an equal condition
resulting into a path through the shader selecting equal sources as well.

short example:

cond = ...;
const0 = 0;
const1 = 1;

if (cond) {
  ssa_1 = const0;
} else {
  ssa_2 = const1;
}
ssa_3 = phi ssa_1 ssa_2;

if (!cond) {
  ssa_4 = const0;
} else {
  ssa_5 = const1;
}
ssa_6 = phi ssa_4 ssa_5;

allthough both phis actually have sources with equal results, merging them
would be wrong due to having a different condition selecting which source to
take.

For now we also stick an assert into GlobalCSE, because it should never end up
having to merge phi instructions.

Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
index e0faf8501bf90237899b02351fdb200fe4d2d60a..520cbee7c1adce6f6845baf6f26620b477d397a3 100644 (file)
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp
@@ -3472,6 +3472,11 @@ Instruction::isActionEqual(const Instruction *that) const
    } else
    if (this->asFlow()) {
       return false;
+   } else
+   if (this->op == OP_PHI && this->bb != that->bb) {
+      /* TODO: we could probably be a bit smarter here by following the
+       * control flow, but honestly, it is quite painful to check */
+      return false;
    } else {
       if (this->ipa != that->ipa ||
           this->lanes != that->lanes ||
@@ -3568,6 +3573,7 @@ GlobalCSE::visit(BasicBlock *bb)
             break;
       }
       if (!phi->srcExists(s)) {
+         assert(ik->op != OP_PHI);
          Instruction *entry = bb->getEntry();
          ik->bb->remove(ik);
          if (!entry || entry->op != OP_JOIN)