[Mesa-stable] [PATCH v2] glsl: Lower variable indexing of system value arrays unconditionally.

[Ilia Mirkin]

On Mon, Apr 4, 2016 at 2:09 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> lower_variable_index_to_cond_assign() did not handle system values.
> gl_SampleMaskIn[] is a system value, and also an array.  Accessing it
> with a variable index would trigger an unreachable() assert.
>
> Rather than adding a new EmitNoIndirectSystemValues flag, we simply
> lower unconditionally.  There is exactly one case where this occurs,
> and for all current drivers, lowering produces optimal code.  Even
> for future drivers with 32x MSAA, it produces reasonable code.
>
> Fixes Piglit's new samplemaskin-indirect test.  Also fixes many ES31-CTS
> tests when OES_sample_variables is enabled.
>
> Cc: mesa-stable at lists.freedesktop.org
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  .../glsl/lower_variable_index_to_cond_assign.cpp     | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/src/compiler/glsl/lower_variable_index_to_cond_assign.cpp b/src/compiler/glsl/lower_variable_index_to_cond_assign.cpp
> index 278d545..057de31 100644
> --- a/src/compiler/glsl/lower_variable_index_to_cond_assign.cpp
> +++ b/src/compiler/glsl/lower_variable_index_to_cond_assign.cpp
> @@ -385,6 +385,26 @@ public:
>        case ir_var_const_in:
>           return this->lower_temps;
>
> +      case ir_var_system_value:
> +         /* There are only a few system values that have array types:
> +          *
> +          *    gl_TessLevelInner[]
> +          *    gl_TessLevelOuter[]
> +          *    gl_SampleMaskIn[]
> +          *
> +          * The tessellation factor arrays are lowered to vec4/vec2s
> +          * by lower_tess_level() before this pass occurs, so we'll
> +          * never see them here.
> +          *
> +          * The only remaining case is gl_SampleMaskIn[], which has
> +          * a length of ceil(ctx->Const.MaxSamples, 32).  Most hardware

DIV_ROUND_UP(ctx->Const.MaxSamples, 32) hopefully. [Unless ceil does
something I don't know about.]

> +          * supports fewer than 32 samples, at which point our lowering
> +          * produces a single read of gl_SampleMaskIn[0].  Even with 32x
> +          * MSAA, the array length is only 2, so the lowering is fairly

Still 1 at 32x MSAA. Would be 2 at a (hypothetical) 33x MSAA or more
probable 64x MSAA.

With the comment adjusted to better reflect reality,

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

> +          * efficient.  Therefore, lower unconditionally.
> +          */
> +         return true;
> +
>        case ir_var_shader_in:
>           /* The input array size is unknown at compiler time for non-patch
>            * inputs in TCS and TES. The arrays are sized to
> --
> 2.7.4
>