[Mesa-dev] [PATCH v2] nvc0/ir: return 0 in imageLoad on incomplete textures
Ilia Mirkin
imirkin at alum.mit.edu
Sat Jul 14 18:44:23 UTC 2018
On Fri, Jun 29, 2018 at 10:27 PM, Karol Herbst <kherbst at 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 <kherbst at 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.
> + }
> +}
> +
> 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list