[Mesa-dev] [PATCH] glsl: when generating out/inout parameter fixups, do indexing before the call

[Ben Widawsky]

On Fri, Jul 31, 2015 at 04:37:16PM +1200, Chris Forbes wrote:
> If the indexing expression involves anything modified during the call,
> the fixup code to copy back after the call would use the new values.
> 
> This fixes the cases where the first expression node encountered is
> ir_binop_vector_extract, fixing the piglits:
> 
> * shaders at out-parameter-indexing@vs-inout-index-inout-mat2-row
> * shaders at out-parameter-indexing@vs-inout-index-inout-vec4
> * shaders at out-parameter-indexing@vs-inout-index-inout-vec4-array-element
> 
> Further changes are needed for other expression types.
> 
> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
> Cc: Ben Widawsky <ben at bwidawsk.net>

Tested-by: Ben Widawsky <ben at bwidawsk.net>

I'm not familiar with this code. I can review it if needed, but I'd prefer not
to if there is someone better equipped.

> ---
>  src/glsl/ast_function.cpp | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 803edf5..e7147dd 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -299,6 +299,21 @@ fix_parameter(void *mem_ctx, ir_rvalue *actual, const glsl_type *formal_type,
>  
>     before_instructions->push_tail(tmp);
>  
> +   if (expr != NULL && expr->operation == ir_binop_vector_extract) {
> +      /* We're going to have to emit a matching insert after the call.
> +       * evaluate the indexing expression before the call, because it
> +       * may reference things that change during the call.
> +       */
> +      ir_variable *index_tmp = new(mem_ctx) ir_variable(expr->operands[1]->type,
> +            "inout_index_tmp", ir_var_temporary);
> +      before_instructions->push_tail(index_tmp);
> +      before_instructions->push_tail(
> +            new(mem_ctx) ir_assignment(
> +               new(mem_ctx) ir_dereference_variable(index_tmp),
> +               expr->operands[1]->clone(mem_ctx, NULL)));
> +      expr->operands[1] = new(mem_ctx) ir_dereference_variable(index_tmp);
> +   }
> +
>     /* If the parameter is an inout parameter, copy the value of the actual
>      * parameter to the new temporary.  Note that no type conversion is allowed
>      * here because inout parameters must match types exactly.
> -- 
> 2.5.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev