On Sat, Feb 01, 2014 at 07:40:26AM -0800, Paul Berry wrote: > On 1 February 2014 01:24, Pohjolainen, Topi <topi.pohjolai...@intel.com> > wrote: > > On Fri, Jan 31, 2014 at 01:12:09PM -0800, Paul Berry wrote: > > --- > > .../glsl-fs-continue-inside-do-while.shader_test | 51 > +++++++++++++++++++++ > > .../glsl-vs-continue-inside-do-while.shader_test | 52 > ++++++++++++++++++++++ > > 2 files changed, 103 insertions(+) > > create mode 100644 > tests/shaders/glsl-fs-continue-inside-do-while.shader_test > > create mode 100644 > tests/shaders/glsl-vs-continue-inside-do-while.shader_test > > > > diff --git > a/tests/shaders/glsl-fs-continue-inside-do-while.shader_test > b/tests/shaders/glsl-fs-continue-inside-do-while.shader_test > > new file mode 100644 > > index 0000000..9f5e491 > > --- /dev/null > > +++ b/tests/shaders/glsl-fs-continue-inside-do-while.shader_test > > @@ -0,0 +1,51 @@ > > +# From the GLSL 4.40 spec, section 6.4 (Jumps): > > +# > > +# The continue jump is used only in loops. It skips the remainder > > +# of the body of the inner most loop of which it is inside. For > > +# while and do-while loops, this jump is to the next evaluation > of > > +# the loop condition-expression from which the loop continues as > > +# previously defined. > > +# > > +# As of 1/31/2014 (commit db8b6fb), Mesa handles "continue" inside a > > +# do-while loop incorrectly; instead of jumping to the loop > > +# condition-expression, it jumps to the top of the loop. This is > > +# particularly problematic because it can lead to infinite loops. > > +# > > +# This test verifies correct behaviour of "continue" inside do-while > > +# loops without risking an infinite loop. > > + > > +[require] > > +GLSL >= 1.10 > > + > > +[vertex shader] > > +void main() > > +{ > > + gl_Position = gl_Vertex; > > +} > > + > > +[fragment shader] > > +void main() > > +{ > > + int x = 0; > > + int y = 0; > > + do { // 1st iteration 2nd iteration 3rd iteration > > + ++x; // x <- 1 x <- 2 x <- 3 > > + if (x >= 4) // false false false > > + break; > > This guarding is for the infinite looping case of the broken mesa to > terminate > after finite amount of iterations, right? Without this extra check the > following > continue would always jump to the top of the loop (as you explained) > causing the > final guarding "while (x < 3)" to be infinitely skipped. > If this is the case, would it make sense to add a small comment for the > future > when the problem in mesa not present anymore and somebody might wonder > why the > extra guarding is needed? > > I already have a comment hinting at this ("This test verifies correct > behaviour of 'continue' inside do-while loops without risking an infinite > loop"). I'll expand the comment so that it's clearer.
True, I take that comment back, I think it's fine as is. > > > Still need to add that this is one the best documented tests that I've > seen. > Thanks Paul, makes really nice reading! > > Thanks! Can I consider that a "Reviewed-by"? Yes. > > > > + if (x >= 2) // false true true > > + continue; > > + ++y; // y=1 skipped skipped > > + } while (x < 3); // true true false > > + > > + // The "continue" should skip ++y on all iterations but the first, > > + // so y should now be 1. The "continue" should not skip (x < 3) > > + // ever, so the loop should terminate when x == 3 (not 4). > > + if (x == 3 && y == 1) > > + gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0); > > + else > > + gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0); > > +} > > + > > +[test] > > +draw rect -1 -1 2 2 > > +probe all rgba 0.0 1.0 0.0 1.0 > > diff --git > a/tests/shaders/glsl-vs-continue-inside-do-while.shader_test > b/tests/shaders/glsl-vs-continue-inside-do-while.shader_test > > new file mode 100644 > > index 0000000..aa6d3e6 > > --- /dev/null > > +++ b/tests/shaders/glsl-vs-continue-inside-do-while.shader_test > > @@ -0,0 +1,52 @@ > > +# From the GLSL 4.40 spec, section 6.4 (Jumps): > > +# > > +# The continue jump is used only in loops. It skips the remainder > > +# of the body of the inner most loop of which it is inside. For > > +# while and do-while loops, this jump is to the next evaluation > of > > +# the loop condition-expression from which the loop continues as > > +# previously defined. > > +# > > +# As of 1/31/2014 (commit db8b6fb), Mesa handles "continue" inside a > > +# do-while loop incorrectly; instead of jumping to the loop > > +# condition-expression, it jumps to the top of the loop. This is > > +# particularly problematic because it can lead to infinite loops. > > +# > > +# This test verifies correct behaviour of "continue" inside do-while > > +# loops without risking an infinite loop. > > + > > +[require] > > +GLSL >= 1.10 > > + > > +[vertex shader] > > +void main() > > +{ > > + gl_Position = gl_Vertex; > > + int x = 0; > > + int y = 0; > > + do { // 1st iteration 2nd iteration 3rd iteration > > + ++x; // x <- 1 x <- 2 x <- 3 > > + if (x >= 4) // false false false > > + break; > > + if (x >= 2) // false true true > > + continue; > > + ++y; // y=1 skipped skipped > > + } while (x < 3); // true true false > > + > > + // The "continue" should skip ++y on all iterations but the first, > > + // so y should now be 1. The "continue" should not skip (x < 3) > > + // ever, so the loop should terminate when x == 3 (not 4). > > + if (x == 3 && y == 1) > > + gl_FrontColor = vec4(0.0, 1.0, 0.0, 1.0); > > + else > > + gl_FrontColor = vec4(1.0, 0.0, 0.0, 1.0); > > +} > > + > > +[fragment shader] > > +void main() > > +{ > > + gl_FragColor = gl_Color; > > +} > > + > > +[test] > > +draw rect -1 -1 2 2 > > +probe all rgba 0.0 1.0 0.0 1.0 > > -- > > 1.8.5.3 > > > > _______________________________________________ > > Piglit mailing list > > Piglit@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit