| Summary: | [GLSL] fail to use >16 varying floats | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Gordon Jin <gordon.jin> |
| Component: | glsl-compiler | Assignee: | Ian Romanick <idr> |
| Status: | VERIFIED FIXED | QA Contact: | fangxun <xunx.fang> |
| Severity: | major | ||
| Priority: | medium | CC: | idr, stereotype441, tstellar, xunx.fang |
| Version: | git | ||
| Hardware: | All | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
new piglit test case
Failing shader from civ4 |
||
Created attachment 43282 [details]
new piglit test case
Currently the compiler doesn't do any packing of varyings. Any shader that uses more than 16 varyings will currently fail. This is probably fixable in the linker, but it's not currently on my schedule to fix. I expect that it will be non-trivial. The crash should be fixed by the commit below. This commit has also already been cherry picked to the 7.10 branch (85b965b). The test case will still fail because it won't link. This behavior is correct. commit de77324d8f14951e4dc17f570e49451a0cd33121 Author: Ian Romanick <ian.d.romanick@intel.com> Date: Thu Jun 9 13:31:32 2011 -0700 linker: Reject shaders that use too many varyings Previously it was up to the driver or later code generator to reject these shaders. It turns out that nobody did this. This will need changes to support geometry shaders. NOTE: This is a candidate for the stable branches. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37743 Reviewed-by: Kenneth Graunke <kenneth@whitecape.org> (In reply to comment #3) > The crash should be fixed by the commit below. This commit has also already > been cherry picked to the 7.10 branch (85b965b). The test case will still fail > because it won't link. This behavior is correct. Verified the crash gone and the test case fails now. Can you explain why this fail behavior is correct? Don't you think this test case valid? Ian, can you explain why the fail is correct? I had misunderstood the spec. I had initially thought that the same counting rules for vertex attributes applied for varyings. That is, the implementation is not required to do any packing. This is not the case, and we do need to fix this. *** Bug 43121 has been marked as a duplicate of this bug. *** Created attachment 53891 [details] Failing shader from civ4 Here is the civ4 shader from bug 43121 that fails with too many varyings. Is this shader legal according to the spec? (In reply to comment #8) > Created attachment 53891 [details] > Failing shader from civ4 > > Here is the civ4 shader from bug 43121 that fails with too many varyings. Is > this shader legal according to the spec? No, but also yes. The no part: The driver says that it supports only 32 varying components, but that shader, without additional optimizations, uses 36. The yes part: I don't think r300 should say that it only supports 32 varying components. I think it supports more than that. The value 32 comes from 8 texture coordinates times 4 components. However, it also has at least 2 color varying slots. If the fragment shader doesn't use gl_Color (or gl_SecondaryColor) those should be used as general varyings. I haven't verified this by looking at the r300 documentation, but Apple, who is even more anal about the spec than I am, has advertised 40 varying components on that hardware since forever. See http://developer.apple.com/graphicsimaging/opengl/capabilities/GLInfo_1058.html. The value 40 comes from 8 texture coordinates plus two colors times 4 components. The i915 driver currently has the same problem. (In reply to comment #9) > I don't think r300 should say that it only supports 32 varying components. I > think it supports more than that. The value 32 comes from 8 texture > coordinates times 4 components. However, it also has at least 2 color varying > slots. If the fragment shader doesn't use gl_Color (or gl_SecondaryColor) > those should be used as general varyings. > > I haven't verified this by looking at the r300 documentation, but Apple, who is > even more anal about the spec than I am, has advertised 40 varying components > on that hardware since forever. See > http://developer.apple.com/graphicsimaging/opengl/capabilities/GLInfo_1058.html. > The value 40 comes from 8 texture coordinates plus two colors times 4 > components. > I checked the docs, and this is correct. r300 supports 8 tex coords + 4 colors, so it should be able to do 40. I'll see if I can make this happen. Is this bug related to the glsl-max-varyings piglit regression on r300g since 1ded658ce074a85bc08c989ff17840b840ff3051, or should I open a new bug? I think this is fixed, at least on i965, now that Paul's packed varying series is complete. It passes here. (In reply to comment #12) > I think this is fixed, at least on i965, now that Paul's packed varying > series is complete. It passes here. Note that there are three separate (and IMHO unrelated) bugs being talked about here. The original bug report was a piglit failure on i965 that happened because Mesa didn't pack varyings. That bug was fixed by commit ca7e891e8aceaf1adb4c9ae776916fa3636747ee (glsl/linker: Pack between varyings.) Comments 7-10 were a failure in civ4 on r300, caused by the civ4 shader using more varying floats than r300 advertises. Mesa doesn't pack varyings on r300, and even if it did, there's no way this particular shader would be helped by varying packing. So this is an unrelated bug, not a duplicate as we originally suspected. Comment #10 suggests that this bug might be fixed now, and I don't have r300 hardware to test it. If it isn't fixed, please reopen bug 43121. In response to comment #11, the glsl-max-varyings piglit regression on r300g is definitely unrelated to this bug. I suspect it is also unrelated to bug 43121. I don't know if it is still present. If it is, I would recommend opening a fresh bug report for it. Marking this bug as fixed, since the originally reported issue has been fixed, and the issues in comments 7-11 need to be addressed in separate bug reports. Ian, can you commit the attached piglit test case (if not yet)? Xun, please help verify. Ian? Xun? Verified on latest mesa master and 10.2 branch. |
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.
The attached case uses 32 varying floats (actually only 17 varying floats could trigger this). It gets assertion with i965 driver and fails with swrast. piglit/bin/shader_runner -auto glsl-varying-32.shader_test libGL: OpenDriver: trying /opt/X11R7/lib/dri/i965_dri.so shader_runner: brw_vs_emit.c:1457: get_dst: Assertion `c->regs[dst.File][dst.Index].nr != 0' failed. Aborted (core dumped) (gdb) bt #0 0xb7fff424 in __kernel_vsyscall () #1 0x42c0fd71 in raise () from /lib/libc.so.6 #2 0x42c1164a in abort () from /lib/libc.so.6 #3 0x42c08de8 in __assert_fail () from /lib/libc.so.6 #4 0xb7a6c715 in get_dst (c=<value optimized out>, dst=...) at brw_vs_emit.c:1457 #5 0xb7a7160b in brw_vs_emit (c=0xbffd41dc) at brw_vs_emit.c:1936 #6 0xb7a6adeb in do_vs_prog (brw=0x806a900, vp=<value optimized out>, key=0xbfffee68) at brw_vs.c:85 #7 0xb7a6b02d in brw_upload_vs_prog (brw=0x806a900) at brw_vs.c:151 #8 0xb7a6a0c7 in brw_validate_state (brw=0x806a900) at brw_state_upload.c:400 #9 0xb7a59bb8 in brw_try_draw_prims (ctx=0x806a900, arrays=0x80ad518, prim=0xbfffefb8, nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=0, max_index=3) at brw_draw.c:362 #10 brw_draw_prims (ctx=0x806a900, arrays=0x80ad518, prim=0xbfffefb8, nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=0, max_index=3) at brw_draw.c:447 #11 0xb7b44354 in vbo_draw_arrays (ctx=0x806a900, mode=<value optimized out>, start=0, count=4, numInstances=1) at vbo/vbo_exec_array.c:588 #12 0x0804e988 in piglit_draw_rect (x=-1, y=-1, w=2, h=2) at /root/gordon/piglit/tests/util/piglit-util.c:748 #13 0x0804c910 in piglit_display () at /root/gordon/piglit/tests/shaders/shader_runner.c:724 # LIBGL_ALWAYS_SOFTWARE=1 piglit/bin/shader_runner -auto glsl-varying-32.shader_test libGL: OpenDriver: trying /opt/X11R7/lib/dri/swrast_dri.so Probe at (0,0) Expected: 0.320000 0.000000 0.000000 1.000000 Observed: 0.160784 0.000000 0.000000 1.000000 PIGLIT: {'result': 'fail' } The GLSL 1.10 spec mentions: const int gl_MaxVaryingFloats = 32; // ARB_vertex_shader I don't have history data to show if it's regression.