From 007e596ac5d5fb43c1a08b8d7fe6125083fb156d Mon Sep 17 00:00:00 2001 From: Karol Herbst Date: Tue, 16 Jun 2020 20:17:55 +0200 Subject: [PATCH] nv50/ir: fix surface lowering when values get shared accross operations With nir I encountered the case where the same value can be written to from multiple surface operations. This caused some weird messups with the unions as the def.rewrite operations caused unrelated instructions to get new their value replaced as well. In order to replace def.rewrite, we have to create a new temp value, write to that one instead and move to the original value. Fixes: 869e32593a9 ("gm107/ir: fix loading z offset for layered 3d image bindings") Signed-off-by: Karol Herbst Reviewed-by: Ilia Mirkin Part-of: (cherry picked from commit c1f938b6475b1c936a78f6aacf76a1601c87a0bb) --- .pick_status.json | 2 +- .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 28 ++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 551ab9f0d33..74109e8001c 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1237,7 +1237,7 @@ "description": "nv50/ir: fix surface lowering when values get shared accross operations", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "869e32593a9096b845dd6106f8f86e1c41fac968" }, diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp index 7703df8f504..a41e989117f 100644 --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp @@ -2328,15 +2328,15 @@ NVC0LoweringPass::insertOOBSurfaceOpResult(TexInstruction *su) bld.setPosition(su, true); for (unsigned i = 0; su->defExists(i); ++i) { - ValueDef &def = su->def(i); + Value *def = su->getDef(i); + Value *newDef = bld.getSSA(); + su->setDef(i, newDef); Instruction *mov = bld.mkMov(bld.getSSA(), bld.loadImm(NULL, 0)); assert(su->cc == CC_NOT_P); mov->setPredicate(CC_P, su->getPredicate()); - Instruction *uni = bld.mkOp2(OP_UNION, TYPE_U32, bld.getSSA(), NULL, mov->getDef(0)); - - def.replace(uni->getDef(0), false); - uni->setSrc(0, def.get()); + Instruction *uni = bld.mkOp2(OP_UNION, TYPE_U32, bld.getSSA(), newDef, mov->getDef(0)); + bld.mkMov(def, uni->getDef(0)); } } @@ -2613,10 +2613,12 @@ NVC0LoweringPass::processSurfaceCoordsGM107(TexInstruction *su, Instruction *ret for (unsigned i = 0; su->defExists(i); ++i) { assert(i < 4); - ValueDef &def = su->def(i); + Value *def = su->getDef(i); + Value *newDef = bld.getSSA(); ValueDef &def2 = su2d->def(i); Instruction *mov = NULL; + su->setDef(i, newDef); if (pred) { mov = bld.mkMov(bld.getSSA(), bld.loadImm(NULL, 0)); mov->setPredicate(CC_P, pred->getDef(0)); @@ -2624,11 +2626,10 @@ NVC0LoweringPass::processSurfaceCoordsGM107(TexInstruction *su, Instruction *ret Instruction *uni = ret[i] = bld.mkOp2(OP_UNION, TYPE_U32, bld.getSSA(), - NULL, def2.get()); - def.replace(uni->getDef(0), false); - uni->setSrc(0, def.get()); + newDef, def2.get()); if (mov) uni->setSrc(2, mov->getDef(0)); + bld.mkMov(def, uni->getDef(0)); } } else if (pred) { // Create a UNION so that RA assigns the same registers @@ -2636,16 +2637,17 @@ NVC0LoweringPass::processSurfaceCoordsGM107(TexInstruction *su, Instruction *ret for (unsigned i = 0; su->defExists(i); ++i) { assert(i < 4); - ValueDef &def = su->def(i); + Value *def = su->getDef(i); + Value *newDef = bld.getSSA(); + su->setDef(i, newDef); Instruction *mov = bld.mkMov(bld.getSSA(), bld.loadImm(NULL, 0)); mov->setPredicate(CC_P, pred->getDef(0)); Instruction *uni = ret[i] = bld.mkOp2(OP_UNION, TYPE_U32, bld.getSSA(), - NULL, mov->getDef(0)); - def.replace(uni->getDef(0), false); - uni->setSrc(0, def.get()); + newDef, mov->getDef(0)); + bld.mkMov(def, uni->getDef(0)); } }