[Mesa-dev] [PATCH] mesa: add const flags to skip MaxVarying and MaxUniform linker checks

[Ian Romanick]

On 12/09/2011 07:14 PM, Marek Olšák wrote:
> This is only temporary until a better solution is available.
> ---
>
> I haven't given up yet because reporting incorrect driver limits
> may cause more troubles than this. I made it pretty small this
> time.
>
> It doesn't change any driver besides Gallium. Please review,
> especially the part in st_extensions.c whether it's acceptable
> or whether some other conditions should be used instead.
> (e.g. strcmp(get_vendor(), "X.Org R300 Project") is possible too)
>
> I plan to implement a better solution, so that this code can go
> away.
>
> I see only two ways out of this.

To be honest, I we want to do both.

> The first one is:
> - Let Gallium drivers report an error when they can't run
>    a certain shader.

I was thinking this would look like:

- Have the linker calculate the most conservative and most optimistic 
uniform and varying usage.

- Have the linker call into the driver to calculate the actual usage. 
Driver's that don't optimize can just report back the data supplied by 
the linker.

- If the driver's usage calculation is less than the conservative 
calculation, generate a link warning as a portability aid.

- If the driver's usage calculation is less than the optimistic 
calculation, generate an assertion failure.  Clearly, some piece of 
software is broken, and some sort of other failure is likely imminent.

- If the driver's usage calculation is greater than the driver's 
advertised limits, generate a link error.

> The second one is:
> - Remove the Mesa IR.

It won't be much longer until i915 and r200 are the only users of Mesa 
IR.  It's a pretty good fit for that hardware, so it won't disappear. 
However, it should disappear from the execution paths of every other driver.

> - Replace TGSI by the GLSL IR completely.

I think that would be useful, but it doesn't seem strictly necessary.

> - Move the dead-constant elimination pass from the R300 compiler
>    to the GLSL IR.

I've added this to my todo list.  The new uniform tracking mechanism may 
make this a little bit more complex.

> - Implement a GLSL optimization pass which breaks varying arrays
>    into separate elements when possible.

This may not be necessary in the short term.  I think r300 is 
under-reporting it's capabilities.  See 
https://bugs.freedesktop.org/show_bug.cgi?id=34201#c9

> Thanks.
>
>   src/glsl/linker.cpp                    |    9 ++++++---
>   src/mesa/main/mtypes.h                 |    9 +++++++++
>   src/mesa/state_tracker/st_extensions.c |    8 ++++++++
>   3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 3527088..fe35ed3 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1814,7 +1814,8 @@ assign_varying_locations(struct gl_context *ctx,
>      }
>
>      if (ctx->API == API_OPENGLES2 || prog->Version == 100) {
> -      if (varying_vectors>  ctx->Const.MaxVarying) {
> +      if (varying_vectors>  ctx->Const.MaxVarying&&
> +          !ctx->Const.GLSLSkipStrictMaxVaryingLimitCheck) {
>   	 linker_error(prog, "shader uses too many varying vectors "
>   		      "(%u>  %u)\n",
>   		      varying_vectors, ctx->Const.MaxVarying);

Even when we have compaction in the front-end, it seems useful, as a 
portability aid, to keep a warning in this case.

> @@ -1822,7 +1823,8 @@ assign_varying_locations(struct gl_context *ctx,
>         }
>      } else {
>         const unsigned float_components = varying_vectors * 4;
> -      if (float_components>  ctx->Const.MaxVarying * 4) {
> +      if (float_components>  ctx->Const.MaxVarying * 4&&
> +          !ctx->Const.GLSLSkipStrictMaxVaryingLimitCheck) {
>   	 linker_error(prog, "shader uses too many varying components "
>   		      "(%u>  %u)\n",
>   		      float_components, ctx->Const.MaxVarying * 4);
> @@ -1959,7 +1961,8 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
>   		      shader_names[i]);
>         }
>
> -      if (sh->num_uniform_components>  max_uniform_components[i]) {
> +      if (sh->num_uniform_components>  max_uniform_components[i]&&
> +          !ctx->Const.GLSLSKipStrictMaxUniformLimitCheck) {
>            linker_error(prog, "Too many %s shader uniform components",
>   		      shader_names[i]);
>         }
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index fc494f7..7e9f6ca 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2829,6 +2829,15 @@ struct gl_constants
>       * Texture borders are deprecated in GL 3.0.
>       **/
>      GLboolean StripTextureBorder;
> +
> +   /**
> +    * For drivers which can do a better job at eliminating unused varyings
> +    * and uniforms than the GLSL compiler.
> +    *
> +    * XXX Remove these as soon as a better solution is available.
> +    */
> +   GLboolean GLSLSkipStrictMaxVaryingLimitCheck;
> +   GLboolean GLSLSKipStrictMaxUniformLimitCheck;
>   };
>
>
> diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
> index 37fb3e7..fdaffa8 100644
> --- a/src/mesa/state_tracker/st_extensions.c
> +++ b/src/mesa/state_tracker/st_extensions.c
> @@ -224,6 +224,14 @@ void st_init_limits(struct st_context *st)
>      c->UniformBooleanTrue = ~0;
>
>      c->StripTextureBorder = GL_TRUE;
> +
> +   c->GLSLSKipStrictMaxUniformLimitCheck =
> +      screen->get_shader_param(screen, PIPE_SHADER_FRAGMENT,
> +                               PIPE_SHADER_CAP_MAX_CONSTS)<= 256;
> +
> +   c->GLSLSkipStrictMaxVaryingLimitCheck =
> +      screen->get_shader_param(screen, PIPE_SHADER_FRAGMENT,
> +                               PIPE_SHADER_CAP_MAX_INPUTS)<= 10;

This will break NVfx (Geforce5) drivers.  On NVfx, the limits are 32 and 
10, respectively, and the the nouveaux maintainers have said that they 
don't have (and won't implement) the array compacting that r300 has.

Maybe adding PIPE_SHADER_CAP_CAN_COMPACT_INPUT_ARRAYS and 
PIPE_SHADER_CAP_CAN_COMPACT_CONST_ARRAYS would be better?

>   }
>
>