[Mesa-dev] [PATCH 04/10] mesa: Check isES before calling validate_io

[Timothy Arceri]

On Fri, 2016-05-20 at 00:25 -0700, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
> 
> There's going to be a second validate_io function, and checking the
> same
> thing twice is silly.

I think we should just do this check
in _mesa_validate_program_pipeline() before we
call _mesa_validate_pipeline_io() otherwise we end up looping over the
shaders at draw time for desktop when there is no need to.

The comment above that call quotes 

"From OpenGL 4.5 Core spec:
    * "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
    *     state-dependent failures can result in validation errors for
such
    *     program objects."

However I think this is a spec bug I filed https://cvs.khronos.org/bugz
illa/show_bug.cgi?id=15331 for it.

> 
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/mesa/main/shader_query.cpp | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/src/mesa/main/shader_query.cpp
> b/src/mesa/main/shader_query.cpp
> index 9e18a1c..a120cb4 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();
> @@ -1501,10 +1497,12 @@ _mesa_validate_pipeline_io(struct
> gl_pipeline_object *pipeline)
>           if (shProg[idx]->_LinkedShaders[idx]->Stage ==
> MESA_SHADER_COMPUTE)
>              break;
>  
> -         if (!validate_io(shProg[prev]->_LinkedShaders[prev],
> -                          shProg[idx]->_LinkedShaders[idx],
> -                          shProg[prev]->IsES || shProg[idx]->IsES))
> -            return false;
> +         if (shProg[prev]->IsES || shProg[idx]->IsES) {
> +            if (!validate_io(shProg[prev]->_LinkedShaders[prev],
> +                             shProg[idx]->_LinkedShaders[idx]))
> +               return false;
> +         }
> +
>           prev = idx;
>        }
>     }