glsl: add some test for shadow variable use in loops

these expose some bugs in mesa

added Tests label

mentioned in merge request mesa!12465 (merged)

[vertex shader passthrough]

With that changed, this test is

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

changed this line in version 2 of the diff

do while is the odd one in that its test doesn't already create a new scope. Yikes.

Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

I take this back... if I change the last line to gl_Position = vec4(++var_1);, glslangValidator compiles this without a complaint.

Are you saying you think that is correct behaviour? Its a compiler error on GCC at least.

I am honestly not sure what to think... other than that I an very annoyed.

GCC was the first thing I tried, and it got angry with the whole construct. It did not like the lack of punctuation between do and int.

$ gcc -Wall /tmp/asdf.c
/tmp/asdf.c: In function ‘main’:
/tmp/asdf.c:4:3: error: expected expression before ‘int’
    4 |   int x = 37;
      |   ^~~
/tmp/asdf.c:7:9: error: ‘x’ undeclared (first use in this function)
    7 |  return x;
      |         ^
/tmp/asdf.c:7:9: note: each undeclared identifier is reported only once for each function it appears in

To me, that's the most annoying part. This shouldn't compile at all in GLSL either since it's "just like C."

I looked at the grammar of the GLSL spec. It says:

iteration_statement:
    WHILE LEFT_PAREN condition RIGHT_PAREN statement_no_new_scope
    DO statement WHILE LEFT_PAREN expression RIGHT_PAREN SEMICOLON
    FOR LEFT_PAREN for_init_statement for_rest_statement RIGHT_PAREN statement_no_new_scope

Normally a statement doesn't automatically introduce a new scope unless it's a compound_statement. statement_no_new_scope is special in that it overrides that behavior by not introducing a new scope even for a compound_statement. After reading that, I checked the glslValidator behavior, and it accepts it.

BUT while writing this response, I found this explicit text in the spec:

The body of a do-while loop introduces a new scope lasting only between the do and while (not including the while test expression), whether or not the body is simple or compound:

    int i = 17;
    do
        int i = 4;    // okay, in nested scope
    while (i == 0);   // i is 17, scoped outside the do-while body

Which is almost identical to this test case. In previous discussions like this, JohnK has always said that the prose of the spec overrules the grammar section. So... glslValidator just has a bug? I'm curious to know what other compilers do.

I'm also curious about how other compilers / drivers handle cases like

    int i = 17;
    int j = 0;
    do
        int i = (j++, 0);
    while (i == 0);

    gl_FragColor = j == 1 ? green : red;

Green or GPU hang seem the most likely answers.

This is Green on the Nvidia blob.

The one time "it works on NVIDIA" is what I actually wanted to hear. I'm also assuming that the original test case fails to compile on NVIDIA? If that's the case, then this is just a glslangValidator bug, and I'll give back my original Rb.

I think I've found a way to modify the test case from my previous comment to test all the things and not GPU hang. This is clearly an area that's under-tested by the CTS, so I think I'll propose something like this for the CTS. I'm also going to submit a glslangValidator bug.

    int i = 17;
    int j = -2;
    do
        // Page 45 of the GLSL 4.60.7 specification says:
        //
        //    The body of a do-while loop introduces a new scope lasting only
        //    between the do and while (not including the while test
        //    expression), whether or not the body is simple or compound
        int i = (j++, 0);
    while (i <= 0 && j < 1);

    if (j == -1)
        // Correct result. Loop executed once and `j` was incremented once.
        gl_FragColor = green;
    else if (j == 0)
        // This means `i` in the loop test incorrectly came from inside the
        // loop and the sequence operator produced the wrong result.
        gl_FragColor = purple;
    else
        // Mostly likely cause is `i` in the loop test is the `i` from inside
        // the loop when it should be the `i` from outside the loop.
        gl_FragColor = red;

For reference, https://github.com/KhronosGroup/glslang/issues/2744.

Upon further testing, there were a couple minor issues in my proposed test case. I fixed those and submitted !575 (merged).

mentioned in merge request !575 (merged)

added 2 commits

• f80e2068 - glsl-1.10: add some testing of shadow vars in loops
• 0245478c - glsl-1.10: test for compiler error when do-while var used out of scope

Compare with previous version

assigned to @marge-bot

mentioned in commit tarceri/piglit@434f72c9

mentioned in commit tarceri/piglit@2d971137

added 12 commits

• 0245478c...43ebbb61 - 10 commits from branch mesa:main
• 2d971137 - glsl-1.10: add some testing of shadow vars in loops
• 434f72c9 - glsl-1.10: test for compiler error when do-while var used out of scope

Compare with previous version

merged

mentioned in commit Mesa_CI/repos/piglit@da5b4601