[Mesa-dev] [PATCH] radeonsi: Use param export count from si_llvm_export_vs in si_shader_vs

[Marek Olšák]

Hi Michel,

VS_EXPORT_COUNT shouldn't include POS exports (POSITION, MISC (psize,
edgeflag, layer, viewport), and CLIPDIST/CULLDIST), it should only
count PARAM exports. Setting higher VS_EXPORT_COUNT doesn't affect
correctness but it allocates more parameter memory. CLIPDIST is
special in that it's exported twice, first as POS and then as PARAM,
the same will be done for CULLDIST in the future, so these two should
indeed be removed from the list.

Marek


On Mon, Jul 6, 2015 at 10:30 AM, Michel Dänzer <michel at daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> This eliminates the error prone logic in si_shader_vs recalculating this
> value.
>
> It also fixes TGSI_SEMANTIC_CLIPDIST outputs incorrectly not being
> counted for VS exports, since they are passed to the fragment shader as
> varyings as well.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91193
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>  src/gallium/drivers/radeonsi/si_shader.c        |  2 ++
>  src/gallium/drivers/radeonsi/si_shader.h        |  1 +
>  src/gallium/drivers/radeonsi/si_state_shaders.c | 25 +++----------------------
>  3 files changed, 6 insertions(+), 22 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c
> index 4d97b58..753b238 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -1218,6 +1218,8 @@ handle_semantic:
>                 }
>         }
>
> +       shader->nr_param_exports = param_count;
> +
>         /* We need to add the position output manually if it's missing. */
>         if (!pos_args[0][0]) {
>                 pos_args[0][0] = lp_build_const_int32(base->gallivm, 0xf); /* writemask */
> diff --git a/src/gallium/drivers/radeonsi/si_shader.h b/src/gallium/drivers/radeonsi/si_shader.h
> index b4339ae..8d309b4 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.h
> +++ b/src/gallium/drivers/radeonsi/si_shader.h
> @@ -165,6 +165,7 @@ struct si_shader {
>
>         bool                    uses_instanceid;
>         unsigned                nr_pos_exports;
> +       unsigned                nr_param_exports;
>         bool                    is_gs_copy_shader;
>         bool                    dx10_clamp_mode; /* convert NaNs to 0 */
>  };
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index eef3baa..a842d9d 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -148,10 +148,9 @@ static void si_shader_gs(struct si_shader *shader)
>
>  static void si_shader_vs(struct si_shader *shader)
>  {
> -       struct tgsi_shader_info *info = &shader->selector->info;
>         struct si_pm4_state *pm4;
>         unsigned num_sgprs, num_user_sgprs;
> -       unsigned nparams, i, vgpr_comp_cnt;
> +       unsigned nparams, vgpr_comp_cnt;
>         uint64_t va;
>         unsigned window_space =
>            shader->selector->info.properties[TGSI_PROPERTY_VS_WINDOW_SPACE_POSITION];
> @@ -180,26 +179,8 @@ static void si_shader_vs(struct si_shader *shader)
>         }
>         assert(num_sgprs <= 104);
>
> -       /* Certain attributes (position, psize, etc.) don't count as params.
> -        * VS is required to export at least one param and r600_shader_from_tgsi()
> -        * takes care of adding a dummy export.
> -        */
> -       for (nparams = 0, i = 0 ; i < info->num_outputs; i++) {
> -               switch (info->output_semantic_name[i]) {
> -               case TGSI_SEMANTIC_CLIPVERTEX:
> -               case TGSI_SEMANTIC_CLIPDIST:
> -               case TGSI_SEMANTIC_CULLDIST:
> -               case TGSI_SEMANTIC_POSITION:
> -               case TGSI_SEMANTIC_PSIZE:
> -               case TGSI_SEMANTIC_EDGEFLAG:
> -                       break;
> -               default:
> -                       nparams++;
> -               }
> -       }
> -       if (nparams < 1)
> -               nparams = 1;
> -
> +       /* VS is required to export at least one param. */
> +       nparams = MAX2(shader->nr_param_exports, 1);
>         si_pm4_set_reg(pm4, R_0286C4_SPI_VS_OUT_CONFIG,
>                        S_0286C4_VS_EXPORT_COUNT(nparams - 1));
>
> --
> 2.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev