[Mesa-dev] [PATCH V2 16/35] glsl: move packing rules for tessellation stages into the packing code

Timothy Arceri timothy.arceri at collabora.com
Wed Jan 6 17:44:04 PST 2016


On Wed, 2016-01-06 at 20:19 -0500, Ilia Mirkin wrote:
> On Wed, Jan 6, 2016 at 8:00 PM, Timothy Arceri
> <timothy.arceri at collabora.com> wrote:
> > Following patches will allow packing of varyings with explicit
> > locations
> > via the component layout qualifier. Moving the rules here will
> > enable
> > us to call an alternate path for packing tessellation stages with
> > explicit locations.
> > ---
> >  V2: move the tessellation packing rules, allow TES output to be
> > packed.
> > 
> >  src/glsl/link_varyings.cpp         | 17 ++--------
> >  src/glsl/lower_packed_varyings.cpp | 63 +++++++++++++++++++++-----
> > ------------
> >  2 files changed, 38 insertions(+), 42 deletions(-)
> > 
> > diff --git a/src/glsl/link_varyings.cpp
> > b/src/glsl/link_varyings.cpp
> > index be662bc..69e24e3 100644
> > --- a/src/glsl/link_varyings.cpp
> > +++ b/src/glsl/link_varyings.cpp
> > @@ -1640,18 +1640,7 @@ assign_varying_locations(struct gl_context
> > *ctx,
> >        assert(!ctx->Extensions.EXT_transform_feedback);
> >     }
> > 
> > -   /* Tessellation shaders treat inputs and outputs as shared
> > memory and can
> > -    * access inputs and outputs of other invocations.
> > -    * Therefore, they can't be lowered to temps easily (and
> > definitely not
> > -    * efficiently).
> > -    */
> > -   bool disable_varying_packing =
> > -      ctx->Const.DisableVaryingPacking ||
> > -      (consumer && consumer->Stage == MESA_SHADER_TESS_EVAL) ||
> > -      (consumer && consumer->Stage == MESA_SHADER_TESS_CTRL) ||
> > -      (producer && producer->Stage == MESA_SHADER_TESS_CTRL);
> > -
> > -   varying_matches matches(disable_varying_packing,
> > +   varying_matches matches(ctx->Const.DisableVaryingPacking,
> >                             producer ? producer->Stage :
> > (gl_shader_stage)-1,
> >                             consumer ? consumer->Stage :
> > (gl_shader_stage)-1);
> >     hash_table *tfeedback_candidates
> > @@ -1864,13 +1853,13 @@ assign_varying_locations(struct gl_context
> > *ctx,
> > 
> >     if (producer) {
> >        lower_packed_varyings(mem_ctx, slots_used,
> > ir_var_shader_out,
> > -                            0, producer, disable_varying_packing);
> > +                            0, producer, ctx
> > ->Const.DisableVaryingPacking);
> >     }
> > 
> >     if (consumer) {
> >        lower_packed_varyings(mem_ctx, slots_used, ir_var_shader_in,
> >                              consumer_vertices, consumer,
> > -                            disable_varying_packing);
> > +                            ctx->Const.DisableVaryingPacking);
> >     }
> > 
> >     return true;
> > diff --git a/src/glsl/lower_packed_varyings.cpp
> > b/src/glsl/lower_packed_varyings.cpp
> > index 2899846..4723c2b 100644
> > --- a/src/glsl/lower_packed_varyings.cpp
> > +++ b/src/glsl/lower_packed_varyings.cpp
> > @@ -737,40 +737,47 @@ 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 &&
> > +          mode == ir_var_shader_in))) {
> 
> To match what was being done before (and IMHO much more readable, you
> might do something like
> 
> if (shader->Stage == TESS_CTRL || (shader->stage == TESS_EVAL && mode
> == in))
>   disable_varying_packing = true;
> 
> That would be equivalent to what was being done before right? Or if
> you don't want the disable_varying_packing thing to be set to true in
> that case, you could just return... would still be easier to read and
> avoid the extra indent.
> 
>   -ilia

The indenting is to reduce code churn and patch size in patch 26.



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


More information about the mesa-dev mailing list