System Environment: -------------------------- Libdrm: (master)2.4.29-3-gef20301a11afae50bfe127002913dbd0b81ddccc Mesa: (master)009ac0618ccb8c1347c353b576442ed70592c20e Xserver: (master)xorg-server-1.11.99.2 Xf86_video_intel: (master)2.17.0-170-g1fa5721f064a8d1f34e4032b52f24597f4015313 Kernel: (drm-intel-next)9a10f401a401ca69c6537641c8fc0d6b57b5aee8 Bug detailed description: ------------------------- uint preincrementing and predecrementing in vertex shader failed to compile. refer to GLSL spec 1.3 page 39, GLSL 1.3 support preincrementing and predecrementing in vertex shader.
Created attachment 54595 [details] uint preincrementing and predecrementing case uint preincrementing and predecrementing case
(In reply to comment #1) > Created attachment 54595 [details] > uint preincrementing and predecrementing case > uint preincrementing and predecrementing case Ouping, have you tried with add a line of "#version 130" in your shader? Can you have a try?
after adding #version 130, uint preincrementing and predecrementing case still failed to compile. (In reply to comment #2) > (In reply to comment #1) > > Created attachment 54595 [details] > > uint preincrementing and predecrementing case > > uint preincrementing and predecrementing case > Ouping, have you tried with add a line of "#version 130" in your shader? Can > you have a try?
after adding "#version 130", uint preincrementing and predecrementing case still failed to compile. (In reply to comment #2) > (In reply to comment #1) > > Created attachment 54595 [details] > > uint preincrementing and predecrementing case > > uint preincrementing and predecrementing case > Ouping, have you tried with add a line of "#version 130" in your shader? Can > you have a try?
(In reply to comment #4) > after adding "#version 130", uint preincrementing and predecrementing case > still failed to compile. > (In reply to comment #2) > > (In reply to comment #1) > > > Created attachment 54595 [details] > > > uint preincrementing and predecrementing case > > > uint preincrementing and predecrementing case > > Ouping, have you tried with add a line of "#version 130" in your shader? Can > > you have a try? Can you have a double check?
Created attachment 54670 [details] glsl-op-uint(negative.uint.lvalue.pre_in-decrement) piglit case
After adding "#version 130" in vertex shader, case can compile successfully. but refer to the Spec 5.9 page 49, preincrementing and predecrementing do not give an l-value in vertex shader, so the case should failed to compile, but in fact, it can compile successfully. (In reply to comment #5) > (In reply to comment #4) > > after adding "#version 130", uint preincrementing and predecrementing case > > still failed to compile. > > (In reply to comment #2) > > > (In reply to comment #1) > > > > Created attachment 54595 [details] > > > > uint preincrementing and predecrementing case > > > > uint preincrementing and predecrementing case > > > Ouping, have you tried with add a line of "#version 130" in your shader? Can > > > you have a try? > Can you have a double check?
I'm really confused. First the bug was that the shader didn't compile, and now the bug is that the shader does compile. Can somebody please straighten out what the expected behavior is? Using this shader: #version 130 uniform uint u; void main() { uint x = 0; x++ = u; gl_Position = vec4(x); } With piglit's 'glslparsertest foo.vert fail', I get: Failed to compile vertex shader /tmp/foo.vert: 0:6(7): error: type mismatch 0:8(3): error: assignment to read-only variable '_post_incdec_tmp' 0:8(3): error: type mismatch PIGLIT: {'result': 'pass' } With shader-runner test in attachment #1 [details] [review], I get Failed to compile VS: 0:5(3): error: assignment to read-only variable '_post_incdec_tmp' 0:6(3): error: assignment to read-only variable '_post_incdec_tmp' PIGLIT: {'result': 'fail' } The error message is a bit silly, but the behavior is correct. Specifically, page 53 of the GLSL 1.30 spec says: "L-values must be writable. Variables that are built-in types, entire structures or arrays, structure fields, l-values with the field selector ( . ) applied to select components or swizzles without repeated fields, l-values within parentheses, and l-values dereferenced with the array subscript operator ( [ ] ) are all l-values. Other binary or unary expressions, function names, swizzles with repeated fields, and constants cannot be l-values."
run the attached piglit cases(I attached two new piglit cases), they failed to compile the unit syntax because I forgot to add the "#version 130" in vertex shader. After adding the "#version 130", it can pass to compile, reference to Spec, preincrementing and predecrementing do not give an l-value in vertex shader. preincrementing and predecrementing shader: [require] GLSL >= 1.30 [vertex shader] #version 130 void main() { uint x = 1u; ++x = 2u; --x = 2u; gl_Position = gl_Vertex; } Expected Result: it failed to compile. But in fact it pass to compile. post-incrementing and post-decrementing shader: [require] GLSL >= 1.30 [vertex shader] #version 130 void main() { uint x = 1u; x++ = 2u; x-- = 2u; gl_Position = gl_Vertex; } Expected Result: it failed to compile. (In reply to comment #8) > I'm really confused. First the bug was that the shader didn't compile, and now > the bug is that the shader does compile. Can somebody please straighten out > what the expected behavior is? > Using this shader: > #version 130 > uniform uint u; > void main() > { > uint x = 0; > x++ = u; > gl_Position = vec4(x); > } > With piglit's 'glslparsertest foo.vert fail', I get: > Failed to compile vertex shader /tmp/foo.vert: 0:6(7): error: type mismatch > 0:8(3): error: assignment to read-only variable '_post_incdec_tmp' > 0:8(3): error: type mismatch > PIGLIT: {'result': 'pass' } > With shader-runner test in attachment #1 [details] [review] [review], I get > Failed to compile VS: 0:5(3): error: assignment to read-only variable > '_post_incdec_tmp' > 0:6(3): error: assignment to read-only variable '_post_incdec_tmp' > PIGLIT: {'result': 'fail' } > The error message is a bit silly, but the behavior is correct. Specifically, > page 53 of the GLSL 1.30 spec says: > "L-values must be writable. Variables that are built-in types, entire > structures or arrays, structure fields, l-values with the field selector > ( . ) applied to select components or swizzles without repeated fields, > l-values within parentheses, and l-values dereferenced with the array > subscript operator ( [ ] ) are all l-values. Other binary or unary > expressions, function names, swizzles with repeated fields, and > constants cannot be l-values."
Created attachment 54736 [details] preincrementing and predecrementing
Created attachment 54737 [details] preincrementing and predecrementing
Created attachment 54738 [details] post-incrementing and post-decrementing
It looks like this happens with other types (e.g., float) and is not specific to GLSL 1.30.
I just posted a complete set of tests to the piglit mailing list. I also posted a series of patches to fix the bug to the mesa-dev mailing list. Could you please retest with the Mesa patches? Thanks.
This should be fixed in Mesa by the following commit series. commit 1f125374e73d1718cd54c946826c76b09998c055 Author: Ian Romanick <ian.d.romanick@intel.com> Date: Fri Dec 23 10:59:38 2011 -0800 glsl: Don't mark assignment temporaries as read-only The various l-value errors this was designed to catch are now caught by other means. Marking the temporaries as read-only now just prevents sensible error messages from being generated. It's 0:0(0): error: function parameter 'out p' references the read-only variable versus 0:13(5): error: function parameter 'out p' references a post-decrement opera Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Reviewed-by: Paul Berry <stereotype441@gmail.com> commit 208b5b113faab324854d457c3ad4abf05cafedd1 Author: Ian Romanick <ian.d.romanick@intel.com> Date: Fri Dec 23 10:58:23 2011 -0800 glsl: Emit extra errors for l-value violations in 'out' or 'inout' parameter Somethings, like pre-increment operations, were not previously caught. After the 8.0 release, this code needs some major refactoring and clean-up. It's a mess. :( Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Reviewed-by: Paul Berry <stereotype441@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42755 commit e9015e99d0d702570dbd3d7bcbafbf6c723cb7a7 Author: Ian Romanick <ian.d.romanick@intel.com> Date: Fri Dec 23 09:56:29 2011 -0800 glsl: Emit errors for assignments to non-l-value expressions Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Reviewed-by: Paul Berry <stereotype441@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42755 commit fa0a9ac5cdf49865cfc289c4326c73c9dd4a61c5 Author: Ian Romanick <ian.d.romanick@intel.com> Date: Fri Dec 23 09:56:03 2011 -0800 glsl: Track descriptions of some expressions that can't be l-values Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Reviewed-by: Paul Berry <stereotype441@gmail.com> In addition, I have committed a series of more exhaustive test cases to piglit. commit 025508171e209f730a035106799529c6fe2fcd8a Author: Ian Romanick <ian.d.romanick@intel.com> Date: Mon Jan 9 10:18:13 2012 -0800 glsl-1.10: Verify pre- and post-inc/dec cannot be l-values These generators create two kinds of tests. The first verifies that pre- and post-increment/decrement operations cannot be assignment targets. These look like: uniform float u; varying vec4 v; void main() { float t = u; ++t = float(v.x); gl_FragColor = vec4(t, v.yzw); } The second kind of test verifies that pre- and post-increment/decrement operations cannot be used for function 'out' parameters. These look like: uniform ivec4 u; attribute vec4 v; void f(out ivec4 p) { p = ivec4(v); } void main() { ivec4 t = u; f(t--); gl_Position = vec4(t); } Function 'inout' parameters are not explicitly tested. A test is generated for both vertex and fragment targets for each int and float vector or scalar datatype. Matrix types and GLSL 1.30 unsigned types are not tested. Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> Reviewed-by: Paul Berry <stereotype441@gmail.com>
System Environment: -------------------------- Libdrm: (master)2.4.30-18-gb643b0713aefdc0611e47654e88263b53b0de6f5 Mesa: (master)f9f8ce3ead04b5334bb323c37ec1f905ca5f58b2 Xserver: (master)xorg-server-1.11.99.902 Xf86_video_intel: (master)2.17.0-599-gca252e5b51d7b2f5a7b2c2e0d8fdb024b08096db Cairo: (master)0f40cdea1bdeedc730dde7814cdf056a12efb2cc with the latest mesa, this issue has been fixed. (In reply to comment #15) > This should be fixed in Mesa by the following commit series. > commit 1f125374e73d1718cd54c946826c76b09998c055 > Author: Ian Romanick <ian.d.romanick@intel.com> > Date: Fri Dec 23 10:59:38 2011 -0800 > glsl: Don't mark assignment temporaries as read-only > The various l-value errors this was designed to catch are now caught > by other means. Marking the temporaries as read-only now just > prevents sensible error messages from being generated. It's > 0:0(0): error: function parameter 'out p' references the read-only variable > versus > 0:13(5): error: function parameter 'out p' references a post-decrement > opera > Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> > Reviewed-by: Paul Berry <stereotype441@gmail.com> > commit 208b5b113faab324854d457c3ad4abf05cafedd1 > Author: Ian Romanick <ian.d.romanick@intel.com> > Date: Fri Dec 23 10:58:23 2011 -0800 > glsl: Emit extra errors for l-value violations in 'out' or 'inout' > parameter > Somethings, like pre-increment operations, were not previously caught. > After the 8.0 release, this code needs some major refactoring and > clean-up. It's a mess. :( > Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> > Reviewed-by: Paul Berry <stereotype441@gmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42755 > commit e9015e99d0d702570dbd3d7bcbafbf6c723cb7a7 > Author: Ian Romanick <ian.d.romanick@intel.com> > Date: Fri Dec 23 09:56:29 2011 -0800 > glsl: Emit errors for assignments to non-l-value expressions > Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> > Reviewed-by: Paul Berry <stereotype441@gmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=42755 > commit fa0a9ac5cdf49865cfc289c4326c73c9dd4a61c5 > Author: Ian Romanick <ian.d.romanick@intel.com> > Date: Fri Dec 23 09:56:03 2011 -0800 > glsl: Track descriptions of some expressions that can't be l-values > Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> > Reviewed-by: Paul Berry <stereotype441@gmail.com> > In addition, I have committed a series of more exhaustive test cases to piglit. > commit 025508171e209f730a035106799529c6fe2fcd8a > Author: Ian Romanick <ian.d.romanick@intel.com> > Date: Mon Jan 9 10:18:13 2012 -0800 > glsl-1.10: Verify pre- and post-inc/dec cannot be l-values > These generators create two kinds of tests. The first verifies that > pre- and post-increment/decrement operations cannot be assignment > targets. These look like: > uniform float u; > varying vec4 v; > void main() > { > float t = u; > ++t = float(v.x); > gl_FragColor = vec4(t, v.yzw); > } > The second kind of test verifies that pre- and > post-increment/decrement operations cannot be used for function 'out' > parameters. These look like: > uniform ivec4 u; > attribute vec4 v; > void f(out ivec4 p) > { > p = ivec4(v); > } > void main() > { > ivec4 t = u; > f(t--); > gl_Position = vec4(t); > } > Function 'inout' parameters are not explicitly tested. > A test is generated for both vertex and fragment targets for each > int and float vector or scalar datatype. Matrix types and GLSL 1.30 > unsigned types are not tested. > Signed-off-by: Ian Romanick <ian.d.romanick@intel.com> > Reviewed-by: Paul Berry <stereotype441@gmail.com>
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.