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

[Christian König]

On 24.08.2012 11:14, Michel Dänzer wrote:
> 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) | ...))).
>
>
I don't think so, and even if it generates inefficient machine code it's 
not a so performance critical section, since it just happens when we 
compile a pixel shader.