[Mesa-dev] Patchwork/mesa-stable question (was: Re: [PATCH v3] r600g: Fix special negative immediate constants when using ABS modifier.)

[Ilia Mirkin]

On Thu, Oct 29, 2015 at 7:11 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 29.10.2015 10:24, Ivan Kalvachev wrote:
> [snip]
>>
>> On 10/29/15, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>>
>>> On 29.10.2015 01:52, Ivan Kalvachev wrote:
>>>>
>>>> On 10/26/15, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>>>>>
>>>>> 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.
>>>
>>>
>>> I see. I guess it's not too surprising that Nine creates shaders that
>>> look a bit different from the Mesa statetracker's.
>>>
>>> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>
>>> This should probably also go to stable.
>>>
>>> Do you need somebody to push this for you or can you do it yourself?
>>>
>>> Cheers,
>>> Nicolai
>>
>>
>> Yes, please.
>> I'm not developer and I cannot push it myself.
>
>
> I pushed the patch.
>
> I am not familiar with patchwork yet and have a related question: on my
> push, I got the following error message related to patchwork:
>
> remote: E: failed to find patch for rev
> f75f21a24ae2dd83507f3d4d8007f0fcfe6db802
>
> Apparently, patchwork didn't pick up Ivan's v3 patch,

Now you know why there are so many stale patches in patchwork... if
the diff part of the commit doesn't match 100% to something in
patchwork, the search fails.

> perhaps because it
> wasn't inline. Is this something to worry about? Specifically, I believe the
> patch is a candidate for the stable branch, and I added the appropriate Cc:
> in the commit message. Does the message above prevent it from being picked
> up?

Nope, all's well. Assuming that the thing you wanted to get pushed has
been. Patchwork isn't part of any "official" process, just a
convenience tool.

  -ilia