<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 1 February 2014 01:24, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@intel.com" target="_blank">topi.pohjolainen@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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; </div></div>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? </blockquote><div> </div><div>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.</div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Still need to add that this is one the best documented tests that I've seen. Thanks Paul, makes really nice reading! </blockquote><div> </div><div>Thanks! Can I consider that a "Reviewed-by"?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div><div class="h5"> > + 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 > </div></div>> _______________________________________________ > Piglit mailing list > <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div> </div></div>