Re: [Mesa-dev] [PATCH 5/8] radeonsi: don't set spi_ps_input_* for monolithic shaders

2019-06-24 Thread Bas Nieuwenhuizen 
sorry, totally missed that it was set in src/amd/common as well.

r-b for this patch too.

On Tue, Jun 25, 2019 at 2:17 AM Marek Olšák  wrote:
>
> If the shader is monolithic, the value of SPI_PS_INPUT_ENA that is generated 
> by LLVM is used as-is.
>
> Marek
>
> On Fri, Jun 21, 2019 at 7:03 PM Bas Nieuwenhuizen  
> wrote:
>>
>> Doesn't this cause assertions in si_shader_ps() for monolithic
>> shaders? Some of these assertions check that at least one bit in a
>> group is set and I think we end up with input_ena = 0 for monolithic
>> shaders now?
>>
>> On Thu, Jun 20, 2019 at 6:20 AM Marek Olšák  wrote:
>> >
>> > From: Marek Olšák 
>> >
>> > The driver doesn't use these values and ac_rtld has assertions
>> > expecting the value of 0.
>> > ---
>> >  src/gallium/drivers/radeonsi/si_shader.c | 39 
>> >  1 file changed, 26 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
>> > b/src/gallium/drivers/radeonsi/si_shader.c
>> > index 54b29d0ae01..0489399b827 100644
>> > --- a/src/gallium/drivers/radeonsi/si_shader.c
>> > +++ b/src/gallium/drivers/radeonsi/si_shader.c
>> > @@ -6128,21 +6128,22 @@ static void si_get_ps_prolog_key(struct si_shader 
>> > *shader,
>> >  key->ps_prolog.states.bc_optimize_for_linear);
>> > key->ps_prolog.ancillary_vgpr_index = 
>> > shader->info.ancillary_vgpr_index;
>> >
>> > if (info->colors_read) {
>> > unsigned *color = shader->selector->color_attr_index;
>> >
>> > if (shader->key.part.ps.prolog.color_two_side) {
>> > /* BCOLORs are stored after the last input. */
>> > key->ps_prolog.num_interp_inputs = 
>> > info->num_inputs;
>> > key->ps_prolog.face_vgpr_index = 
>> > shader->info.face_vgpr_index;
>> > -   shader->config.spi_ps_input_ena |= 
>> > S_0286CC_FRONT_FACE_ENA(1);
>> > +   if (separate_prolog)
>> > +   shader->config.spi_ps_input_ena |= 
>> > S_0286CC_FRONT_FACE_ENA(1);
>> > }
>> >
>> > for (unsigned i = 0; i < 2; i++) {
>> > unsigned interp = 
>> > info->input_interpolate[color[i]];
>> > unsigned location = 
>> > info->input_interpolate_loc[color[i]];
>> >
>> > if (!(info->colors_read & (0xf << i*4)))
>> > continue;
>> >
>> > key->ps_prolog.color_attr_index[i] = color[i];
>> > @@ -6159,66 +6160,78 @@ static void si_get_ps_prolog_key(struct si_shader 
>> > *shader,
>> > case TGSI_INTERPOLATE_COLOR:
>> > /* Force the interpolation location for 
>> > colors here. */
>> > if 
>> > (shader->key.part.ps.prolog.force_persp_sample_interp)
>> > location = 
>> > TGSI_INTERPOLATE_LOC_SAMPLE;
>> > if 
>> > (shader->key.part.ps.prolog.force_persp_center_interp)
>> > location = 
>> > TGSI_INTERPOLATE_LOC_CENTER;
>> >
>> > switch (location) {
>> > case TGSI_INTERPOLATE_LOC_SAMPLE:
>> > 
>> > key->ps_prolog.color_interp_vgpr_index[i] = 0;
>> > -   shader->config.spi_ps_input_ena |=
>> > -   
>> > S_0286CC_PERSP_SAMPLE_ENA(1);
>> > +   if (separate_prolog) {
>> > +   
>> > shader->config.spi_ps_input_ena |=
>> > +   
>> > S_0286CC_PERSP_SAMPLE_ENA(1);
>> > +   }
>> > break;
>> > case TGSI_INTERPOLATE_LOC_CENTER:
>> > 
>> > key->ps_prolog.color_interp_vgpr_index[i] = 2;
>> > -   shader->config.spi_ps_input_ena |=
>> > -   
>> > S_0286CC_PERSP_CENTER_ENA(1);
>> > +   if (separate_prolog) {
>> > +   
>> > shader->config.spi_ps_input_ena |=
>> > +   
>> > S_0286CC_PERSP_CENTER_ENA(1);
>> > +   }
>> > break;
>> > case TGSI_INTERPOLATE_LOC_CENTROID:
>> > 
>> > key->ps_prolog.color_interp_vgpr_index[i] = 4;
>> > -   shader->config.spi_ps_input_ena |=
>> > -   
>> >

Re: [Mesa-dev] [PATCH 5/8] radeonsi: don't set spi_ps_input_* for monolithic shaders

2019-06-24 Thread Marek Olšák 
If the shader is monolithic, the value of SPI_PS_INPUT_ENA that is
generated by LLVM is used as-is.

Marek

On Fri, Jun 21, 2019 at 7:03 PM Bas Nieuwenhuizen 
wrote:

> Doesn't this cause assertions in si_shader_ps() for monolithic
> shaders? Some of these assertions check that at least one bit in a
> group is set and I think we end up with input_ena = 0 for monolithic
> shaders now?
>
> On Thu, Jun 20, 2019 at 6:20 AM Marek Olšák  wrote:
> >
> > From: Marek Olšák 
> >
> > The driver doesn't use these values and ac_rtld has assertions
> > expecting the value of 0.
> > ---
> >  src/gallium/drivers/radeonsi/si_shader.c | 39 
> >  1 file changed, 26 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/gallium/drivers/radeonsi/si_shader.c
> b/src/gallium/drivers/radeonsi/si_shader.c
> > index 54b29d0ae01..0489399b827 100644
> > --- a/src/gallium/drivers/radeonsi/si_shader.c
> > +++ b/src/gallium/drivers/radeonsi/si_shader.c
> > @@ -6128,21 +6128,22 @@ static void si_get_ps_prolog_key(struct
> si_shader *shader,
> >  key->ps_prolog.states.bc_optimize_for_linear);
> > key->ps_prolog.ancillary_vgpr_index =
> shader->info.ancillary_vgpr_index;
> >
> > if (info->colors_read) {
> > unsigned *color = shader->selector->color_attr_index;
> >
> > if (shader->key.part.ps.prolog.color_two_side) {
> > /* BCOLORs are stored after the last input. */
> > key->ps_prolog.num_interp_inputs =
> info->num_inputs;
> > key->ps_prolog.face_vgpr_index =
> shader->info.face_vgpr_index;
> > -   shader->config.spi_ps_input_ena |=
> S_0286CC_FRONT_FACE_ENA(1);
> > +   if (separate_prolog)
> > +   shader->config.spi_ps_input_ena |=
> S_0286CC_FRONT_FACE_ENA(1);
> > }
> >
> > for (unsigned i = 0; i < 2; i++) {
> > unsigned interp =
> info->input_interpolate[color[i]];
> > unsigned location =
> info->input_interpolate_loc[color[i]];
> >
> > if (!(info->colors_read & (0xf << i*4)))
> > continue;
> >
> > key->ps_prolog.color_attr_index[i] = color[i];
> > @@ -6159,66 +6160,78 @@ static void si_get_ps_prolog_key(struct
> si_shader *shader,
> > case TGSI_INTERPOLATE_COLOR:
> > /* Force the interpolation location for
> colors here. */
> > if
> (shader->key.part.ps.prolog.force_persp_sample_interp)
> > location =
> TGSI_INTERPOLATE_LOC_SAMPLE;
> > if
> (shader->key.part.ps.prolog.force_persp_center_interp)
> > location =
> TGSI_INTERPOLATE_LOC_CENTER;
> >
> > switch (location) {
> > case TGSI_INTERPOLATE_LOC_SAMPLE:
> >
>  key->ps_prolog.color_interp_vgpr_index[i] = 0;
> > -   shader->config.spi_ps_input_ena
> |=
> > -
>  S_0286CC_PERSP_SAMPLE_ENA(1);
> > +   if (separate_prolog) {
> > +
>  shader->config.spi_ps_input_ena |=
> > +
>  S_0286CC_PERSP_SAMPLE_ENA(1);
> > +   }
> > break;
> > case TGSI_INTERPOLATE_LOC_CENTER:
> >
>  key->ps_prolog.color_interp_vgpr_index[i] = 2;
> > -   shader->config.spi_ps_input_ena
> |=
> > -
>  S_0286CC_PERSP_CENTER_ENA(1);
> > +   if (separate_prolog) {
> > +
>  shader->config.spi_ps_input_ena |=
> > +
>  S_0286CC_PERSP_CENTER_ENA(1);
> > +   }
> > break;
> > case TGSI_INTERPOLATE_LOC_CENTROID:
> >
>  key->ps_prolog.color_interp_vgpr_index[i] = 4;
> > -   shader->config.spi_ps_input_ena
> |=
> > -
>  S_0286CC_PERSP_CENTROID_ENA(1);
> > +   if (separate_prolog) {
> > +
>  shader->config.spi_ps_input_ena |=
> > +
>  S_0286CC_PERSP_CENTROID_ENA(1);
> > +   }
> > break;
> > default:
> > assert(0);
> > }
> > break;
> > case TGSI_INTERPOLATE_LINEAR:
> > /* Force the interpolation location for
> colors here. */
> > if
> (shader->key.part.ps.prolog.force_linear_sample_interp)
> > location =
> TGSI_INTERPOLATE_LOC_SAMPLE;
>

Re: [Mesa-dev] [PATCH 5/8] radeonsi: don't set spi_ps_input_* for monolithic shaders

2019-06-21 Thread Bas Nieuwenhuizen 
Doesn't this cause assertions in si_shader_ps() for monolithic
shaders? Some of these assertions check that at least one bit in a
group is set and I think we end up with input_ena = 0 for monolithic
shaders now?

On Thu, Jun 20, 2019 at 6:20 AM Marek Olšák  wrote:
>
> From: Marek Olšák 
>
> The driver doesn't use these values and ac_rtld has assertions
> expecting the value of 0.
> ---
>  src/gallium/drivers/radeonsi/si_shader.c | 39 
>  1 file changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
> b/src/gallium/drivers/radeonsi/si_shader.c
> index 54b29d0ae01..0489399b827 100644
> --- a/src/gallium/drivers/radeonsi/si_shader.c
> +++ b/src/gallium/drivers/radeonsi/si_shader.c
> @@ -6128,21 +6128,22 @@ static void si_get_ps_prolog_key(struct si_shader 
> *shader,
>  key->ps_prolog.states.bc_optimize_for_linear);
> key->ps_prolog.ancillary_vgpr_index = 
> shader->info.ancillary_vgpr_index;
>
> if (info->colors_read) {
> unsigned *color = shader->selector->color_attr_index;
>
> if (shader->key.part.ps.prolog.color_two_side) {
> /* BCOLORs are stored after the last input. */
> key->ps_prolog.num_interp_inputs = info->num_inputs;
> key->ps_prolog.face_vgpr_index = 
> shader->info.face_vgpr_index;
> -   shader->config.spi_ps_input_ena |= 
> S_0286CC_FRONT_FACE_ENA(1);
> +   if (separate_prolog)
> +   shader->config.spi_ps_input_ena |= 
> S_0286CC_FRONT_FACE_ENA(1);
> }
>
> for (unsigned i = 0; i < 2; i++) {
> unsigned interp = info->input_interpolate[color[i]];
> unsigned location = 
> info->input_interpolate_loc[color[i]];
>
> if (!(info->colors_read & (0xf << i*4)))
> continue;
>
> key->ps_prolog.color_attr_index[i] = color[i];
> @@ -6159,66 +6160,78 @@ static void si_get_ps_prolog_key(struct si_shader 
> *shader,
> case TGSI_INTERPOLATE_COLOR:
> /* Force the interpolation location for 
> colors here. */
> if 
> (shader->key.part.ps.prolog.force_persp_sample_interp)
> location = 
> TGSI_INTERPOLATE_LOC_SAMPLE;
> if 
> (shader->key.part.ps.prolog.force_persp_center_interp)
> location = 
> TGSI_INTERPOLATE_LOC_CENTER;
>
> switch (location) {
> case TGSI_INTERPOLATE_LOC_SAMPLE:
> 
> key->ps_prolog.color_interp_vgpr_index[i] = 0;
> -   shader->config.spi_ps_input_ena |=
> -   S_0286CC_PERSP_SAMPLE_ENA(1);
> +   if (separate_prolog) {
> +   
> shader->config.spi_ps_input_ena |=
> +   
> S_0286CC_PERSP_SAMPLE_ENA(1);
> +   }
> break;
> case TGSI_INTERPOLATE_LOC_CENTER:
> 
> key->ps_prolog.color_interp_vgpr_index[i] = 2;
> -   shader->config.spi_ps_input_ena |=
> -   S_0286CC_PERSP_CENTER_ENA(1);
> +   if (separate_prolog) {
> +   
> shader->config.spi_ps_input_ena |=
> +   
> S_0286CC_PERSP_CENTER_ENA(1);
> +   }
> break;
> case TGSI_INTERPOLATE_LOC_CENTROID:
> 
> key->ps_prolog.color_interp_vgpr_index[i] = 4;
> -   shader->config.spi_ps_input_ena |=
> -   
> S_0286CC_PERSP_CENTROID_ENA(1);
> +   if (separate_prolog) {
> +   
> shader->config.spi_ps_input_ena |=
> +   
> S_0286CC_PERSP_CENTROID_ENA(1);
> +   }
> break;
> default:
> assert(0);
> }
> break;
> case TGSI_INTERPOLATE_LINEAR:
> /* Force the

[Mesa-dev] [PATCH 5/8] radeonsi: don't set spi_ps_input_* for monolithic shaders

2019-06-19 Thread Marek Olšák 
From: Marek Olšák 

The driver doesn't use these values and ac_rtld has assertions
expecting the value of 0.
---
 src/gallium/drivers/radeonsi/si_shader.c | 39 
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_shader.c 
b/src/gallium/drivers/radeonsi/si_shader.c
index 54b29d0ae01..0489399b827 100644
--- a/src/gallium/drivers/radeonsi/si_shader.c
+++ b/src/gallium/drivers/radeonsi/si_shader.c
@@ -6128,21 +6128,22 @@ static void si_get_ps_prolog_key(struct si_shader 
*shader,
 key->ps_prolog.states.bc_optimize_for_linear);
key->ps_prolog.ancillary_vgpr_index = shader->info.ancillary_vgpr_index;
 
if (info->colors_read) {
unsigned *color = shader->selector->color_attr_index;
 
if (shader->key.part.ps.prolog.color_two_side) {
/* BCOLORs are stored after the last input. */
key->ps_prolog.num_interp_inputs = info->num_inputs;
key->ps_prolog.face_vgpr_index = 
shader->info.face_vgpr_index;
-   shader->config.spi_ps_input_ena |= 
S_0286CC_FRONT_FACE_ENA(1);
+   if (separate_prolog)
+   shader->config.spi_ps_input_ena |= 
S_0286CC_FRONT_FACE_ENA(1);
}
 
for (unsigned i = 0; i < 2; i++) {
unsigned interp = info->input_interpolate[color[i]];
unsigned location = 
info->input_interpolate_loc[color[i]];
 
if (!(info->colors_read & (0xf << i*4)))
continue;
 
key->ps_prolog.color_attr_index[i] = color[i];
@@ -6159,66 +6160,78 @@ static void si_get_ps_prolog_key(struct si_shader 
*shader,
case TGSI_INTERPOLATE_COLOR:
/* Force the interpolation location for colors 
here. */
if 
(shader->key.part.ps.prolog.force_persp_sample_interp)
location = TGSI_INTERPOLATE_LOC_SAMPLE;
if 
(shader->key.part.ps.prolog.force_persp_center_interp)
location = TGSI_INTERPOLATE_LOC_CENTER;
 
switch (location) {
case TGSI_INTERPOLATE_LOC_SAMPLE:

key->ps_prolog.color_interp_vgpr_index[i] = 0;
-   shader->config.spi_ps_input_ena |=
-   S_0286CC_PERSP_SAMPLE_ENA(1);
+   if (separate_prolog) {
+   shader->config.spi_ps_input_ena 
|=
+   
S_0286CC_PERSP_SAMPLE_ENA(1);
+   }
break;
case TGSI_INTERPOLATE_LOC_CENTER:

key->ps_prolog.color_interp_vgpr_index[i] = 2;
-   shader->config.spi_ps_input_ena |=
-   S_0286CC_PERSP_CENTER_ENA(1);
+   if (separate_prolog) {
+   shader->config.spi_ps_input_ena 
|=
+   
S_0286CC_PERSP_CENTER_ENA(1);
+   }
break;
case TGSI_INTERPOLATE_LOC_CENTROID:

key->ps_prolog.color_interp_vgpr_index[i] = 4;
-   shader->config.spi_ps_input_ena |=
-   S_0286CC_PERSP_CENTROID_ENA(1);
+   if (separate_prolog) {
+   shader->config.spi_ps_input_ena 
|=
+   
S_0286CC_PERSP_CENTROID_ENA(1);
+   }
break;
default:
assert(0);
}
break;
case TGSI_INTERPOLATE_LINEAR:
/* Force the interpolation location for colors 
here. */
if 
(shader->key.part.ps.prolog.force_linear_sample_interp)
location = TGSI_INTERPOLATE_LOC_SAMPLE;
if 
(shader->key.part.ps.prolog.force_linear_center_interp)
location = TGSI_INTERPOLATE_LOC_CENTER;
 
/* The VGPR assignment for non-monolithic