spirv: Fix InterpolateAt* instructions for vecs with dynamic index
authorNeil Roberts <nroberts@igalia.com>
Wed, 9 May 2018 13:14:36 +0000 (15:14 +0200)
committerAlejandro Piñeiro <apinheiro@igalia.com>
Tue, 10 Jul 2018 09:43:40 +0000 (11:43 +0200)
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@igalia.com>
Signed-off-by: Alejandro Piñeiro <apinheiro@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.

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
src/compiler/spirv/vtn_glsl450.c

index 7f941ceeb0d0a95b8b69a76a3c30f3d3e9d9e646..8393450f2f474f684d3eb2dda12b8515ed00baf2 100644 (file)
@@ -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