[Mesa-dev] [PATCH 1/2] glsl/opt_array_splitting: Fix crash when doing array indexing into other arrays

[Iago Toral]

I think this never got a review, anyone willing to take it?

Iago

On Mon, 2015-09-14 at 13:49 +0200, Iago Toral Quiroga wrote:
> When we find indirect indexing into an array, the current implementation
> of the array spliiting optimization pass does not look further into the
> expression tree. However, if the variable expression involves variable
> indexing into other arrays, we can miss that these other arrays also have
> variable indexing. If that happens, the pass will crash later on after
> hitting an assertion put there to ensure that split arrays are in fact
> always indexed via constants:
> 
> shader_runner: opt_array_splitting.cpp:296:
> void ir_array_splitting_visitor::split_deref(ir_dereference**): Assertion `constant' failed.
> 
> This patch fixes the problem by letting the pass step into the variable
> index expression to identify these cases properly.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89607
> ---
>  src/glsl/opt_array_splitting.cpp | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/glsl/opt_array_splitting.cpp b/src/glsl/opt_array_splitting.cpp
> index 9e73f3c..1fdd013 100644
> --- a/src/glsl/opt_array_splitting.cpp
> +++ b/src/glsl/opt_array_splitting.cpp
> @@ -185,8 +185,18 @@ ir_array_reference_visitor::visit_enter(ir_dereference_array *ir)
>     /* If the access to the array has a variable index, we wouldn't
>      * know which split variable this dereference should go to.
>      */
> -   if (entry && !ir->array_index->as_constant())
> -      entry->split = false;
> +   if (!ir->array_index->as_constant()) {
> +      if (entry)
> +         entry->split = false;
> +      /* This variable indexing could come from a different array dereference
> +       * that also has variable indexing, that is, something like a[b[a[b[0]]]].
> +       * If we return visit_continue_with_parent here for the first appearence
> +       * of a, then we can miss that b also has indirect indexing (if this is
> +       * the only place in the program where such indirect indexing into b
> +       * happens), so keep going.
> +       */
> +      return visit_continue;
> +   }
>  
>     return visit_continue_with_parent;
>  }