On August 11, 2018 06:13:17 Alejandro Piñeiro <apinhe...@igalia.com> wrote:
After commit "nir: Use derefs in nir_lower_samplers"
(75286c2d083cdbdfb202a93349e567df0441d5f7) assumes one deref for both
the texture and the sampler. However there are cases (on OpenGL, using
ARB_gl_spirv) where SPIR-V is not providing a sampler, like for
texture query levels ops. Although we could make spirv_to_nir to
provide a sampler deref for those cases, it is not really needed, and
wrong from the Vulkan point of view.
This patch fixes the following (borrowed) tests run on SPIR-V mode:
arb_compute_shader/execution/basic-texelFetch.shader_test
arb_gpu_shader5/execution/sampler_array_indexing/fs-simple-texture-size.shader_test
arb_texture_query_levels/execution/fs-baselevel.shader_test
arb_texture_query_levels/execution/fs-maxlevel.shader_test
arb_texture_query_levels/execution/fs-miptree.shader_test
arb_texture_query_levels/execution/fs-nomips.shader_test
arb_texture_query_levels/execution/vs-baselevel.shader_test
arb_texture_query_levels/execution/vs-maxlevel.shader_test
arb_texture_query_levels/execution/vs-miptree.shader_test
arb_texture_query_levels/execution/vs-nomips.shader_test
glsl-1.30/execution/fs-textureSize-compare.shader_test
v2: merge lower_tex_src_to_offset and calc_sampler_offsets together,
update texture/sampler index and texture_array_size directly on
lower_tex_src_to_offset (Jason)
---
src/compiler/glsl/gl_nir_lower_samplers.c | 111 ++++++++++++++++--------------
1 file changed, 58 insertions(+), 53 deletions(-)
diff --git a/src/compiler/glsl/gl_nir_lower_samplers.c
b/src/compiler/glsl/gl_nir_lower_samplers.c
index 43fe318a835..e001792d689 100644
--- a/src/compiler/glsl/gl_nir_lower_samplers.c
+++ b/src/compiler/glsl/gl_nir_lower_samplers.c
@@ -31,21 +31,20 @@
#include "main/compiler.h"
#include "main/mtypes.h"
-/* Calculate the sampler index based on array indicies and also
- * calculate the base uniform location for struct members.
- */
static void
-calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
- const struct gl_shader_program *shader_program,
- unsigned *base_index, nir_ssa_def **index,
- unsigned *array_elements)
+lower_tex_src_to_offset(nir_builder *b,
+ nir_tex_instr *instr, unsigned src_idx,
+ const struct gl_shader_program *shader_program)
{
- *base_index = 0;
- *index = NULL;
- *array_elements = 1;
+ nir_ssa_def *index = NULL;
+ unsigned base_index = 0;
+ unsigned array_elements = 1;
+ nir_tex_src *src = &instr->src[src_idx];
+ bool is_sampler = src->src_type == nir_tex_src_sampler_deref;
unsigned location = 0;
- nir_deref_instr *deref = nir_instr_as_deref(ptr->parent_instr);
+ /* We compute first the offsets */
+ nir_deref_instr *deref = nir_instr_as_deref(src->src.ssa->parent_instr);
while (deref->deref_type != nir_deref_type_var) {
assert(deref->parent.is_ssa);
nir_deref_instr *parent =
@@ -61,22 +60,22 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
nir_const_value *const_deref_index =
nir_src_as_const_value(deref->arr.index);
- if (const_deref_index && *index == NULL) {
+ if (const_deref_index && index == NULL) {
/* We're still building a direct index */
- *base_index += const_deref_index->u32[0] * *array_elements;
+ base_index += const_deref_index->u32[0] * array_elements;
} else {
- if (*index == NULL) {
+ if (index == NULL) {
/* We used to be direct but not anymore */
- *index = nir_imm_int(b, *base_index);
- *base_index = 0;
+ index = nir_imm_int(b, base_index);
+ base_index = 0;
}
- *index = nir_iadd(b, *index,
- nir_imul(b, nir_imm_int(b, *array_elements),
- nir_ssa_for_src(b, deref->arr.index, 1)));
+ index = nir_iadd(b, index,
+ nir_imul(b, nir_imm_int(b, array_elements),
+ nir_ssa_for_src(b, deref->arr.index,
1)));
}
- *array_elements *= glsl_get_length(parent->type);
+ array_elements *= glsl_get_length(parent->type);
break;
}
@@ -87,8 +86,8 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
deref = parent;
}
- if (*index)
- *index = nir_umin(b, *index, nir_imm_int(b, *array_elements - 1));
+ if (index)
+ index = nir_umin(b, index, nir_imm_int(b, array_elements - 1));
/* We hit the deref_var. This is the end of the line */
assert(deref->deref_type == nir_deref_type_var);
@@ -99,8 +98,31 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
assert(location < shader_program->data->NumUniformStorage &&
shader_program->data->UniformStorage[location].opaque[stage].active);
- *base_index +=
+ base_index +=
shader_program->data->UniformStorage[location].opaque[stage].index;
+
+ /* We have the offsets, we apply them, rewriting/removing instr if
+ * needed
+ */
We're removing it rewriting the source, not the instruction. With that fixed,
Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net>
+ if (index) {
+ nir_instr_rewrite_src(&instr->instr, &src->src,
+ nir_src_for_ssa(index));
+
+ src->src_type = is_sampler ?
+ nir_tex_src_sampler_offset :
+ nir_tex_src_texture_offset;
+
+ instr->texture_array_size = array_elements;
+ } else {
+ nir_tex_instr_remove_src(instr, src_idx);
+ }
+
+ if (is_sampler) {
+ instr->sampler_index = base_index;
+ } else {
+ instr->texture_index = base_index;
+ instr->texture_array_size = array_elements;
+ }
}
static bool
@@ -109,42 +131,25 @@ lower_sampler(nir_builder *b, nir_tex_instr *instr,
{
int texture_idx =
nir_tex_instr_src_index(instr, nir_tex_src_texture_deref);
- int sampler_idx =
- nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
-
- if (texture_idx < 0)
- return false;
- assert(texture_idx >= 0 && sampler_idx >= 0);
- assert(instr->src[texture_idx].src.is_ssa);
- assert(instr->src[sampler_idx].src.is_ssa);
- assert(instr->src[texture_idx].src.ssa == instr->src[sampler_idx].src.ssa);
+ if (texture_idx >= 0) {
+ b->cursor = nir_before_instr(&instr->instr);
- b->cursor = nir_before_instr(&instr->instr);
-
- unsigned base_offset, array_elements;
- nir_ssa_def *indirect;
- calc_sampler_offsets(b, instr->src[texture_idx].src.ssa, shader_program,
- &base_offset, &indirect, &array_elements);
+ lower_tex_src_to_offset(b, instr, texture_idx,
+ shader_program);
+ }
- instr->texture_index = base_offset;
- instr->sampler_index = base_offset;
- if (indirect) {
- nir_instr_rewrite_src(&instr->instr, &instr->src[texture_idx].src,
- nir_src_for_ssa(indirect));
- instr->src[texture_idx].src_type = nir_tex_src_texture_offset;
- nir_instr_rewrite_src(&instr->instr, &instr->src[sampler_idx].src,
- nir_src_for_ssa(indirect));
- instr->src[sampler_idx].src_type = nir_tex_src_sampler_offset;
+ int sampler_idx =
+ nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
- instr->texture_array_size = array_elements;
- } else {
- nir_tex_instr_remove_src(instr, texture_idx);
- /* The sampler index may have changed */
- sampler_idx = nir_tex_instr_src_index(instr, nir_tex_src_sampler_deref);
- nir_tex_instr_remove_src(instr, sampler_idx);
+ if (sampler_idx >= 0) {
+ lower_tex_src_to_offset(b, instr, sampler_idx,
+ shader_program);
}
+ if (texture_idx < 0 && sampler_idx < 0)
+ return false;
+
return true;
}
--
2.14.1
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
