Skip to content
Snippets Groups Projects

glsl-1.20: Test various aspects of do-while loop scoping

Merged Ian Romanick requested to merge idr/piglit:review/do-while-scope into main
1 unresolved thread

This test also inadvertently tests the sequence operator. At the time this test was written, Mesa would not compile it. See also !569 (merged). glslangValidator also fails to compile it (see https://github.com/KhronosGroup/glslang/issues/2744), but NVIDIA's closed-source driver does compile it.

Once the compilation problem is resolved, I would like to wait to land this test until we are at least sure it won't lead to a GPU hang. The original version of this test would, but this version should be safe. Be making various tweaks to the test (adding { } around the body of the loop, s/int i/i/ inside the loop), I'm pretty confident that it tests everything the way I intend without causing an infinite loop.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
21 // between the do and while (not including the while test
22 // expression), whether or not the body is simple or compound
23 //
24 // Earlier versions of the spec have similar wording. Any changes
25 // here are taken to be clarifications rather than intentional
26 // version-related changes.
27 int i = (j++, 0);
28 while (i <= 0 && j < 3);
29
30 if (j == -1)
31 // Correct result. Loop executed once and `j` was incremented once.
32 gl_FragColor = green;
33 else if (j == 2)
34 // This means `i` in the loop test incorrectly came from inside the
35 // loop and the sequence operator produced the wrong result (i.e., j++
36 // instead of 0).
  • Wouldn't we end up in the last else of the sequence operator was producing the wrong result? i.e isn't the last part of the comment in the wrong block?

    Otherwise the test looks good. Thanks Ian.

    Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>

  • Author Owner

    I tried several things to see various results.

    I changed the loop to this, and I got green:

        do {
            int i = (j++, 0);
        } while (i <= 0 && j < 3);

    I changed the loop to this, and I got blue:

        do
            i = j++;
        while (i <= 0 && j < 3);

    This was the scenario I was trying to describe in that comment. The loop test uses the i from inside the loop, and that i is assigned j++ instead of 0.

    I changed the loop to this, and I got red:

        do
            i = (j++, 0);
        while (i <= 0 && j < 3);

    This is the case where the loop test uses the i from inside the loop, but everything else is working properly.

    I just now changed the loop to this, and I got black:

        do
            i = 0;
        while (i <= 0 && j < 3);

    Which indicates there's another bug. :frowning2: Setting INTEL_DEBUG=fs shows that the whole shader is optimized away. Or do we intentionally nuke shaders that we know have infinite loops?

  • Or do we intentionally nuke shaders that we know have infinite loops?

    Yes I believe something was added for this a while ago.

    Update: At least I thought there was, but I can't seem to find the commit.

    Update2: Maybe this? mesa!2307 (merged)

    Edited by Timothy Arceri
  • Author Owner

    Yeah, that looks like it would do it. Okay. This seems okay.

    I'm going to add to the end of that comment, "The loop terminated when i was 1, and, due to post-increment, j is 2." How about that?

    Edited by Ian Romanick
  • Right. Yeah that makes sense.

  • changed this line in version 2 of the diff

  • Please register or sign in to reply
  • Ian Romanick added 2 commits

    added 2 commits

    • 99be1b06 - 1 commit from branch mesa:main
    • 59e1c93d - glsl-1.20: Test various aspects of do-while loop scoping

    Compare with previous version

  • mentioned in commit idr/piglit@da5b4601

  • Marge Bot added 1 commit

    added 1 commit

    • da5b4601 - glsl-1.20: Test various aspects of do-while loop scoping

    Compare with previous version

  • merged

  • Please register or sign in to reply
    Loading