Add glsl-array-bounds-13.shader_test tests

Submitted by Nicolai Hähnle on Aug. 10, 2016, 6:34 p.m.

Details

Message ID 1470854096-32021-1-git-send-email-nhaehnle@gmail.com
State New
Headers show
Series "Add glsl-array-bounds-13.shader_test tests" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Nicolai Hähnle Aug. 10, 2016, 6:34 p.m.
From: Nicolai Hähnle <nicolai.haehnle@amd.com>

Compared to the other array bounds tests, this one exercises a larger array
size.

The motivation is for Radeon, where arrays can be lowered either into the
register file or into "real" memory. The larger array in this test will be
sure to hit real memory.
---
 tests/shaders/glsl-array-bounds-13.shader_test | 33 ++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 tests/shaders/glsl-array-bounds-13.shader_test

Patch hide | download patch | download mbox

diff --git a/tests/shaders/glsl-array-bounds-13.shader_test b/tests/shaders/glsl-array-bounds-13.shader_test
new file mode 100644
index 0000000..95aa550
--- /dev/null
+++ b/tests/shaders/glsl-array-bounds-13.shader_test
@@ -0,0 +1,33 @@ 
+[require]
+GLSL >= 1.20
+
+[vertex shader passthrough]
+
+[fragment shader]
+/* Verify that out-of-bounds access to an array does not result in any sort of
+ * program interruption.
+ *
+ * This test case uses a larger array, to cover drivers that have different
+ * lowering paths for different array sizes.
+ *
+ * In this test case the array index will not be constant folded.
+ */
+#version 120
+
+uniform int idx;
+float array[32];
+
+void main()
+{
+   /* Ensure that there can be no constant folding due to undefined values. */
+   array[0] = 0.0;
+   array[1] = 1.0;
+   gl_FragColor = vec4(0.0, 1.0, 0.0, array[idx]);
+}
+
+[test]
+uniform int idx -2
+clear color 0.0 0.0 0.0 0.0
+clear
+draw rect -1 -1 2 2
+probe all rgb 0.0 1.0 0.0

Comments

Hi;

On 08/10/2016 09:34 PM, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>
> Compared to the other array bounds tests, this one exercises a larger array
> size.
>
> The motivation is for Radeon, where arrays can be lowered either into the
> register file or into "real" memory. The larger array in this test will be
> sure to hit real memory.
> ---
>  tests/shaders/glsl-array-bounds-13.shader_test | 33 ++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 tests/shaders/glsl-array-bounds-13.shader_test
>
> diff --git a/tests/shaders/glsl-array-bounds-13.shader_test b/tests/shaders/glsl-array-bounds-13.shader_test
> new file mode 100644
> index 0000000..95aa550
> --- /dev/null
> +++ b/tests/shaders/glsl-array-bounds-13.shader_test
> @@ -0,0 +1,33 @@
> +[require]
> +GLSL >= 1.20
> +
> +[vertex shader passthrough]
> +
> +[fragment shader]
> +/* Verify that out-of-bounds access to an array does not result in any sort of
> + * program interruption.
> + *
> + * This test case uses a larger array, to cover drivers that have different
> + * lowering paths for different array sizes.
> + *
> + * In this test case the array index will not be constant folded.
> + */
> +#version 120
> +
> +uniform int idx;
> +float array[32];
> +
> +void main()
> +{
> +   /* Ensure that there can be no constant folding due to undefined values. */
> +   array[0] = 0.0;
> +   array[1] = 1.0;
> +   gl_FragColor = vec4(0.0, 1.0, 0.0, array[idx]);
> +}
> +
> +[test]
> +uniform int idx -2

According to GLSL 1.20 specs (and up to 4.50 spec):

'Undefined behavior results from indexing an array with a non-constant 
expression that’s greater than or equal to the array’s size or less than 0."

Having said that, we seem to have many similar tests so I guess it's 
fine to assume some output from the shader, just wanted to prompt that 
perhaps we are testing something that is undefined here?

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>

> +clear color 0.0 0.0 0.0 0.0
> +clear
> +draw rect -1 -1 2 2
> +probe all rgb 0.0 1.0 0.0

