[Mesa-dev] [PATCH v3] r600g: Fix special negative immediate constants when using ABS modifier.

[Ivan Kalvachev]

---------- Forwarded message ----------
From: Ivan Kalvachev <ikalvachev at gmail.com>
Date: Wed, 28 Oct 2015 23:46:44 +0200
Subject: [PATCH v3] r600g: Fix special negative immediate constants
when using ABS modifier.
To: Nicolai Hähnle <nhaehnle at gmail.com>

On 10/26/15, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> Hi Ivan,
>
> On 25.10.2015 02:00, Ivan Kalvachev wrote:
>> Some constants (like 1.0 and 0.5) could be inlined as immediate inputs
>> without using their literal value. The r600_bytecode_special_constants()
>> function emulates the negative of these constants by using NEG modifier.
>>
>> However some shaders define -1.0 constant and want to use it as 1.0.
>> They do so by using ABS modifier. But r600_bytecode_special_constants()
>> set NEG in addition to ABS. Since NEG modifier have priority over ABS
>> one,
>> we get -|1.0| as result, instead of |1.0|.
>>
>> The patch simply prevents the additional switching of NEG when ABS is
>> set.
>
> Nice catch. Is there a simple test case (e.g. in piglit) that exposes
> the incorrect behavior?

Not that I know of.

I've located the bug investigating visual problem in Nine.
https://github.com/iXit/Mesa-3D/issues/126
https://github.com/iXit/Mesa-3D/issues/127

I also heard that it fixes artifacts in "Need for Speed: Undercover"
and "Skyrim", once again, when using Nine.

>> Signed-off-by: Ivan Kalvachev <ikalvachev at gmail.com>
>> ---
>>   src/gallium/drivers/r600/r600_asm.c    | 9 +++++----
>>   src/gallium/drivers/r600/r600_shader.c | 2 +-
>>   2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/gallium/drivers/r600/r600_asm.c
>> b/src/gallium/drivers/r600/r600_asm.c
>> index bc69806..8fc622c 100644
>> --- a/src/gallium/drivers/r600/r600_asm.c
>> +++ b/src/gallium/drivers/r600/r600_asm.c
>> @@ -635,8 +635,9 @@ static int replace_gpr_with_pv_ps(struct r600_bytecode
>> *bc,
>>          return 0;
>>   }
>>
>> -void r600_bytecode_special_constants(uint32_t value, unsigned *sel,
>> unsigned *neg)
>> +void r600_bytecode_special_constants(uint32_t value, unsigned *sel,
>> unsigned *neg, unsigned abs)
>>   {
>> +
>
> Please remove the extra whitespace line.
>
> Cheers,
> Nicolai
>

I'm attaching v3 of the patch. Same as v2, but without the extra empty line.

Best Regards
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-r600-Fix-special-negative-immediate-constants.v3.patch
Type: text/x-diff
Size: 3688 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151029/787a0191/attachment.patch>