From de139787332eb83381d7b5ed794d834f44f2ac45 Mon Sep 17 00:00:00 2001 From: Karol Herbst Date: Thu, 28 Jun 2018 18:55:00 +0200 Subject: [PATCH] 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 --- src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_peephole.cpp index e0faf8501bf..520cbee7c1a 100644 --- 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) -- 2.30.2