[Mesa-dev] [PATCH 1/2] ac/nir: Fix nir_texop_lod on GFX for 1D arrays.

[Bas Nieuwenhuizen]

Fixes: 1bcb953e166 'radv: handle GFX9 1D textures'
---
src/amd/common/ac_nir_to_llvm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 83b49b535c6..473dd67355b 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -4747,7 +4747,9 @@ static void visit_tex(struct ac_nir_context *ctx, nir_tex_instr *instr)
filler = LLVMConstReal(ctx->ac.f32, 0.5);

if (instr->sampler_dim == GLSL_SAMPLER_DIM_1D) {
- if (instr->is_array) {
+ /* No nir_texop_lod, because it does not take a slice
+ * even with array textures. */
+ if (instr->is_array && instr->op != nir_texop_lod ) {
address[count] = address[count - 1];
address[count - 1] = filler;
count++;

[Bas Nieuwenhuizen]

Since it also uses the output vector before writing to memory.

Fixes: e38685cc62e 'Revert "radv: disable support for VEGA for now."'
---
src/amd/vulkan/radv_shader.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 07e68d6032b..6176a2e590d 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -265,9 +265,7 @@ radv_shader_compile_to_nir(struct radv_device *device,
indirect_mask |= nir_var_shader_in;
}
if (!llvm_has_working_vgpr_indexing &&
- (nir->info.stage == MESA_SHADER_VERTEX ||
- nir->info.stage == MESA_SHADER_TESS_EVAL ||
- nir->info.stage == MESA_SHADER_FRAGMENT))
+ nir->info.stage != MESA_SHADER_TESS_CTRL)
indirect_mask |= nir_var_shader_out;

/* TODO: We shouldn't need to do this, however LLVM isn't currently

[Timothy Arceri]

Post by Bas Nieuwenhuizen
Since it also uses the output vector before writing to memory.
Fixes: e38685cc62e 'Revert "radv: disable support for VEGA for now."'
---
src/amd/vulkan/radv_shader.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 07e68d6032b..6176a2e590d 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -265,9 +265,7 @@ radv_shader_compile_to_nir(struct radv_device *device,
indirect_mask |= nir_var_shader_in;
}
if (!llvm_has_working_vgpr_indexing &&
- (nir->info.stage == MESA_SHADER_VERTEX ||
- nir->info.stage == MESA_SHADER_TESS_EVAL ||
- nir->info.stage == MESA_SHADER_FRAGMENT))
+ nir->info.stage != MESA_SHADER_TESS_CTRL)
indirect_mask |= nir_var_shader_out;
/* TODO: We shouldn't need to do this, however LLVM isn't currently

[Andres Gomez]

Bas, this patch looks like it should have been marked as fixing
6ce550453f1 instead of e38685cc62e (?).

In any case, I was wondering whether it would be interesting to bring
them both to the 17.2 stable queue and whether we would also want
Timothy's preceding patch:

087e010b2b3dd83a539f97203909d6c43b5da87c radv: copy indirect lowering settings from radeonsi

Let me know what you think.

Post by Bas Nieuwenhuizen
Since it also uses the output vector before writing to memory.
Fixes: e38685cc62e 'Revert "radv: disable support for VEGA for now."'
---
src/amd/vulkan/radv_shader.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 07e68d6032b..6176a2e590d 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -265,9 +265,7 @@ radv_shader_compile_to_nir(struct radv_device *device,
indirect_mask |= nir_var_shader_in;
}
if (!llvm_has_working_vgpr_indexing &&
- (nir->info.stage == MESA_SHADER_VERTEX ||
- nir->info.stage == MESA_SHADER_TESS_EVAL ||
- nir->info.stage == MESA_SHADER_FRAGMENT))
+ nir->info.stage != MESA_SHADER_TESS_CTRL)
indirect_mask |= nir_var_shader_out;
/* TODO: We shouldn't need to do this, however LLVM isn't currently

[Bas Nieuwenhuizen]

Post by Andres Gomez
Bas, this patch looks like it should have been marked as fixing
6ce550453f1 instead of e38685cc62e (?).

Well my reasoning was that the bug got "visible" when we enabled Vega.
The patch you refer to fixed part of it, but not all of it, and then
this patch fixes the rest.

Post by Andres Gomez
In any case, I was wondering whether it would be interesting to bring
them both to the 17.2 stable queue and whether we would also want
087e010b2b3dd83a539f97203909d6c43b5da87c radv: copy indirect lowering settings from radeonsi

Yes, that would be reasonable. Timothy's patch is an optimization
though, so I'd be happy to send a backport that only generates the
variable needed for the other two if you'd prefer that.

Post by Andres Gomez
Let me know what you think.

Post by Bas Nieuwenhuizen
Since it also uses the output vector before writing to memory.
Fixes: e38685cc62e 'Revert "radv: disable support for VEGA for now."'
---
src/amd/vulkan/radv_shader.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index 07e68d6032b..6176a2e590d 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -265,9 +265,7 @@ radv_shader_compile_to_nir(struct radv_device *device,
indirect_mask |= nir_var_shader_in;
}
if (!llvm_has_working_vgpr_indexing &&
- (nir->info.stage == MESA_SHADER_VERTEX ||
- nir->info.stage == MESA_SHADER_TESS_EVAL ||
- nir->info.stage == MESA_SHADER_FRAGMENT))
+ nir->info.stage != MESA_SHADER_TESS_CTRL)
indirect_mask |= nir_var_shader_out;
/* TODO: We shouldn't need to do this, however LLVM isn't currently

--
Br,
Andres

[Andres Gomez]

[...]

Post by Bas Nieuwenhuizen

Post by Andres Gomez
In any case, I was wondering whether it would be interesting to bring
them both to the 17.2 stable queue and whether we would also want
087e010b2b3dd83a539f97203909d6c43b5da87c radv: copy indirect lowering settings from radeonsi

Yes, that would be reasonable. Timothy's patch is an optimization
though, so I'd be happy to send a backport that only generates the
variable needed for the other two if you'd prefer that.

That would be great.

Thanks, Bas! ☺

[Andres Gomez]

Bas, how is the backport for this patch going? The pre-release announce
is planned for this Wednesday so it'd great to have it latest by
tomorrow.

Thanks!

Post by Andres Gomez
[...]

Post by Bas Nieuwenhuizen

Post by Andres Gomez
In any case, I was wondering whether it would be interesting to bring
them both to the 17.2 stable queue and whether we would also want
087e010b2b3dd83a539f97203909d6c43b5da87c radv: copy indirect lowering settings from radeonsi

Yes, that would be reasonable. Timothy's patch is an optimization
though, so I'd be happy to send a backport that only generates the
variable needed for the other two if you'd prefer that.

That would be great.
Thanks, Bas! ☺

[Bas Nieuwenhuizen]

The backports and cherry-picks are available at
https://patchwork.freedesktop.org/series/33323/ .

Post by Andres Gomez
Bas, how is the backport for this patch going? The pre-release announce
is planned for this Wednesday so it'd great to have it latest by
tomorrow.
Thanks!

Post by Andres Gomez
[...]

Post by Bas Nieuwenhuizen

Post by Andres Gomez
In any case, I was wondering whether it would be interesting to bring
them both to the 17.2 stable queue and whether we would also want
087e010b2b3dd83a539f97203909d6c43b5da87c radv: copy indirect lowering settings from radeonsi

Yes, that would be reasonable. Timothy's patch is an optimization
though, so I'd be happy to send a backport that only generates the
variable needed for the other two if you'd prefer that.

That would be great.
Thanks, Bas! ☺

--
Br,
Andres

[Andres Gomez]

Queued.

Thanks a lot, Bas. Very much appreciated ☺

Post by Bas Nieuwenhuizen
The backports and cherry-picks are available at
https://patchwork.freedesktop.org/series/33323/ .

Post by Andres Gomez
Bas, how is the backport for this patch going? The pre-release announce
is planned for this Wednesday so it'd great to have it latest by
tomorrow.
Thanks!

Post by Andres Gomez
[...]

Post by Bas Nieuwenhuizen

Post by Andres Gomez
In any case, I was wondering whether it would be interesting to bring
them both to the 17.2 stable queue and whether we would also want
087e010b2b3dd83a539f97203909d6c43b5da87c radv: copy indirect lowering settings from radeonsi

Yes, that would be reasonable. Timothy's patch is an optimization
though, so I'd be happy to send a backport that only generates the
variable needed for the other two if you'd prefer that.

That would be great.
Thanks, Bas! ☺

--
Br,
Andres

_______________________________________________
mesa-stable mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-stable