Bug 100785 - [regression, bisected] arb_gpu_shader5 piglit fail
Summary: [regression, bisected] arb_gpu_shader5 piglit fail
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/r600 (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Default DRI bug account
QA Contact: Default DRI bug account
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-25 11:30 UTC by Hi-Angel
Modified: 2017-06-26 07:58 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
ps,vs dump HEAD (31.49 KB, text/plain)
2017-04-26 07:46 UTC, Hi-Angel
Details
ps,vs dump HEAD with reverted eb8aa93c03 (30.73 KB, text/plain)
2017-04-26 07:47 UTC, Hi-Angel
Details
ps,vs,nosb dump HEAD (22.26 KB, text/plain)
2017-04-27 04:21 UTC, Hi-Angel
Details
ps,vs,nosb dump HEAD with reverted eb8aa93c03 (21.64 KB, text/plain)
2017-04-27 04:22 UTC, Hi-Angel
Details
fix (2.38 KB, patch)
2017-06-23 15:57 UTC, Hi-Angel
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Hi-Angel 2017-04-25 11:30:31 UTC
Failing tests:

	bin/arb_gpu_shader5-interpolateAtSample -auto -fbo
	bin/arb_gpu_shader5-interpolateAtSample-nonconst -auto -fbo

Bisection result:


eb8aa93c03ee89ffd3041d41b6293e4b282b6ce6 is the first bad commit
commit eb8aa93c03ee89ffd3041d41b6293e4b282b6ce6
Author: Timothy Arceri <tarceri@itsqueeze.com>
Date:   Fri Apr 14 16:25:58 2017 +1000

    glsl: disable varying packing for varying used by interpolateAt*
    
    Currently the NIR backends depend on GLSL IR copy propagation to
    fix up the interpolateAt* function params after varying packing
    changes the shader input to a global. It's possible copy propagation
    might not always do what we need it too, and we also shouldn't
    depend on optimisations to do this type of thing for us.
    
    I'm not sure if the same is true for TGSI, but the following
    commit should re-enable packing for most cases in a safer way,
    so we just disable it everywhere.
    
    No change in shader-db for i965 (BDW)
    
    Acked-by: Elie Tournier <elie.tournier@collabora.com>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

Reverting the commit fixes the problem.
Comment 1 Hi-Angel 2017-04-25 11:39:42 UTC
Ah, a fail output:

$ LD_LIBRARY_PATH=/home/constantine/Projects/mesa/lib LIBGL_DRIVERS_PATH=/home/constantine/Projects/mesa/lib/gallium /home/constantine/Projects/piglit/bin/arb_gpu_shader5-interpolateAtSample -auto -fbo
Probe color at (0,0)
  Expected: 0 255 0
  Observed: 20 195 0
PIGLIT: {"result": "fail" }
Comment 2 Timothy Arceri 2017-04-25 23:54:00 UTC
Does it work if you set:

R600_DEBUG=nosb

Also it would be helpful if you could attach a before and after shader dump using.

R600_DEBUG=vs,ps
Comment 3 Hi-Angel 2017-04-26 07:45:38 UTC
(In reply to Timothy Arceri from comment #2)
> Does it work if you set:
> 
> R600_DEBUG=nosb

Yeah, this way it does pass.

> Also it would be helpful if you could attach a before and after shader dump
> using.
> 
> R600_DEBUG=vs,ps

Attaching.
Comment 4 Hi-Angel 2017-04-26 07:46:48 UTC
Created attachment 131043 [details]
ps,vs dump HEAD
Comment 5 Hi-Angel 2017-04-26 07:47:27 UTC
Created attachment 131044 [details]
ps,vs dump HEAD with reverted eb8aa93c03
Comment 6 Timothy Arceri 2017-04-26 22:49:10 UTC
Thanks. Looks like a bug in sb optimisations so the output of  R600_DEBUG=vs,ps,nosb on HEAD would also be useful.
Comment 7 Hi-Angel 2017-04-27 04:21:53 UTC
Created attachment 131062 [details]
ps,vs,nosb dump HEAD
Comment 8 Hi-Angel 2017-04-27 04:22:21 UTC
Created attachment 131063 [details]
ps,vs,nosb dump HEAD with reverted eb8aa93c03
Comment 9 Hi-Angel 2017-04-27 04:25:43 UTC
Hmm, wait. Now for some reason it fails with nosb also.
Comment 10 Hi-Angel 2017-04-27 04:45:17 UTC
Not sure why, but I can't get it passing with "nosb" anymore :(
Comment 11 Hi-Angel 2017-04-27 05:09:48 UTC
So, actually there're 4 different combinations:

 						 | usual run | nosb  |
HEAD 					 |   fails   | passes|
HEAD eb8aa93c03 reverted |   passes  | fails |

With that said, I've attached dumps for all 4 configurations. But I do actually recall that "nosb" option was buggy, see this https://bugs.freedesktop.org/show_bug.cgi?id=93715
Comment 12 Hi-Angel 2017-04-27 05:12:12 UTC
(EDIT: replaced tabs with space in the table)

So, actually there're 4 different combinations:

                         | usual run | nosb  |
HEAD                     |   fails   | passes|
HEAD eb8aa93c03 reverted |   passes  | fails |

With that said, I've attached dumps for all 4 configurations. But I do actually recall that "nosb" option was buggy, see this https://bugs.freedesktop.org/show_bug.cgi?id=93715
Comment 13 Hi-Angel 2017-06-19 10:33:55 UTC
Possible fix https://lists.freedesktop.org/archives/mesa-dev/2017-June/159872.html

It solves the problem, but needs more work to exclude other bugs of alike type.
Comment 14 Hi-Angel 2017-06-23 15:57:44 UTC
Created attachment 132157 [details] [review]
fix

FTR, attaching the fix. However I don't know when I get it sent to the ML because I can't finish piglit testing — GPU hangs.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.