On Wed, 2015-08-12 at 19:39 +1000, Timothy Arceri wrote: > Cc: Francisco Jerez <curroje...@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.
Just to add some more to this, I'm pretty sure my code is generating the correct offsets. If I hardcode the img_offset offset to 72 to get the uniform value of tex[1][0] I get the value I expected, but if I set image.reladdr to a register that contains 72 I don't what I expect. If I change the array to be a single dimension e.g. tex[4] and I hardcode the offset as described above then it works as expected for both scenarios, it also works if I split the offset across img_offset and image.reladdr, there is something going on with image.reladdr for multi-dimensional arrays that I can' t quite put my finger on. Any hints appreciated. > > 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; > } > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev