[Mesa-dev] [PATCH 1/4] st/mesa: handle indirect samplers in arrays/structs properly (v3)

[Dave Airlie]

On 5 February 2016 at 14:55, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> On Fri, 2016-02-05 at 13:40 +1000, Dave Airlie wrote:
>> From: Dave Airlie <airlied at redhat.com>
>>
>> The state tracker never handled this properly, and it finally
>> annoyed me for the second time so I decided to fix it properly.
>>
>> This is inspired by the NIR sampler lowering code and I only realised
>> NIR seems to do its deref ordering different to GLSL at the last
>> minute, once I got that things got much easier.
>>
>> it fixes a bunch of tests in
>> tests/spec/arb_gpu_shader5/execution/sampler_array_indexing/
>>
>> v2: fix AoA tests when forced on.
>> I was right I didn't need all that code, fixing the AoA code
>> meant cleaning up a chunk of code I didn't like in the array
>> handling.
>>
>> v3: start generalising the code a bit more for atomics.
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 145
>> +++++++++++++++++++++++++----
>>  1 file changed, 127 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index b8182de..c36edc9 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -40,7 +40,6 @@
>>  #include "main/uniforms.h"
>>  #include "main/shaderapi.h"
>>  #include "program/prog_instruction.h"
>> -#include "program/sampler.h"
>>
>>  #include "pipe/p_context.h"
>>  #include "pipe/p_screen.h"
>> @@ -257,6 +256,7 @@ public:
>>     GLboolean cond_update;
>>     bool saturate;
>>     st_src_reg sampler; /**< sampler register */
>> +   int sampler_base;
>>     int sampler_array_size; /**< 1-based size of sampler array, 1 if
>> not array */
>>     int tex_target; /**< One of TEXTURE_*_INDEX */
>>     glsl_base_type tex_type;
>> @@ -502,6 +502,18 @@ public:
>>
>>     void emit_arl(ir_instruction *ir, st_dst_reg dst, st_src_reg
>> src0);
>>
>> +   void get_deref_offsets(ir_dereference *ir,
>> +                          unsigned *array_size,
>> +                          unsigned *base,
>> +                          unsigned *index,
>> +                          st_src_reg *reladdr);
>> +  void calc_deref_offsets(ir_dereference *head,
>> +                          ir_dereference *tail,
>> +                          unsigned *array_elements,
>> +                          unsigned *index,
>> +                          st_src_reg *indirect,
>> +                          unsigned *location);
>> +
>>     bool try_emit_mad(ir_expression *ir,
>>                int mul_operand);
>>     bool try_emit_mad_for_and_not(ir_expression *ir,
>> @@ -3436,18 +3448,118 @@ glsl_to_tgsi_visitor::visit(ir_call *ir)
>>     this->result = entry->return_reg;
>>  }
>>
>> +static unsigned
>> +glsl_get_length(const struct glsl_type *type)
>> +{
>> +   return type->is_matrix() ? type->matrix_columns : type->length;
>> +}
>> +
>> +void
>> +glsl_to_tgsi_visitor::calc_deref_offsets(ir_dereference *head,
>> +                                         ir_dereference *tail,
>> +                                         unsigned *array_elements,
>> +                                         unsigned *index,
>> +                                         st_src_reg *indirect,
>> +                                         unsigned *location)
>> +{
>> +   switch (tail->ir_type) {
>> +   case ir_type_dereference_record: {
>> +      ir_dereference_record *deref_record = tail-
>> >as_dereference_record();
>> +      const glsl_type *struct_type = deref_record->record->type;
>> +      int tmp = 0;
>> +      unsigned i;
>> +
>> +      calc_deref_offsets(head, deref_record->record-
>> >as_dereference(), array_elements, index, indirect, location);
>> +
>> +      for (i = 0; i < struct_type->length; i++) {
>> +         if (strcmp(struct_type->fields.structure[i].name,
>> deref_record->field) == 0)
>> +            break;
>> +      }
>> +      if (i < struct_type->length)
>> +         tmp = struct_type->record_location_offset(i);
>> +
>> +      *location += tmp;
>> +      break;
>> +   }
>> +
>> +   case ir_type_dereference_array: {
>> +      ir_dereference_array *deref_arr = tail-
>> >as_dereference_array();
>> +      ir_constant *array_index = deref_arr->array_index-
>> >constant_expression_value();
>> +
>> +      if (deref_arr->array->ir_type == ir_type_dereference_variable
>> && head == tail) {
>> +         ir_dereference_variable *deref_var = deref_arr->array-
>> >as_dereference_variable();
>> +         *location += deref_var->var->data.location;
>
> This bit doesn't seem right. This is the base location of the array, if
> you keep adding it I don't see how you could be getting the right
> answer.
>
> Don't you just need to pass this into calc_deref_offsets on the first
> call then add to it with each recursive call?

note the head == tail above. This only applies if the first level is an array
of a variable.

Dave.