[Mesa-dev] [PATCH 1/6] glsl: Mark array access when copying to a temporary for the ?: operator.

[Jason Ekstrand]

I gave you an back on 3; I'll let Eric actually review it.  The rest are

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
On Mar 6, 2015 2:18 AM, "Kenneth Graunke" <kenneth at whitecape.org> wrote:

> Piglit's spec/glsl-1.20/compiler/structure-and-array-operations/
> array-selection.vert test contains the following code:
>
>    gl_Position = (pick_from_a_or_b ? a : b)[i];
>
> where "a" and "b" are uniform vec4[2] variables.
>
> ast_to_hir creates a temporary vec4[2] variable, conditional_tmp, and
> generates an if-block to copy one or the other:
>
>    (declare (temporary) (array vec4 2) conditional_tmp)
>    (if (var_ref pick_from_a_or_b)
>      ((assign () (var_ref conditional_tmp) (var_ref a)))
>      ((assign () (var_ref conditional_tmp) (var_ref b))))
>
> However, we failed to update max_array_access for "a" and "b", so it
> remained 0 - here, the whole array is being accessed.  At link time,
> update_array_sizes() used this bogus information to change the types
> of "a" and "b" to vec4[1].  We then had assignments from a vec4[1] to
> a vec4[2], which is highly illegal.
>
> This tripped assertions in nir_split_var_copies with scalar VS.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/glsl/ast_to_hir.cpp | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index acb5c76..d387b2e 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -1617,6 +1617,12 @@ ast_expression::do_hir(exec_list *instructions,
>            && cond_val != NULL) {
>           result = cond_val->value.b[0] ? op[1] : op[2];
>        } else {
> +         /* The copy to conditional_tmp reads the whole array. */
> +         if (type->is_array()) {
> +            mark_whole_array_access(op[1]);
> +            mark_whole_array_access(op[2]);
> +         }
> +
>           ir_variable *const tmp =
>              new(ctx) ir_variable(type, "conditional_tmp",
> ir_var_temporary);
>           instructions->push_tail(tmp);
> --
> 2.2.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150306/aaa99dd0/attachment-0001.html>