// Tapani
On 11.08.2016 07:56, Tapani Pälli wrote:
> Hi;
>
> On 08/10/2016 09:34 PM, Nicolai Hähnle wrote:
>> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>>
>> Compared to the other array bounds tests, this one exercises a larger
>> array
>> size.
>>
>> The motivation is for Radeon, where arrays can be lowered either into the
>> register file or into "real" memory. The larger array in this test
>> will be
>> sure to hit real memory.
>> ---
>>  tests/shaders/glsl-array-bounds-13.shader_test | 33
>> ++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>  create mode 100644 tests/shaders/glsl-array-bounds-13.shader_test
>>
>> diff --git a/tests/shaders/glsl-array-bounds-13.shader_test
>> b/tests/shaders/glsl-array-bounds-13.shader_test
>> new file mode 100644
>> index 0000000..95aa550
>> --- /dev/null
>> +++ b/tests/shaders/glsl-array-bounds-13.shader_test
>> @@ -0,0 +1,33 @@
>> +[require]
>> +GLSL >= 1.20
>> +
>> +[vertex shader passthrough]
>> +
>> +[fragment shader]
>> +/* Verify that out-of-bounds access to an array does not result in
>> any sort of
>> + * program interruption.
>> + *
>> + * This test case uses a larger array, to cover drivers that have
>> different
>> + * lowering paths for different array sizes.
>> + *
>> + * In this test case the array index will not be constant folded.
>> + */
>> +#version 120
>> +
>> +uniform int idx;
>> +float array[32];
>> +
>> +void main()
>> +{
>> +   /* Ensure that there can be no constant folding due to undefined
>> values. */
>> +   array[0] = 0.0;
>> +   array[1] = 1.0;
>> +   gl_FragColor = vec4(0.0, 1.0, 0.0, array[idx]);
>> +}
>> +
>> +[test]
>> +uniform int idx -2
>
> According to GLSL 1.20 specs (and up to 4.50 spec):
>
> 'Undefined behavior results from indexing an array with a non-constant
> expression that’s greater than or equal to the array’s size or less than
> 0."
>
> Having said that, we seem to have many similar tests so I guess it's
> fine to assume some output from the shader, just wanted to prompt that
> perhaps we are testing something that is undefined here?

I think you're right in theory. Undefined behavior might mean that the 
pixel shader gets terminated without writing anything to the 
framebuffer, though it definitely isn't allowed to mean program 
termination or hangs.

In practice, given that nobody has complained about these tests in the 
past, the UB seems to be limited to undefined data results in all 
implementations... if anybody ever complains, we might have to remove 
the probe checks from the test script.

> Reviewed-by: Tapani Pälli <tapani.palli@intel.com>

Thanks!

Nicolai


>> +clear color 0.0 0.0 0.0 0.0
>> +clear
>> +draw rect -1 -1 2 2
>> +probe all rgb 0.0 1.0 0.0
>
> // Tapani
Nicolai Hähnle <nhaehnle@gmail.com> writes:

> On 11.08.2016 07:56, Tapani Pälli wrote:
>> Hi;
>>
>> On 08/10/2016 09:34 PM, Nicolai Hähnle wrote:
>>> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>>>
>>> Compared to the other array bounds tests, this one exercises a larger
>>> array
>>> size.
>>>
>>> The motivation is for Radeon, where arrays can be lowered either into the
>>> register file or into "real" memory. The larger array in this test
>>> will be
>>> sure to hit real memory.
>>> ---
>>>  tests/shaders/glsl-array-bounds-13.shader_test | 33
>>> ++++++++++++++++++++++++++
>>>  1 file changed, 33 insertions(+)
>>>  create mode 100644 tests/shaders/glsl-array-bounds-13.shader_test
>>>
>>> diff --git a/tests/shaders/glsl-array-bounds-13.shader_test
>>> b/tests/shaders/glsl-array-bounds-13.shader_test
>>> new file mode 100644
>>> index 0000000..95aa550
>>> --- /dev/null
>>> +++ b/tests/shaders/glsl-array-bounds-13.shader_test
>>> @@ -0,0 +1,33 @@
>>> +[require]
>>> +GLSL >= 1.20
>>> +
>>> +[vertex shader passthrough]
>>> +
>>> +[fragment shader]
>>> +/* Verify that out-of-bounds access to an array does not result in
>>> any sort of
>>> + * program interruption.
>>> + *
>>> + * This test case uses a larger array, to cover drivers that have
>>> different
>>> + * lowering paths for different array sizes.
>>> + *
>>> + * In this test case the array index will not be constant folded.
>>> + */
>>> +#version 120
>>> +
>>> +uniform int idx;
>>> +float array[32];
>>> +
>>> +void main()
>>> +{
>>> +   /* Ensure that there can be no constant folding due to undefined
>>> values. */
>>> +   array[0] = 0.0;
>>> +   array[1] = 1.0;
>>> +   gl_FragColor = vec4(0.0, 1.0, 0.0, array[idx]);
>>> +}
>>> +
>>> +[test]
>>> +uniform int idx -2
>>
>> According to GLSL 1.20 specs (and up to 4.50 spec):
>>
>> 'Undefined behavior results from indexing an array with a non-constant
>> expression that’s greater than or equal to the array’s size or less than
>> 0."
>>
>> Having said that, we seem to have many similar tests so I guess it's
>> fine to assume some output from the shader, just wanted to prompt that
>> perhaps we are testing something that is undefined here?
>
> I think you're right in theory. Undefined behavior might mean that the 
> pixel shader gets terminated without writing anything to the 
> framebuffer, though it definitely isn't allowed to mean program 
> termination or hangs.
>
> In practice, given that nobody has complained about these tests in the 
> past, the UB seems to be limited to undefined data results in all 
> implementations... if anybody ever complains, we might have to remove 
> the probe checks from the test script.

I've definitely had to remove people's broken probes for undefined array
accesses in the past.  Please remove the probe checks now.