[Mesa-dev] [PATCH v2] glsl: make a copy of array indices that are used to deref a function out param

[Ian Romanick]

On 07/20/2018 10:22 PM, Timothy Arceri wrote:
> V2: make use of visit_tree()
> 
> Fixes new piglit test:
> tests/spec/glsl-1.20/execution/qualifiers/vs-out-conversion-int-to-float-vec4-index.shader_test

Should this get tagged for stable?

> ---
>  src/compiler/glsl/ast_function.cpp | 54 ++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp
> index 127aa1f91c4..b0edcc0c5de 100644
> --- a/src/compiler/glsl/ast_function.cpp
> +++ b/src/compiler/glsl/ast_function.cpp
> @@ -348,6 +348,49 @@ verify_parameter_modes(_mesa_glsl_parse_state *state,
>     return true;
>  }
>  
> +struct copy_index_deref_data {
> +   void *mem_ctx;
> +   exec_list *before_instructions;
> +};
> +
> +static void
> +copy_index_derefs_to_temps(ir_instruction *ir, void *data)
> +{
> +   struct copy_index_deref_data *d = (struct copy_index_deref_data *)data;
> +
> +   if (ir->ir_type == ir_type_dereference_array) {
> +      ir_dereference_array *a = (ir_dereference_array *) ir;
> +      ir = a->array->as_dereference();
> +
> +      ir_rvalue *idx = a->array_index;
> +      if (idx->as_dereference_variable()) {
> +         ir_variable *var = idx->variable_referenced();
> +
> +         /* If the index is read only it cannot change so there is no need
> +          * to copy it.
> +          */
> +         if (var->data.read_only || var->data.memory_read_only)
> +            return;
> +
> +         ir_variable *tmp = new(d->mem_ctx) ir_variable(idx->type, "idx_tmp",
> +                                                     ir_var_temporary);

Indentation doesn't line up here.

With that fixed,

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> +         d->before_instructions->push_tail(tmp);
> +
> +         ir_dereference_variable *const deref_tmp_1 =
> +            new(d->mem_ctx) ir_dereference_variable(tmp);
> +         ir_assignment *const assignment =
> +            new(d->mem_ctx) ir_assignment(deref_tmp_1,
> +                                          idx->clone(d->mem_ctx, NULL));
> +         d->before_instructions->push_tail(assignment);
> +
> +         /* Replace the array index with a dereference of the new temporary */
> +         ir_dereference_variable *const deref_tmp_2 =
> +            new(d->mem_ctx) ir_dereference_variable(tmp);
> +         a->array_index = deref_tmp_2;
> +      }
> +   }
> +}
> +
>  static void
>  fix_parameter(void *mem_ctx, ir_rvalue *actual, const glsl_type *formal_type,
>                exec_list *before_instructions, exec_list *after_instructions,
> @@ -362,6 +405,17 @@ fix_parameter(void *mem_ctx, ir_rvalue *actual, const glsl_type *formal_type,
>         && (expr == NULL || expr->operation != ir_binop_vector_extract))
>        return;
>  
> +   /* An array index could also be an out variable so we need to make a copy
> +    * of them before the function is called.
> +    */
> +   if (!actual->as_dereference_variable()) {
> +      struct copy_index_deref_data data;
> +      data.mem_ctx = mem_ctx;
> +      data.before_instructions = before_instructions;
> +
> +      visit_tree(actual, copy_index_derefs_to_temps, &data);
> +   }
> +
>     /* To convert an out parameter, we need to create a temporary variable to
>      * hold the value before conversion, and then perform the conversion after
>      * the function call returns.
>