On Sat, Jul 14, 2018 at 8:44 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote: > On Fri, Jun 29, 2018 at 10:27 PM, Karol Herbst <kher...@redhat.com> wrote: >> We already guarded all OP_SULDP against out of bound accesses, but those >> ended up just reusing whatever value was stored in the dest registers. >> >> fixes CTS test shader_image_load_store.incomplete_textures >> >> v2: fix for loads not ending up with predicates (bindless_texture) >> >> Signed-off-by: Karol Herbst <kher...@redhat.com> >> --- >> .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 33 +++++++++++++++++-- >> .../nouveau/codegen/nv50_ir_lowering_nvc0.h | 1 + >> 2 files changed, 31 insertions(+), 3 deletions(-) >> >> 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 5723847234e..f55e9a34c59 100644 >> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp >> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp >> @@ -2180,13 +2180,36 @@ >> NVC0LoweringPass::convertSurfaceFormat(TexInstruction *su) >> } >> } >> >> +void >> +NVC0LoweringPass::insertOOBSurfaceOpResult(TexInstruction *su) >> +{ >> + if (!su->getPredicate()) >> + return; >> + >> + bld.setPosition(su, true); >> + >> + for (unsigned i = 0; su->defExists(i); ++i) { >> + ValueDef &def = su->def(i); >> + >> + 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), true); >> + uni->setSrc(0, def.get()); > > Pretty sure this doesn't work. You do def.replace(x, true) which will > set the def's value to x. And then you do def.get() which will just > return x -- not what you want. You're going to end up with a union > that has its def as an argument. > > It might all end up panning out sometimes, but check the IR that's > generated. Perhaps I'm misreading what def.replace does. >
right.. I have a local patch to fix this with a "def.replace(uni->getDef(0), false);". I simply forgot I had this fix locally. >> + } >> +} >> + >> void >> NVC0LoweringPass::handleSurfaceOpNVE4(TexInstruction *su) >> { >> processSurfaceCoordsNVE4(su); >> >> - if (su->op == OP_SULDP) >> + if (su->op == OP_SULDP) { >> convertSurfaceFormat(su); >> + insertOOBSurfaceOpResult(su); >> + } >> >> if (su->op == OP_SUREDB || su->op == OP_SUREDP) { >> assert(su->getPredicate()); >> @@ -2296,8 +2319,10 @@ NVC0LoweringPass::handleSurfaceOpNVC0(TexInstruction >> *su) >> >> processSurfaceCoordsNVC0(su); >> >> - if (su->op == OP_SULDP) >> + if (su->op == OP_SULDP) { >> convertSurfaceFormat(su); >> + insertOOBSurfaceOpResult(su); >> + } >> >> if (su->op == OP_SUREDB || su->op == OP_SUREDP) { >> const int dim = su->tex.target.getDim(); >> @@ -2397,8 +2422,10 @@ NVC0LoweringPass::handleSurfaceOpGM107(TexInstruction >> *su) >> { >> processSurfaceCoordsGM107(su); >> >> - if (su->op == OP_SULDP) >> + if (su->op == OP_SULDP) { >> convertSurfaceFormat(su); >> + insertOOBSurfaceOpResult(su); >> + } >> >> if (su->op == OP_SUREDP) { >> Value *def = su->getDef(0); >> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h >> b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h >> index 91771fbf7e9..d7350d03b78 100644 >> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h >> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.h >> @@ -143,6 +143,7 @@ private: >> void processSurfaceCoordsNVE4(TexInstruction *); >> void processSurfaceCoordsNVC0(TexInstruction *); >> void convertSurfaceFormat(TexInstruction *); >> + void insertOOBSurfaceOpResult(TexInstruction *); >> Value *calculateSampleOffset(Value *sampleID); >> >> protected: >> -- >> 2.17.1 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev