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

[Timothy Arceri]

On Mon, 2016-11-28 at 18:59 -0800, Ian Romanick wrote:
> 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.

Your guess was correct. The check above this
one program_stages_all_active() ensures that all stages of a program
are active so we will always fail before validating interleaved stages
in the example you gave. Therefore cur->data->linked_stages ==
prev_linked_stages can only ever mean that we are looking at the same
program.

I could add a comment if you think it would help?

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