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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev