From af194bd38e1f75495f9251934340d3d3bcc80ee4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alejandro=20Pi=C3=B1eiro?= Date: Fri, 13 Jul 2018 12:39:41 +0200 Subject: [PATCH] nir/lower_samplers: don't assume a deref for both texture and sampler srcs 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) v3: clarify one comment (Jason) Reviewed-by: Jason Ekstrand --- 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..593967b7108 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 the source or removing + * instr if needed + */ + 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.30.2