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

[Marek Olšák]

I thought your patch does something else, but after re-reading it, it
seems to do the right thing.

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Mon, Jul 6, 2015 at 1:38 PM, Marek Olšák <maraeo at gmail.com> wrote:
> 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