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

[Kenneth Graunke]

On Friday, July 31, 2015 04:37:16 PM 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>
> ---
>  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.
> 

Yikes.  You can get into trouble with array dereferences too.  Would be
nice to fix those as well...not sure on the best way to handle it
offhand.

Still, this fixes bugs, so in the mean time,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150803/ec2c7451/attachment.sig>