[Mesa-dev] [PATCH] mesa: Only validate SSO shader IO in OpenGL ES or debug context

[Ian Romanick]

On 05/26/2016 03:03 PM, Ilia Mirkin wrote:
> This will cause st/mesa to break, no? Right now validate_io iterates
> over the shader ir, which st/mesa frees after linking.

Only as much as it is already broken. :) Any desktop OpenGL application
using GLSL ES shaders would already have that problem.  I suspect there
are more of those than there are applications using debug contexts.  I
believe this should all be cleared up after patch 6 in this series which
deletes the old validate_io function.

>   -ilia
> 
> On Thu, May 26, 2016 at 5:59 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Suggested-by: Timothy Arceri <timothy.arceri at collabora.com>
>> Cc: Timothy Arceri <timothy.arceri at collabora.com>
>> ---
>>  src/mesa/main/pipelineobj.c    | 18 ++++++++++++++++--
>>  src/mesa/main/shader_query.cpp | 12 ++++--------
>>  2 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
>> index b150198..5a46cfe 100644
>> --- a/src/mesa/main/pipelineobj.c
>> +++ b/src/mesa/main/pipelineobj.c
>> @@ -906,7 +906,8 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>>     /* Validate inputs against outputs, this cannot be done during linking
>>      * since programs have been linked separately from each other.
>>      *
>> -    * From OpenGL 4.5 Core spec:
>> +    * Section 11.1.3.11 (Validation) of the OpenGL 4.5 Core Profile spec says:
>> +    *
>>      *     "Separable program objects may have validation failures that cannot be
>>      *     detected without the complete program pipeline. Mismatched interfaces,
>>      *     improper usage of program objects together, and the same
>> @@ -914,8 +915,21 @@ _mesa_validate_program_pipeline(struct gl_context* ctx,
>>      *     program objects."
>>      *
>>      * OpenGL ES 3.1 specification has the same text.
>> +    *
>> +    * Section 11.1.3.11 (Validation) of the OpenGL ES spec also says:
>> +    *
>> +    *    An INVALID_OPERATION error is generated by any command that transfers
>> +    *    vertices to the GL or launches compute work if the current set of
>> +    *    active program objects cannot be executed, for reasons including:
>> +    *
>> +    *    * The current program pipeline object contains a shader interface
>> +    *      that doesn't have an exact match (see section 7.4.1)
>> +    *
>> +    * Based on this, only perform the most-strict checking on ES or when the
>> +    * application has created a debug context.
>>      */
>> -   if (!_mesa_validate_pipeline_io(pipe))
>> +   if ((_mesa_is_gles(ctx) || (ctx->Const.ContextFlags & GL_CONTEXT_FLAG_DEBUG_BIT)) &&
>> +       !_mesa_validate_pipeline_io(pipe))
>>        return GL_FALSE;
>>
>>     pipe->Validated = GL_TRUE;
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index 9e18a1c..a37c179 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -1372,7 +1372,7 @@ _mesa_get_program_resourceiv(struct gl_shader_program *shProg,
>>
>>  static bool
>>  validate_io(const struct gl_shader *producer,
>> -            const struct gl_shader *consumer, bool isES)
>> +            const struct gl_shader *consumer)
>>  {
>>     assert(producer && consumer);
>>     unsigned inputs = 0, outputs = 0;
>> @@ -1416,10 +1416,6 @@ validate_io(const struct gl_shader *producer,
>>      * packing makes this challenging.
>>      */
>>
>> -   /* Currently no matching done for desktop. */
>> -   if (!isES)
>> -      return true;
>> -
>>     /* For each output in a, find input in b and do any required checks. */
>>     foreach_in_list(ir_instruction, out, producer->ir) {
>>        ir_variable *out_var = out->as_variable();
>> @@ -1502,9 +1498,9 @@ _mesa_validate_pipeline_io(struct gl_pipeline_object *pipeline)
>>              break;
>>
>>           if (!validate_io(shProg[prev]->_LinkedShaders[prev],
>> -                          shProg[idx]->_LinkedShaders[idx],
>> -                          shProg[prev]->IsES || shProg[idx]->IsES))
>> -            return false;
>> +                          shProg[idx]->_LinkedShaders[idx]))
>> +               return false;
>> +
>>           prev = idx;
>>        }
>>     }
>> --
>> 2.5.5
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>