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

[Ilia Mirkin]

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

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