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

[Ilia Mirkin]

On Wed, Jan 6, 2016 at 6:40 PM, Timothy Arceri
<timothy.arceri at collabora.com> wrote:
> 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.

Presumably it also does this for GS? Although it's a lot more common
for a single GS invocation to consume

>
> 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.

I thought it already *was* disabled... but I think you still have to
have packing on TES outputs, because (a) your arguments against don't
apply and (b) it might feed into transform feedback, which i have
faint recollections must go through packing.

>
>
>>
>> >
>> > 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