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

[Christoph Bumiller]

On 10.12.2011 20:19, Ian Romanick wrote:
> 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.

I was only thinking of nv50+ there, nvfx doesn't have uniform buffer
objects so we have to touch all the constants data with the CPU anyway,
which makes compactification would be a good idea for nvfx.

Oh, and, the Uniform variable name contains a typo (capital K in "skip").

>
> Maybe adding PIPE_SHADER_CAP_CAN_COMPACT_INPUT_ARRAYS and
> PIPE_SHADER_CAP_CAN_COMPACT_CONST_ARRAYS would be better?
>
>>   }
>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>