Re: [Piglit] [PATCH] arb_program_interface_query: corrected AoA's index variable expectation
On 22/3/19 1:48 am, Andres Gomez wrote:
On Sat, 2019-02-16 at 10:05 +1100, Timothy Arceri wrote:
NAK.
The problem is this will end up making the test fail on the Nvidia blob.
Technically neither is incorrect, but the test does show Mesa's failure
to detect the unused element. Again this is not technically a failure of
the spec as it's dependent on the implementations ability to detect
active array elements. However I'd rather leave this than work around
our substandard detection of inactive elements.
I wonder if it's really worthy, then, to test something that is
implementation dependent.
See [1] for more information, and note I also rejected the Mesa solution
proposed by Andrii in the bug report because it was too much code for
something that didn't actually remove the unused components but just hid
them from the resource list.
I think if we actually want to fix this properly then we could do it by
making a NIR linker for GLSL.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=92822
OK. Thanks for the thorough explanation! I'll drop this patch.
I also don't think we need a full NIR linker to pass this test. Just
switching to the new nir_build_program_resource_list() once further
support for ARB_gl_spirv lands (and we add any missing GL support)
should do the trick.
On 9/2/19 3:59 am, Andres Gomez wrote:
Naming conventions, from the GL_ARB_program_interface_query extension:
" * For an active variable declared as an array of an aggregate
data type (structures or arrays), a separate entry will be
generated for each active array element, unless noted
immediately below. The name of each entry is formed by
concatenating the name of the array, the "[" character, an
integer identifying the element number, and the "]" character.
These enumeration rules are applied recursively, treating each
enumerated array element as a separate active variable."
Cc: Timothy Arceri
Cc: Martin Peres
Signed-off-by: Andres Gomez
---
.../spec/arb_program_interface_query/getprogramresourceindex.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/spec/arb_program_interface_query/getprogramresourceindex.c
b/tests/spec/arb_program_interface_query/getprogramresourceindex.c
index 16b38e2d5..2afc9eeb9 100755
--- a/tests/spec/arb_program_interface_query/getprogramresourceindex.c
+++ b/tests/spec/arb_program_interface_query/getprogramresourceindex.c
@@ -167,7 +167,7 @@ static const struct subtest_index_t index_subtests[] = {
{ vs_aofa, GL_PROGRAM_INPUT, "vs_input2",
false, -1, GL_NO_ERROR },
{ vs_aofa, GL_PROGRAM_INPUT, "vs_input2[0]",
true, -1, GL_NO_ERROR },
{ vs_aofa, GL_PROGRAM_INPUT,"vs_input2[0][0]",
true, -1, GL_NO_ERROR },
- { vs_aofa, GL_PROGRAM_INPUT,"vs_input2[1][0]",
false, -1, GL_NO_ERROR },
+ { vs_aofa, GL_PROGRAM_INPUT,"vs_input2[1][0]",
true, -1, GL_NO_ERROR },
{ vs_aofa, GL_PROGRAM_INPUT,"vs_input2[0][1]",
false, -1, GL_NO_ERROR },
{vs_sub, GL_VERTEX_SUBROUTINE,"vss",
true, -1, GL_NO_ERROR },
{vs_sub, GL_VERTEX_SUBROUTINE, "vss2",
true, -1, GL_NO_ERROR },
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] arb_program_interface_query: corrected AoA's index variable expectation
On Sat, 2019-02-16 at 10:05 +1100, Timothy Arceri wrote:
> NAK.
>
> The problem is this will end up making the test fail on the Nvidia blob.
> Technically neither is incorrect, but the test does show Mesa's failure
> to detect the unused element. Again this is not technically a failure of
> the spec as it's dependent on the implementations ability to detect
> active array elements. However I'd rather leave this than work around
> our substandard detection of inactive elements.
I wonder if it's really worthy, then, to test something that is
implementation dependent.
> See [1] for more information, and note I also rejected the Mesa solution
> proposed by Andrii in the bug report because it was too much code for
> something that didn't actually remove the unused components but just hid
> them from the resource list.
>
> I think if we actually want to fix this properly then we could do it by
> making a NIR linker for GLSL.
>
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=92822
OK. Thanks for the thorough explanation! I'll drop this patch.
>
> On 9/2/19 3:59 am, Andres Gomez wrote:
> > Naming conventions, from the GL_ARB_program_interface_query extension:
> >
> > " * For an active variable declared as an array of an aggregate
> > data type (structures or arrays), a separate entry will be
> > generated for each active array element, unless noted
> > immediately below. The name of each entry is formed by
> > concatenating the name of the array, the "[" character, an
> > integer identifying the element number, and the "]" character.
> > These enumeration rules are applied recursively, treating each
> > enumerated array element as a separate active variable."
> >
> > Cc: Timothy Arceri
> > Cc: Martin Peres
> > Signed-off-by: Andres Gomez
> > ---
> > .../spec/arb_program_interface_query/getprogramresourceindex.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> > a/tests/spec/arb_program_interface_query/getprogramresourceindex.c
> > b/tests/spec/arb_program_interface_query/getprogramresourceindex.c
> > index 16b38e2d5..2afc9eeb9 100755
> > --- a/tests/spec/arb_program_interface_query/getprogramresourceindex.c
> > +++ b/tests/spec/arb_program_interface_query/getprogramresourceindex.c
> > @@ -167,7 +167,7 @@ static const struct subtest_index_t index_subtests[] = {
> > { vs_aofa, GL_PROGRAM_INPUT, "vs_input2",
> > false, -1, GL_NO_ERROR },
> > { vs_aofa, GL_PROGRAM_INPUT, "vs_input2[0]",
> > true, -1, GL_NO_ERROR },
> > { vs_aofa, GL_PROGRAM_INPUT,"vs_input2[0][0]",
> > true, -1, GL_NO_ERROR },
> > - { vs_aofa, GL_PROGRAM_INPUT,"vs_input2[1][0]",
> > false, -1, GL_NO_ERROR },
> > + { vs_aofa, GL_PROGRAM_INPUT,"vs_input2[1][0]",
> > true, -1, GL_NO_ERROR },
> > { vs_aofa, GL_PROGRAM_INPUT,"vs_input2[0][1]",
> > false, -1, GL_NO_ERROR },
> > {vs_sub, GL_VERTEX_SUBROUTINE,"vss",
> > true, -1, GL_NO_ERROR },
> > {vs_sub, GL_VERTEX_SUBROUTINE, "vss2",
> > true, -1, GL_NO_ERROR },
> >
--
Br,
Andres
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] glsl-1.10: test unreachable code in a loop is not accessed
Reviewed-by: Tapani Pälli
On 3/21/19 3:38 AM, Timothy Arceri wrote:
---
.../vs-loop-with-dead-code-unroll.shader_test | 43 +++
1 file changed, 43 insertions(+)
create mode 100644
tests/spec/glsl-1.10/execution/vs-loop-with-dead-code-unroll.shader_test
diff --git
a/tests/spec/glsl-1.10/execution/vs-loop-with-dead-code-unroll.shader_test
b/tests/spec/glsl-1.10/execution/vs-loop-with-dead-code-unroll.shader_test
new file mode 100644
index 0..780dd6428
--- /dev/null
+++ b/tests/spec/glsl-1.10/execution/vs-loop-with-dead-code-unroll.shader_test
@@ -0,0 +1,43 @@
+# Tests that unreachable code in a loop doesn't get accessed.
+[require]
+GLSL >= 1.10
+
+[vertex shader]
+uniform int loop_count;
+
+void main()
+{
+ vec4 colour_array[4];
+
+ colour_array[0] = vec4(0.25, 0.0, 0.0, 1.0);
+ colour_array[1] = vec4(0.5, 0.0, 0.0, 1.0);
+ colour_array[2] = vec4(0.75, 0.0, 0.0, 1.0);
+ colour_array[3] = vec4(1.0, 0.0, 0.0, 1.0);
+
+ gl_Position = gl_Vertex;
+
+ vec4 colour = vec4(0.0, 1.0, 0.0, 1.0);
+ for (int i = 0; i < 4; i++) {
+if (i < loop_count)
+ continue;
+else
+ break;
+
+colour = colour_array[i];
+ }
+
+ gl_FrontColor = colour;
+}
+
+[fragment shader]
+void main()
+{
+ gl_FragColor = gl_Color;
+}
+
+[test]
+clear color 0.5 0.5 0.5 0.5
+
+uniform int loop_count 4
+draw rect -1 -1 2 2
+probe all rgba 0.0 1.0 0.0 1.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
