[Mesa-dev] [PATCH 02/58] mesa: optimise interleaved sso validation

[Ian Romanick]

On 11/20/2016 05:28 AM, Timothy Arceri wrote:
> Now that we have a linked_stages bitfield we can use this
> to check if the program is used at a later stage.
> 
> This change is also required to be able to use gl_program
> rather than gl_shader_program in the CurrentProgram array.
> ---
>  src/mesa/main/pipelineobj.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
> index 35d416d..45131d2 100644
> --- a/src/mesa/main/pipelineobj.c
> +++ b/src/mesa/main/pipelineobj.c
> @@ -730,30 +730,27 @@ program_stages_all_active(struct gl_pipeline_object *pipe,
>  static bool
>  program_stages_interleaved_illegally(const struct gl_pipeline_object *pipe)
>  {
> -   struct gl_shader_program *prev = NULL;
> -   unsigned i, j;
> +   unsigned prev_linked_stages = 0;
>  
>     /* Look for programs bound to stages: A -> B -> A, with any intervening
>      * sequence of unrelated programs or empty stages.
>      */
> -   for (i = 0; i < MESA_SHADER_STAGES; i++) {
> +   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
>        struct gl_shader_program *cur = pipe->CurrentProgram[i];
>  
>        /* Empty stages anywhere in the pipe are OK */
> -      if (!cur || cur == prev)
> +      if (!cur || cur->data->linked_stages == prev_linked_stages)

The previous 'cur == prev' check would prevent validation when, say, a
program with a linked vertex shader and geometry shader were attached to
a pipeline.  Just looking at this line of code, it's not immediately
obvious why the new check is equivalent.  I'm guessing that code
elsewhere generates an error if the application binds a programs with
the same set of stages to a pipeline

    glGenProgramPipelines(1, &pipe);
    glGenPrograms(2, prog);

    // compile and link two programs that have a vertex & fragment
    // shader.
    // ...

    glBindProgramPipeline(pipe);
    glUseProgramStages(pipe, GL_VERTEX_SHADER_BIT, prog[0]);
    glUseProgramStages(pipe, GL_FRAGMENT_SHADER_BIT, prog[1]);

In the existing code, this pipeline will get validated here (and fail).
With the new code, it will not get validated here.

>           continue;
>  
> -      if (prev) {
> +      if (prev_linked_stages) {
>           /* We've seen an A -> B transition; look at the rest of the pipe
>            * to see if we ever see A again.
>            */
> -         for (j = i + 1; j < MESA_SHADER_STAGES; j++) {
> -            if (pipe->CurrentProgram[j] == prev)
> -               return true;
> -         }
> +         if (prev_linked_stages >> (i + 1))
> +            return true;
>        }
>  
> -      prev = cur;
> +      prev_linked_stages = cur->data->linked_stages;
>     }
>  
>     return false;
>