[Mesa-dev] [PATCH v2] spirv: Fix InterpolateAt* instructions for vecs with dynamic index

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

On Mon, Jul 9, 2018 at 5:21 AM Alejandro Piñeiro <apinheiro at igalia.com>
wrote:

> From: Neil Roberts <nroberts at igalia.com>
>
> If the glsl is something like this:
>
>   in vec4 some_input;
>   interpolateAtCentroid(some_input[idx])
>
> then it now gets generated as if it were:
>
>   interpolateAtCentroid(some_input)[idx]
>
> This is necessary because the index will get generated as a series of
> nir_bcsel instructions so it would no longer be an input variable. It
> is similar to what is done for GLSL in ca63a5ed3e9efb2bd645b42.
>
> Although I can’t find anything explicit in the Vulkan specs to say
> this should be allowed, the SPIR-V spec just says “the operand
> interpolant must be a pointer to the Input Storage Class”, which I
> guess doesn’t rule out any type of pointer to an input.
>
> This was found using the spec/glsl-4.40/execution/fs-interpolateAt*
> Piglit tests with the ARB_gl_spirv branch.
>
> Signed-off-by: Neil Roberts <nroberts at igalia.com>
> Signed-off-by: Alejandro Piñeiro <apinheiro at igalia.com>
>
> v2: update after nir_deref_instr land on master. Implemented by
>     Alejandro Piñeiro. Special thanks to Jason Ekstrand for guidance
>     at the new nir_deref_instr world.
> ---
>
> Although this was initially detected while testing the ARB_gl_spirv
> ongoing implementation, as v1, this can be tested on Vulkan using
> vkrunner:
>
> git clone https://github.com/Igalia/vkrunner.git -b tests
> cd vkrunner && ./autogen.sh && make
> ./src/vkrunner examples/interpolation/*.shader_test
>
>  src/compiler/spirv/vtn_glsl450.c | 38
> +++++++++++++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/src/compiler/spirv/vtn_glsl450.c
> b/src/compiler/spirv/vtn_glsl450.c
> index 7f941ceeb0d..8393450f2f4 100644
> --- a/src/compiler/spirv/vtn_glsl450.c
> +++ b/src/compiler/spirv/vtn_glsl450.c
> @@ -802,7 +802,22 @@ handle_glsl450_interpolation(struct vtn_builder *b,
> enum GLSLstd450 opcode,
>
>     struct vtn_pointer *ptr =
>        vtn_value(b, w[5], vtn_value_type_pointer)->pointer;
> -   intrin->src[0] = nir_src_for_ssa(&vtn_pointer_to_deref(b,
> ptr)->dest.ssa);
> +   nir_deref_instr *deref = vtn_pointer_to_deref(b, ptr);
> +
> +   /* If the value we are interpolating has an index into a vector then
> +    * interpolate the vector and index the result of that instead. This is
> +    * necessary because the index will get generated as a series of
> nir_bcsel
> +    * instructions so it would no longer be an input variable.
> +    */
> +   const bool vec_array_deref = deref->deref_type == nir_deref_type_array
> &&
> +      glsl_type_is_vector(nir_deref_instr_parent(deref)->type);
> +
> +   nir_deref_instr *vec_deref = NULL;
> +   if (vec_array_deref) {
> +      vec_deref = deref;
> +      deref = nir_deref_instr_parent(deref);
> +   }
> +   intrin->src[0] = nir_src_for_ssa(&deref->dest.ssa);
>
>     switch (opcode) {
>     case GLSLstd450InterpolateAtCentroid:
> @@ -815,13 +830,26 @@ handle_glsl450_interpolation(struct vtn_builder *b,
> enum GLSLstd450 opcode,
>        vtn_fail("Invalid opcode");
>     }
>
> -   intrin->num_components = glsl_get_vector_elements(dest_type);
> +   intrin->num_components = glsl_get_vector_elements(deref->type);
>     nir_ssa_dest_init(&intrin->instr, &intrin->dest,
> -                     glsl_get_vector_elements(dest_type),
> -                     glsl_get_bit_size(dest_type), NULL);
> -   val->ssa->def = &intrin->dest.ssa;
> +                     glsl_get_vector_elements(deref->type),
> +                     glsl_get_bit_size(deref->type), NULL);
>
>     nir_builder_instr_insert(&b->nb, &intrin->instr);
> +
> +   if (vec_array_deref) {
> +      assert(vec_deref);
> +      nir_const_value *const_index =
> nir_src_as_const_value(vec_deref->arr.index);
> +      if (const_index) {
> +         val->ssa->def = vtn_vector_extract(b, &intrin->dest.ssa,
> +                                            const_index->u32[0]);
> +      } else {
> +         val->ssa->def = vtn_vector_extract_dynamic(b, &intrin->dest.ssa,
> +
> vec_deref->arr.index.ssa);
> +      }
> +   } else {
> +      val->ssa->def = &intrin->dest.ssa;
> +   }
>  }
>
>  bool
> --
> 2.14.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180709/16abb3a1/attachment.html>