[Mesa-dev] [RFC 2/2] i965: add support for image AoA

[Timothy Arceri]

Cc: Francisco Jerez <currojerez at riseup.net>
---
 This patch works for direct indexing of images, but I'm having some trouble
 getting indirect to work.

 For example for:
 tests/spec/arb_arrays_of_arrays/execution/image_store/basic-imageStore
 -non-const-uniform-index.shader_test

 Which has and image writeonly uniform image2D tex[2][2]

 Indirect indexing will work for tex[0][0] and text[0][1] but not for
 tex[1][0] and tex[1][1] they seem to always end up refering to the
 image in 0.

 I can't quite seem to see either my mistake or what I'm missing so I thought
 I'd send this out, and see if anyone has any ideas. I've also sent some
 tests with mixed direct/indirect indexing which seem to calculate the correct
 offest for the direct but the indirect indexing is not working there either. 

 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 53 ++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index d7a2500..a49c230 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -226,6 +226,7 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
       * our name.
       */
    unsigned index = var->data.driver_location;
+   bool set_image_location = true;
    for (unsigned u = 0; u < shader_prog->NumUniformStorage; u++) {
       struct gl_uniform_storage *storage = &shader_prog->UniformStorage[u];
 
@@ -244,7 +245,13 @@ fs_visitor::nir_setup_uniform(nir_variable *var)
           * because their size is driver-specific, so we need to allocate
           * space for them here at the end of the parameter array.
           */
-         var->data.driver_location = uniforms;
+         if (set_image_location) {
+            /* For arrays of arrays we only want to set this once at the base
+             * location.
+             */
+            var->data.driver_location = uniforms;
+            set_image_location = false;
+         }
          param_size[uniforms] =
             BRW_IMAGE_PARAM_SIZE * MAX2(storage->array_elements, 1);
 
@@ -1165,19 +1172,27 @@ fs_visitor::get_nir_image_deref(const nir_deref_var *deref)
 {
    fs_reg image(UNIFORM, deref->var->data.driver_location,
                 BRW_REGISTER_TYPE_UD);
-
-   if (deref->deref.child) {
-      const nir_deref_array *deref_array =
-         nir_deref_as_array(deref->deref.child);
-      assert(deref->deref.child->deref_type == nir_deref_type_array &&
-             deref_array->deref.child == NULL);
-      const unsigned size = glsl_get_length(deref->var->type);
+   fs_reg *indirect_offset = NULL;
+
+   unsigned img_offset = 0;
+   const nir_deref *tail = &deref->deref;
+   while (tail->child) {
+      const nir_deref_array *deref_array = nir_deref_as_array(tail->child);
+      assert(tail->child->deref_type == nir_deref_type_array);
+      tail = tail->child;
+      const unsigned size = glsl_get_length(tail->type);
+      const unsigned child_array_elements = tail->child != NULL ?
+         glsl_get_aoa_size(tail->type) : 1;
       const unsigned base = MIN2(deref_array->base_offset, size - 1);
-
-      image = offset(image, bld, base * BRW_IMAGE_PARAM_SIZE);
+      const unsigned aoa_size = child_array_elements * BRW_IMAGE_PARAM_SIZE;
+      img_offset += base * aoa_size;
 
       if (deref_array->deref_array_type == nir_deref_array_type_indirect) {
-         fs_reg *tmp = new(mem_ctx) fs_reg(vgrf(glsl_type::int_type));
+         fs_reg tmp = vgrf(glsl_type::int_type);
+         if (indirect_offset == NULL) {
+            indirect_offset = new(mem_ctx) fs_reg(vgrf(glsl_type::int_type));
+            bld.MOV(*indirect_offset, fs_reg(0));
+         }
 
          if (devinfo->gen == 7 && !devinfo->is_haswell) {
             /* IVB hangs when trying to access an invalid surface index with
@@ -1188,18 +1203,22 @@ fs_visitor::get_nir_image_deref(const nir_deref_var *deref)
              * of the possible outcomes of the hang.  Clamp the index to
              * prevent access outside of the array bounds.
              */
-            bld.emit_minmax(*tmp, retype(get_nir_src(deref_array->indirect),
-                                         BRW_REGISTER_TYPE_UD),
+            bld.emit_minmax(tmp, retype(get_nir_src(deref_array->indirect),
+                                        BRW_REGISTER_TYPE_UD),
                             fs_reg(size - base - 1), BRW_CONDITIONAL_L);
          } else {
-            bld.MOV(*tmp, get_nir_src(deref_array->indirect));
+            bld.MOV(tmp, get_nir_src(deref_array->indirect));
          }
-
-         bld.MUL(*tmp, *tmp, fs_reg(BRW_IMAGE_PARAM_SIZE));
-         image.reladdr = tmp;
+         bld.MUL(tmp, tmp, fs_reg(aoa_size));
+         bld.ADD(*indirect_offset, *indirect_offset, tmp);
       }
    }
 
+   if (indirect_offset) {
+      image.reladdr = indirect_offset;
+   }
+   image = offset(image, bld, img_offset);
+
    return image;
 }
 
-- 
2.4.3