Hi Jason, I'm CCing you for this one as you basically provided the
pseudocode I used as reference. So I assume that you already know what's
going on, hoping that would make the review easier.
On 09/08/18 15:43, Alejandro Piñeiro 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
> ---
> src/compiler/glsl/gl_nir_lower_samplers.c | 83
> ++++++++++++++++++++-----------
> 1 file changed, 55 insertions(+), 28 deletions(-)
>
> diff --git a/src/compiler/glsl/gl_nir_lower_samplers.c
> b/src/compiler/glsl/gl_nir_lower_samplers.c
> index 43fe318a835..1b50b10d345 100644
> --- a/src/compiler/glsl/gl_nir_lower_samplers.c
> +++ b/src/compiler/glsl/gl_nir_lower_samplers.c
> @@ -103,48 +103,75 @@ calc_sampler_offsets(nir_builder *b, nir_ssa_def *ptr,
> shader_program->data->UniformStorage[location].opaque[stage].index;
> }
>
> +static void
> +lower_tex_src_to_offset(nir_builder *b,
> + nir_tex_instr *instr, unsigned src_idx,
> + unsigned *index, unsigned *array_size,
> + const struct gl_shader_program *shader_program)
> +{
> + nir_ssa_def *indirect;
> + unsigned base_offset, array_elements;
> + nir_tex_src *src = &instr->src[src_idx];
> + bool is_sampler = src->src_type == nir_tex_src_sampler_deref;
> +
> + calc_sampler_offsets(b, src->src.ssa, shader_program, &base_offset,
> + &indirect, &array_elements);
> + if (indirect) {
> + nir_instr_rewrite_src(&instr->instr, &src->src,
> + nir_src_for_ssa(indirect));
> +
> + 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 (index)
> + *index = base_offset;
> +
> + if (array_size)
> + *array_size = array_elements;
> +}
> +
> static bool
> lower_sampler(nir_builder *b, nir_tex_instr *instr,
> const struct gl_shader_program *shader_program)
> {
> 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;
> + if (texture_idx >= 0) {
> + unsigned texture_index;
> + unsigned texture_array_size;
>
> - 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);
> + b->cursor = nir_before_instr(&instr->instr);
>
> - b->cursor = nir_before_instr(&instr->instr);
> + lower_tex_src_to_offset(b, instr, texture_idx,
> + &texture_index, &texture_array_size,
> + shader_program);
>
> - 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);
> + instr->texture_index = texture_index;
> + instr->texture_array_size = texture_array_size;
> + }
>
> - 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) {
> + unsigned sampler_index;
> +
> + lower_tex_src_to_offset(b, instr, sampler_idx,
> + &sampler_index, NULL,
> + shader_program);
> + instr->sampler_index = sampler_index;
> }
>
> + if (texture_idx < 0 && sampler_idx < 0)
> + return false;
> +
> return true;
> }
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
