System Environment: -------------------------- Arch: i386 Platform: surgabay Libdrm: (master)2.4.33-7-g2f1e2101b4dc0a6dc1c1d1c59c5cc5fbc54b90cf Mesa: (master)bd2410b48df261251f75c2c69785c8cc3182d94d Xserver: (master)xorg-server-1.12.0-50-g641a1b9363d59808d2586f9e84847ccc69701482 Xf86_video_intel: (master)2.18.0-171-g98ad4c3cd8647ba3ec90fb45157773c8e85e886c Libva: (vaapi-ext)f12f80371fb534e6bbf248586b3c17c298a31f4e Libva_intel_driver:(vaapi-ext)82fa52510a37ab645daaa3bb7091ff5096a20d0b Kernel: (drm-intel-next-queued) 9c33f85cf3cc3f8717e56d42fd678573ed1727d3 Bug detailed description: ------------------------- It fails on Sandybridge and Calpella with mesa master branch.It doesn't happen on mesa 8.0 branch. Case 'vertexshader(advanced.TestMaterials)' and 'vertexshader(advanced.TestLightsTwoSided)' also fail with same commit. Bisect shows: 0405bd08ca0e01ebc68891ee1ff47d320983f775 is the first bad commit commit 0405bd08ca0e01ebc68891ee1ff47d320983f775 Author: Kenneth Graunke <kenneth@whitecape.org> Date: Wed Mar 28 14:00:42 2012 -0700 glsl: Don't trust loop analysis in the presence of function calls. Function calls may have side effects that alter variables used inside the loop. In the fragment shader, they may even terminate the shader. This means our analysis about loop-constant or induction variables may be completely wrong. In general it's impossible to determine whether they actually do or not (due to the halting problem), so we'd need to perform conservative static analysis. For now, it's not worth the complexity: most functions will be inlined, at which point we can unroll them successfully. Fixes Piglit tests: - shaders/glsl-fs-unroll-out-param - shaders/glsl-fs-unroll-side-effect NOTE: This is a candidate for release branches. Signed-off-by: Kenneth Graunke <kenneth@whitecape.org> Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> Reproduce steps: ---------------- 1. start X 2. ./oglconform -z -s -suite all -v 2 -D 117 -test vertexshader advanced.TestLights
The test is failing because the vertex shader fails to compile---we run out of registers and haven't implemented VS spilling yet. The loop of interest is: void main() { int i; for (i = 0; i < gl_MaxLights; i++) { ...do various function calls, passing i as an 'in' parameter... } } Previously, we would unroll this right away, ignoring the function calls completely. They would later get inlined. This resulted in no loop. Now, we delay unrolling until after function inlining and other optimizations. It looks like something is rewriting the code enough that loop analysis gets confused, and fails to unroll the loop. This then triggers an issue in the i965/vs backend, where our register liveness analysis within loops is basically "uh, it must be alive". So we run out of registers. And then we don't support spilling. So it doesn't compile. And then nothing works. I see several solutions to this bug, all of which sound difficult: 1. Make loop analysis smarter about function calls. In this case, since the induction variable 'i' is local to main(), and it's only ever passed as an 'in' parameter, it isn't possible for any of the function calls to alter it. So our analysis is safe here. 2. Make loop analysis/unrolling smarter in general. It ought to be able to unroll the loop even after function inlining. We already know that the unroller is hackish, and had planned to replace it at some point. It's just been ZBB'd repeatedly. 3. Make i965's liveness interval computations smarter in the presence of loops. The backend really ought to be able to figure this out. 4. Implement register spilling in the VS backend. This should be simple enough and is worth doing. It would get the test passing again. But there would still be a large code quality regression; spilling is bad and should be avoided at almost any cost.
It fails on Sandybridge, Ivybridge and Ironlake with mesa master branch.It doesn't happen on mesa 8.0 branch.
It also happens on Pineview with mase master branch.
It also happens on mesa 9.0 branch(commit:6886da783ac2fc549b4ffc1f4)
Unfortunately, it's too late in the release process to fix this for 9.0. Perhaps it can be fixed in a subsequent 9.0.x release. It doesn't make sense to revert the patch because it fixes a critical bug where we generated completely incorrect code for certain shaders. Plus, reverting it would regress other Piglit tests...
Changes from crash to fail with my VS live intervals code. Having read through about half the dump, the register allocation is looking reasonable so far and there's no spilling, so I'm wondering if there may be other issues with the change from unrolled to looping. One significant change is the static array indexing with unrolled. That code also looks pretty good to me, other than a small note that uniform_vector_size[] is 4 for the scalar components of the structure because of the hilariously broken last_swiz test in setup_builtin_uniforms(). I don't think that's hurting us, though.
Now that the state of this testcase has changed significantly as of: commit 20ebebac5153affcbd44350332678a2fb04d4c96 Author: Eric Anholt <eric@anholt.net> Date: Wed Oct 3 13:44:12 2012 -0700 i965/vs: Improve live interval calculation. I've replaced this bug report with bug #56104 because I think it's just revealing an existing underlying issue (variable-indexed array access on builtin uniforms? What are the chances there's a bug there?) and all the spilling discussion here will just distract us.
It still fails on commit 6c281749696da417a88c3d1ee460f642f92a4cee).
Yes, I said it was still failing, and I'm moving the remaining, non-regression bug handling to a fresh bug report so that we don't have to wade through the fixed issue when looking at hte bug report for this testcase.
Fixed on master branch by commit 7e28d6c1ab101894c3a3c90bef929c34e9da6148. commit 7e28d6c1ab101894c3a3c90bef929c34e9da6148 Author: Eric Anholt <eric@anholt.net> AuthorDate: Wed Nov 21 14:30:55 2012 -0800 Commit: Eric Anholt <eric@anholt.net> CommitDate: Fri Dec 28 10:53:50 2012 -0800 i965: Consistently use nr_pull_params instead of NumParameters. NumParameters used to be an upper bound on the number of vec4s to be uploaded, which was basically safe (unless your buffer was bound near the top of address space *and* you array indexed outside the buffer, in which case I think you might GPU hang). As I migrate the driver away from ParameterValues[], this is no longer true.
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.