[Piglit] [PATCH 1/2] Test that "continue" works properly inside a do-while loop.
Chris Forbes
chrisf at ijw.co.nz
Sat Feb 1 20:56:08 PST 2014
This test looks great.
For the series:
Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>
On Sun, Feb 2, 2014 at 5:10 AM, Pohjolainen, Topi
<topi.pohjolainen at intel.com> wrote:
> On Sat, Feb 01, 2014 at 07:40:26AM -0800, Paul Berry wrote:
>> On 1 February 2014 01:24, Pohjolainen, Topi <topi.pohjolainen at 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 at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/piglit
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list