[Mesa-dev] [PATCH 1/2] radeonsi: fix SPI_PS_INPUT_ENA handling

[Michel Dänzer]

On Fre, 2012-08-24 at 10:47 +0200, Christian König wrote: 
> On 23.08.2012 16:37, Michel Dänzer wrote:
> > On Mit, 2012-08-22 at 12:54 +0200, Christian König wrote:
> >> +	/* we need to enable at least one of them, otherwise we hang the GPU */
> >> +	if (!spi_ps_input_ena & (C_0286CC_PERSP_SAMPLE_ENA |
> >> +				 C_0286CC_PERSP_CENTROID_ENA |
> >> +				 C_0286CC_PERSP_PULL_MODEL_ENA |
> >> +				 C_0286CC_LINEAR_SAMPLE_ENA |
> >> +				 C_0286CC_LINEAR_CENTER_ENA |
> >> +				 C_0286CC_LINEAR_CENTROID_ENA |
> >> +				 C_0286CC_LINE_STIPPLE_TEX_ENA)) {
> >> +		spi_ps_input_ena |= S_0286CC_PERSP_SAMPLE_ENA(1);
> >> +	}
> > I just noticed that this causes a warning:
> >
> > si_state_draw.c: In function ‘si_pipe_shader_ps’:
> > si_state_draw.c:179:6: warning: suggest parentheses around operand of ‘!’ or change ‘&’ to ‘&&’ or ‘!’ to ‘~’ [-Wparentheses]
> >
> > Did you mean the following?
> >
> > if (!(spi_ps_input_ena & (~C_0286CC_PERSP_SAMPLE_ENA |
> >    ~C_0286CC_PERSP_CENTROID_ENA |
> >    ~C_0286CC_PERSP_PULL_MODEL_ENA |
> >    ~C_0286CC_LINEAR_SAMPLE_ENA |
> >    ~C_0286CC_LINEAR_CENTER_ENA |
> >    ~C_0286CC_LINEAR_CENTROID_ENA |
> >    ~C_0286CC_LINE_STIPPLE_TEX_ENA)) {
> > spi_ps_input_ena |= S_0286CC_PERSP_SAMPLE_ENA(1);
> > }
> 
> Oh yes indeed. But lets use the G_* variants instead. I think they are 
> better readable:
> 
>          if (!G_0286CC_PERSP_SAMPLE_ENA(spi_ps_input_ena) &&
>              !G_0286CC_PERSP_CENTROID_ENA(spi_ps_input_ena) &&
>              !G_0286CC_PERSP_PULL_MODEL_ENA(spi_ps_input_ena) &&
>              !G_0286CC_LINEAR_SAMPLE_ENA(spi_ps_input_ena) &&
>              !G_0286CC_LINEAR_CENTER_ENA(spi_ps_input_ena) &&
>              !G_0286CC_LINEAR_CENTROID_ENA(spi_ps_input_ena) &&
>              !G_0286CC_LINE_STIPPLE_TEX_ENA(spi_ps_input_ena)) {
> 
>                  spi_ps_input_ena |= S_0286CC_PERSP_SAMPLE_ENA(1);
>          }

I agree that would be more readable, but I'm afraid it might result in
generating inefficient machine code?

Maybe use (!(spi_ps_input_ena & (S_0286CC_*(1) | ...))).


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer