[Mesa-dev] [PATCH 16/28] glsl: don't pack tessellation stages like we do other stages

[Timothy Arceri]

On Wed, 2016-01-06 at 17:50 -0500, Ilia Mirkin wrote:
> On Tue, Dec 29, 2015 at 12:00 AM, Timothy Arceri
> <timothy.arceri at collabora.com> wrote:
> > Tessellation shaders treat varyings as shared memory and
> > invocations
> > can access each others varyings therefore we can't use the existing
> > method to lower them.
> 
> That's not strictly true... this is only true of tess control outputs
> (which can be written by the current invocation, but also read in by
> other invocations, effectively acting as a shared memory -- both true
> of per-invocation outputs as well as per-patch outputs). Does that
> information change this patch at all?

I don't think so. The problem is that the current packing code works
like this:

- Change vars to be packed to temporaries, create new packed varyings.
- Copy *all* values from the new packed input varying to the
 temporaries at the start of main.
- Copy *all* values from the temporaries to the new packed output vars
at the end of main (or before emit for GS).

As well as the invocations stomping on each other this results in 32
(GL_MAX_PATCH_VERTICES?) copies for each TCS input as it just copies
the full array.

The current packing just doesn't work well for tessellation, its easier
to just disbale it for tessellation and do it all using a different
method rather than trying to mix and match.


> 
> > 
> > This adds a check for these stages as following patches will
> > allow explicit locations to be lowered even when the driver and
> > existing
> > tesselation checks ask for it to be disabled, we do this to enable
> > support
> > for the component layout qualifier.
> > ---
> >  src/glsl/lower_packed_varyings.cpp | 62 +++++++++++++++++++++-----
> > ------------
> >  1 file changed, 34 insertions(+), 28 deletions(-)
> > 
> > diff --git a/src/glsl/lower_packed_varyings.cpp
> > b/src/glsl/lower_packed_varyings.cpp
> > index 2899846..e4e9a35 100644
> > --- a/src/glsl/lower_packed_varyings.cpp
> > +++ b/src/glsl/lower_packed_varyings.cpp
> > @@ -737,40 +737,46 @@ lower_packed_varyings(void *mem_ctx, unsigned
> > locations_used,
> >                        ir_variable_mode mode, unsigned
> > gs_input_vertices,
> >                        gl_shader *shader, bool
> > disable_varying_packing)
> >  {
> > -   exec_list *instructions = shader->ir;
> >     ir_function *main_func = shader->symbols->get_function("main");
> >     exec_list void_parameters;
> >     ir_function_signature *main_func_sig
> >        = main_func->matching_signature(NULL, &void_parameters,
> > false);
> > -   exec_list new_instructions, new_variables;
> > -   lower_packed_varyings_visitor visitor(mem_ctx, locations_used,
> > mode,
> > -                                         gs_input_vertices,
> > -                                         &new_instructions,
> > -                                         &new_variables,
> > -                                         disable_varying_packing);
> > -   visitor.run(shader);
> > -   if (mode == ir_var_shader_out) {
> > -      if (shader->Stage == MESA_SHADER_GEOMETRY) {
> > -         /* For geometry shaders, outputs need to be lowered
> > before each call
> > -          * to EmitVertex()
> > -          */
> > -         lower_packed_varyings_gs_splicer splicer(mem_ctx,
> > &new_instructions);
> > -
> > -         /* Add all the variables in first. */
> > -         main_func_sig->body.head->insert_before(&new_variables);
> > 
> > -         /* Now update all the EmitVertex instances */
> > -         splicer.run(instructions);
> > +   if (!(shader->Stage == MESA_SHADER_TESS_CTRL ||
> > +         shader->Stage == MESA_SHADER_TESS_EVAL)) {
> > +      exec_list *instructions = shader->ir;
> > +      exec_list new_instructions, new_variables;
> > +
> > +      lower_packed_varyings_visitor visitor(mem_ctx,
> > locations_used, mode,
> > +                                            gs_input_vertices,
> > +                                            &new_instructions,
> > +                                            &new_variables,
> > +                                           
> >  disable_varying_packing);
> > +      visitor.run(shader);
> > +      if (mode == ir_var_shader_out) {
> > +         if (shader->Stage == MESA_SHADER_GEOMETRY) {
> > +            /* For geometry shaders, outputs need to be lowered
> > before each
> > +             * call to EmitVertex()
> > +             */
> > +            lower_packed_varyings_gs_splicer splicer(mem_ctx,
> > +                                                    
> >  &new_instructions);
> > +
> > +            /* Add all the variables in first. */
> > +            main_func_sig->body.head
> > ->insert_before(&new_variables);
> > +
> > +            /* Now update all the EmitVertex instances */
> > +            splicer.run(instructions);
> > +         } else {
> > +            /* For other shader types, outputs need to be lowered
> > at the end
> > +             * of main()
> > +             */
> > +            main_func_sig->body.append_list(&new_variables);
> > +            main_func_sig->body.append_list(&new_instructions);
> > +         }
> >        } else {
> > -         /* For other shader types, outputs need to be lowered at
> > the end of
> > -          * main()
> > -          */
> > -         main_func_sig->body.append_list(&new_variables);
> > -         main_func_sig->body.append_list(&new_instructions);
> > +         /* Shader inputs need to be lowered at the beginning of
> > main() */
> > +         main_func_sig->body.head
> > ->insert_before(&new_instructions);
> > +         main_func_sig->body.head->insert_before(&new_variables);
> >        }
> > -   } else {
> > -      /* Shader inputs need to be lowered at the beginning of
> > main() */
> > -      main_func_sig->body.head->insert_before(&new_instructions);
> > -      main_func_sig->body.head->insert_before(&new_variables);
> >     }
> >  }
> > --
> > 2.4.3
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev