Re: [Mesa-dev] [PATCH v3] r600g: Fix special negative immediate constants when using ABS modifier.
On 29 October 2015 at 09:24, Ivan Kalvachevwrote: > On 10/29/15, Ilia Mirkin wrote: >> On Wed, Oct 28, 2015 at 8:52 PM, Ivan Kalvachev >> wrote: >>> I'm attaching v3 of the patch. Same as v2, but without the extra empty >>> line. >> >> FYI, there's a lot of overhead to reviewing an attached patch (in >> fact, gmail makes it extra difficult since you can't even see the >> thing in the name of protection from the unknown). Please use 'git >> send-email' (or equivalent mechanism) to make the patches appear >> inline. >> >> Cheers, >> >> -ilia > > Sorry about that. > I did inline my v1 patch, to easy the review. > > Configuring smtp server is too much hassle for a single patch and I > would like to avoid writing my email credentials if possible. > If I'm about to send more patches, then I guess I would have to do that. > > I suspect that it is not possible to send mail directly to > lists.freedesktop.org mail server, is it? > Setting things up it's as trivial as adding the following lines to your $HOME/.config/git/config [sendemail] smtpuser = ikalvac...@gmail.com smtpencryption = tls smtpserverport = 587 smtpserver = smtp.gmail.com For ease of use you can also set the TO address in $(mesa_top)/.git/config. You can also move the above setup in here. [sendemail] to = mesa-dev@lists.freedesktop.org -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] r600g: Fix special negative immediate constants when using ABS modifier.
Am 29.10.2015 um 10:24 schrieb Ivan Kalvachev: > Configuring smtp server is too much hassle for a single patch and I > would like to avoid writing my email credentials if possible. > If I'm about to send more patches, then I guess I would have to do that. You might want to look into "git imap-send" and/or "git format-patch --stdout" so git can generate emails which you can send with an email client like thunderbird. That way you also get a nice way to "preview" the messages/patches you are going to send. Felix ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] r600g: Fix special negative immediate constants when using ABS modifier.
On 29.10.2015 01:52, Ivan Kalvachev wrote: -- Forwarded message -- From: Ivan KalvachevDate: 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 On 10/26/15, Nicolai Hähnle 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. 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 This should probably also go to stable. Do you need somebody to push this for you or can you do it yourself? Cheers, Nicolai Signed-off-by: Ivan Kalvachev --- 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] r600g: Fix special negative immediate constants when using ABS modifier.
On 10/29/15, Ilia Mirkinwrote: > On Wed, Oct 28, 2015 at 8:52 PM, Ivan Kalvachev > wrote: >> I'm attaching v3 of the patch. Same as v2, but without the extra empty >> line. > > FYI, there's a lot of overhead to reviewing an attached patch (in > fact, gmail makes it extra difficult since you can't even see the > thing in the name of protection from the unknown). Please use 'git > send-email' (or equivalent mechanism) to make the patches appear > inline. > > Cheers, > > -ilia Sorry about that. I did inline my v1 patch, to easy the review. Configuring smtp server is too much hassle for a single patch and I would like to avoid writing my email credentials if possible. If I'm about to send more patches, then I guess I would have to do that. I suspect that it is not possible to send mail directly to lists.freedesktop.org mail server, is it? On 10/29/15, Nicolai Hähnle wrote: > On 29.10.2015 01:52, Ivan Kalvachev wrote: >> -- Forwarded message -- >> From: Ivan Kalvachev >> 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 >> >> On 10/26/15, Nicolai Hähnle 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. > > 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 > > 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. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v3] r600g: Fix special negative immediate constants when using ABS modifier.
On Wed, Oct 28, 2015 at 8:52 PM, Ivan Kalvachevwrote: > I'm attaching v3 of the patch. Same as v2, but without the extra empty line. FYI, there's a lot of overhead to reviewing an attached patch (in fact, gmail makes it extra difficult since you can't even see the thing in the name of protection from the unknown). Please use 'git send-email' (or equivalent mechanism) to make the patches appear inline. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev