On Sat, Apr 30, 2016 at 12:35 PM, Samuel Pitoiset
<samuel.pitoi...@gmail.com> wrote:
> After the MemoryOpt pass, load and store instructions might have been
> combined and we have to take this in account when detecting if the
> destination register of a texture instruction is used by another
> instruction as srcs/defs.
>
> This fixes 11 dEQP-GLES31 subtests:
> dEQP-GLES3.functional.shaders.opaque_type_indexing.sampler.*.vertex.*
>
> There are still some samplers shadow fails with compute shaders but
> this is most likely an other issue because the barriers seem to be
> correctly added with this fix.
>
> This might also fix some rendering issues in real games.
>
> Fixes: 7752bbc ("gk104/ir: simplify and fool-proof texbar algorithm")
> Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com>
> Cc: "11.1 11.2" <mesa-sta...@lists.freedesktop.org>
> ---
> src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 11
> +++++++++--
> 1 file changed, 9 insertions(+), 2 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 3bce962..15864c8 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
> @@ -226,8 +226,11 @@ NVC0LegalizePostRA::findFirstUsesBB(
> continue;
>
> for (int d = 0; insn->defExists(d); ++d) {
> + int numGPR = insn->def(d).rep()->reg.size / 4;
> +
> if (insn->def(d).getFile() != FILE_GPR ||
> - insn->def(d).rep()->reg.data.id < minGPR ||
> + (insn->def(d).rep()->reg.data.id < minGPR &&
> + insn->def(d).rep()->reg.data.id + numGPR - 1 < minGPR) ||
Let x = insn->def(d).rep()->reg.data.id. x is guaranteed to be
non-negative, as is numGPR. How could the first condition matter
there?
> insn->def(d).rep()->reg.data.id > maxGPR)
> continue;
> addTexUse(uses, insn, texi);
> @@ -235,8 +238,12 @@ NVC0LegalizePostRA::findFirstUsesBB(
> }
>
> for (int s = 0; insn->srcExists(s); ++s) {
> + int numGPR = insn->src(s).rep()->reg.size / 4;
> +
> if (insn->src(s).getFile() != FILE_GPR ||
> - insn->src(s).rep()->reg.data.id < minGPR ||
> + (insn->src(s).rep()->reg.data.id < minGPR &&
> + insn->src(s).rep()->reg.data.id +
> + insn->src(s).rep()->reg.data.id + numGPR - 1 < minGPR) ||
That seems quite clearly wrong... you're doubling the reg id?
> insn->src(s).rep()->reg.data.id > maxGPR)
> continue;
> addTexUse(uses, insn, texi);
What you're trying to do makes sense. However it appears that you got
the details a little wrong. Basically my original code was just
looking at the "base" register. But you're correctly adding a second
set of conditions for the "top" register. So instead of
if (x >= min || x <= max) { do stuff }
you need to do
if (x + n >= min || x <= max) { do stuff }
Note that the actual code is inverted - it has a condition to skip.
But you get the idea.
Nice find!
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
