https://bugs.freedesktop.org/show_bug.cgi?id=62696
Priority: medium Bug ID: 62696 Keywords: regression CC: deathsimple@vodafone.de Assignee: dri-devel@lists.freedesktop.org Summary: [r300g, bisected] Around 50 piglit vs variable-indexing tests fails after "glsl_to_tgsi: allocate arrays separately v2" Severity: normal Classification: Unclassified OS: All Reporter: pavel.ondracka@email.cz Hardware: Other Status: NEW Version: git Component: Drivers/Gallium/r300 Product: Mesa
Created attachment 76962 --> https://bugs.freedesktop.org/attachment.cgi?id=76962&action=edit RADEON_DEBUG=fp,vp output with latest mesa master
first bad commit: commit 3f67251e3d0ce61a0e7fc16de91de6fb49cad768 Author: Christian König christian.koenig@amd.com Date: Sun Mar 10 14:33:29 2013 +0100
glsl_to_tgsi: allocate arrays separately v2
Instead of allocating everything as temporaries, use the new array allocation functions.
v2: fix bug in simplify_cmp, declare arrays on demand
example test output: ./bin/shader_runner tests/spec/glsl-1.20/execution/variable-indexing/vs-temp-array-mat2-index-col-row-rd.shader_test -auto r300: DRM version: 2.29.0, Name: ATI RV530, ID: 0x71c5, GB: 1, Z: 2 r300: GART size: 509 MB, VRAM size: 256 MB r300: AA compression RAM: YES, Z compression RAM: YES, HiZ RAM: YES fallback: no matching format for GL_RGB, GL_FLOAT Probe at (10,10) Expected: 0.000000 1.000000 0.000000 Observed: 0.450980 0.549020 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (10,25) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (25,10) Expected: 0.000000 1.000000 0.000000 Observed: 0.450980 0.549020 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (25,25) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (50,10) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (50,25) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (65,10) Expected: 0.000000 1.000000 0.000000 Observed: 0.549020 0.450980 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (65,25) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (90,10) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (90,25) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (105,10) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 fallback: no matching format for GL_RGB, GL_FLOAT Probe at (105,25) Expected: 0.000000 1.000000 0.000000 Observed: 1.000000 0.000000 0.000000 PIGLIT: {'result': 'fail' }
(RADEON_DEBUG=fp,vp outputs before bad commit and with latest master attached)
GPU:RV530 Mesa:ec9c3882d949298366c872f766d3d18c6ae93f8e Kernel: 3.8.3-103.fc17.i686
https://bugs.freedesktop.org/show_bug.cgi?id=62696
--- Comment #1 from Pavel Ondračka pavel.ondracka@email.cz --- Created attachment 76963 --> https://bugs.freedesktop.org/attachment.cgi?id=76963&action=edit RADEON_DEBUG=fp,vp output before
https://bugs.freedesktop.org/show_bug.cgi?id=62696
--- Comment #2 from Christian König deathsimple@vodafone.de --- Created attachment 76967 --> https://bugs.freedesktop.org/attachment.cgi?id=76967&action=edit Possible fix
Please try the attached patch.
https://bugs.freedesktop.org/show_bug.cgi?id=62696
--- Comment #3 from Pavel Ondračka pavel.ondracka@email.cz --- Created attachment 76971 --> https://bugs.freedesktop.org/attachment.cgi?id=76971&action=edit RADEON_DEBUG=fp,vp from vs-uniform-array-mat2-col-row-rd test
(In reply to comment #2)
Created attachment 76967 [details] [review] Possible fix
Please try the attached patch.
Yes, your patch fixes the vs-temp-array-mat* tests and vs-varying-array-mat* tests. Still failing are some vs-uniform-array-mat* tests. I'm attaching output from one of those...
https://bugs.freedesktop.org/show_bug.cgi?id=62696
--- Comment #4 from Marek Olšák maraeo@gmail.com --- (In reply to comment #2)
Created attachment 76967 [details] [review] Possible fix
Please try the attached patch.
I thought the array support was backward compatible. If if were true, this patch wouldn't be needed, right?
https://bugs.freedesktop.org/show_bug.cgi?id=62696
Christian König deathsimple@vodafone.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |NEEDINFO
--- Comment #5 from Christian König deathsimple@vodafone.de --- (In reply to comment #4)
I thought the array support was backward compatible. If if were true, this patch wouldn't be needed, right?
Yes indeed, and I find that rather strange, too.
Well one thing that I haven't considered before is that when a target doesn't support indirect addressing all indirect accesses are replaced with a chain of "if (i == 0) then return x[0] else if (i == 1) then return x[1].....", so having arrays separated makes this inefficient code even more inefficient (cause the register scavenger then leaves arrays alone), that's what this patch really should fix.
But you're right, it should NEVER result in incorrect code, so we either have a bug in R300g that's triggered by using more registers than necessary (unlikely), or we are still missing something in the glsl_to_tgsi pass (likely).
@Pavel: Could you make sure that the vs-uniform-array-mat2-col-row-rd test is indeed failing with that change? And also please attach a log of the good case for this test.
Thanks in advance, Christian.
https://bugs.freedesktop.org/show_bug.cgi?id=62696
--- Comment #6 from Christian König deathsimple@vodafone.de --- Created attachment 76987 --> https://bugs.freedesktop.org/attachment.cgi?id=76987&action=edit Alternative fix.
Ok, I think I've figured out what's going wrong here.
Could you test the attached patch? It's an alternative version of the previous patch (and should NOT be applied on top).
Thanks, Christian.
https://bugs.freedesktop.org/show_bug.cgi?id=62696
--- Comment #7 from Pavel Ondračka pavel.ondracka@email.cz --- (In reply to comment #6)
Created attachment 76987 [details] Alternative fix.
Ok, I think I've figured out what's going wrong here.
Could you test the attached patch? It's an alternative version of the previous patch (and should NOT be applied on top).
Thanks, Christian.
Yes, the latest patch fixes all regressed tests here. Also no new regressions with quick piglit tests.
https://bugs.freedesktop.org/show_bug.cgi?id=62696
Christian König deathsimple@vodafone.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |RESOLVED Resolution|--- |FIXED
dri-devel@lists.freedesktop.org