[1/5] glsl-1.30: Test large local array

Submitted by Glenn Kennard on March 6, 2017, 11:11 p.m.

Details

Message ID 1488841878-11710-2-git-send-email-glenn.kennard@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Glenn Kennard March 6, 2017, 11:11 p.m.
GPUs typically will need to spill/reload values
for a large enough locally declared array.
---
 .../execution/fs-large-local-array.shader_test     | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test

Patch hide | download patch | download mbox

diff --git a/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test b/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
new file mode 100644
index 0000000..c050ba2
--- /dev/null
+++ b/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
@@ -0,0 +1,43 @@ 
+# Test correct handling of local-scope declared arrays large enough to
+# typically not fit into first level GPU memory such as its register file,
+# requiring storing/loading to some device-specific scratch space.
+#
+# One hardware example is R600 where arrays larger than 124*vec4 cannot
+# fit into its GPR register file and has to be spilled to scratch memory.
+# As of 2017-03-06 this is the largest known register file of any GPU, so
+# the test uses that as a size to guarantee some form of spilling on any GPU.
+#
+# GLSL 1.10 specification loosely implies a minimum limit of 64k elements
+# for an array due to 16 bit minimum precision for integer indexes. This
+# is higher than R600 scratch supports and likely more than many other gpus
+# support in practice, so this test restricts itself to testing "large enough"
+# array size.
+#
+# Later specifications leave this undefined but allow 32 bit integers for indexing.
+[require]
+GLSL >= 1.30
+
+[vertex shader passthrough]
+
+[fragment shader]
+uniform uint i;
+void main()
+{
+	uint A[130];
+	A[20] = 0u;
+	A[i] = 37u;
+	gl_FragColor.rba = vec3(0.0, 0.0, 1.0);
+	gl_FragColor.g = float(A[20] == 37u);
+}
+
+[test]
+clear color 1.0 0.0 0.0 1.0
+clear
+uniform uint i 19
+draw rect -1 -1 2 2
+probe all rgba 0.0 0.0 0.0 1.0
+
+clear
+uniform uint i 20
+draw rect -1 -1 2 2
+probe all rgba 0.0 1.0 0.0 1.0

Comments

On 07/03/17 00:11, Glenn Kennard wrote:
> GPUs typically will need to spill/reload values
> for a large enough locally declared array.
> ---
>  .../execution/fs-large-local-array.shader_test     | 43 ++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
>
> diff --git a/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test b/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
> new file mode 100644
> index 0000000..c050ba2
> --- /dev/null
> +++ b/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
> @@ -0,0 +1,43 @@
> +# Test correct handling of local-scope declared arrays large enough to
> +# typically not fit into first level GPU memory such as its register file,
> +# requiring storing/loading to some device-specific scratch space.
> +#
> +# One hardware example is R600 where arrays larger than 124*vec4 cannot
> +# fit into its GPR register file and has to be spilled to scratch memory.
> +# As of 2017-03-06 this is the largest known register file of any GPU, so
> +# the test uses that as a size to guarantee some form of spilling on any GPU.
> +#
> +# GLSL 1.10 specification loosely implies a minimum limit of 64k elements
> +# for an array due to 16 bit minimum precision for integer indexes. This
> +# is higher than R600 scratch supports and likely more than many other gpus
> +# support in practice, so this test restricts itself to testing "large enough"
> +# array size.
> +#
> +# Later specifications leave this undefined but allow 32 bit integers for indexing.
> +[require]
> +GLSL >= 1.30
> +
> +[vertex shader passthrough]
> +
> +[fragment shader]
> +uniform uint i;
> +void main()
> +{
> +	uint A[130];
> +	A[20] = 0u;
> +	A[i] = 37u;
> +	gl_FragColor.rba = vec3(0.0, 0.0, 1.0);
> +	gl_FragColor.g = float(A[20] == 37u);

What's the idea behind this comparison? I guess that at some point of
your development, when it was not supported, A[20] or A[i] was not
properly assigned, or the comparison was not working properly. But it is
somewhat non-intuitive, at least to me.

> +}
> +
> +[test]
> +clear color 1.0 0.0 0.0 1.0
> +clear
> +uniform uint i 19
> +draw rect -1 -1 2 2
> +probe all rgba 0.0 0.0 0.0 1.0
> +
> +clear
> +uniform uint i 20
> +draw rect -1 -1 2 2
> +probe all rgba 0.0 1.0 0.0 1.0
On Tue, 07 Mar 2017 10:34:48 +0100, Alejandro Piñeiro <apinheiro@igalia.com> wrote:

>
>
> On 07/03/17 00:11, Glenn Kennard wrote:
>> GPUs typically will need to spill/reload values
>> for a large enough locally declared array.
>> ---
>>  .../execution/fs-large-local-array.shader_test     | 43 ++++++++++++++++++++++
>>  1 file changed, 43 insertions(+)
>>  create mode 100644 tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
>>
>> diff --git a/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test b/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
>> new file mode 100644
>> index 0000000..c050ba2
>> --- /dev/null
>> +++ b/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
>> @@ -0,0 +1,43 @@
>> +# Test correct handling of local-scope declared arrays large enough to
>> +# typically not fit into first level GPU memory such as its register file,
>> +# requiring storing/loading to some device-specific scratch space.
>> +#
>> +# One hardware example is R600 where arrays larger than 124*vec4 cannot
>> +# fit into its GPR register file and has to be spilled to scratch memory.
>> +# As of 2017-03-06 this is the largest known register file of any GPU, so
>> +# the test uses that as a size to guarantee some form of spilling on any GPU.
>> +#
>> +# GLSL 1.10 specification loosely implies a minimum limit of 64k elements
>> +# for an array due to 16 bit minimum precision for integer indexes. This
>> +# is higher than R600 scratch supports and likely more than many other gpus
>> +# support in practice, so this test restricts itself to testing "large enough"
>> +# array size.
>> +#
>> +# Later specifications leave this undefined but allow 32 bit integers for indexing.
>> +[require]
>> +GLSL >= 1.30
>> +
>> +[vertex shader passthrough]
>> +
>> +[fragment shader]
>> +uniform uint i;
>> +void main()
>> +{
>> +	uint A[130];
>> +	A[20] = 0u;
>> +	A[i] = 37u;
>> +	gl_FragColor.rba = vec3(0.0, 0.0, 1.0);
>> +	gl_FragColor.g = float(A[20] == 37u);
>
> What's the idea behind this comparison? I guess that at some point of
> your development, when it was not supported, A[20] or A[i] was not
> properly assigned, or the comparison was not working properly. But it is
> somewhat non-intuitive, at least to me.

Hmm, how about:

  + # Test that storing and reading from a large local array returns expected results
...

  +	uint A[130]; // Large enough to require spilling on all known GPUs
  +	A[20] = 0u; // First store a known value to the element we will be reading back
  +	// Store to a compile-time unknown location, which may alias the above value, forcing the compiler to emit the previous store as-is
  +	// The non-constant indirection also inhibits possible splitting or lowering of the array to scalar values by the compiler
  +	A[i] = 37u; // 37 is a arbitrarily chosen value highly unlikely to be found at random in uninitialized scratch space memory
  +	gl_FragColor.rba = vec3(0.0, 0.0, 1.0);
  +	gl_FragColor.g = float(A[20] == 37u); // Read back the value

>
>> +}
>> +

  + # First check that storing to a location next to A[20] doesn't overwrite its initial value

>> +[test]
>> +clear color 1.0 0.0 0.0 1.0
>> +clear
>> +uniform uint i 19
>> +draw rect -1 -1 2 2
>> +probe all rgba 0.0 0.0 0.0 1.0
>> +

  + # Then check that storing to A[20] results in 37 being read back

>> +clear
>> +uniform uint i 20
>> +draw rect -1 -1 2 2
>> +probe all rgba 0.0 1.0 0.0 1.0
On 07/03/17 20:56, Glenn Kennard wrote:
> On Tue, 07 Mar 2017 10:34:48 +0100, Alejandro Piñeiro
> <apinheiro@igalia.com> wrote:
>
>>
>>
>> On 07/03/17 00:11, Glenn Kennard wrote:
>>> GPUs typically will need to spill/reload values
>>> for a large enough locally declared array.
>>> ---
>>>  .../execution/fs-large-local-array.shader_test     | 43
>>> ++++++++++++++++++++++
>>>  1 file changed, 43 insertions(+)
>>>  create mode 100644
>>> tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
>>>
>>> diff --git
>>> a/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
>>> b/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
>>> new file mode 100644
>>> index 0000000..c050ba2
>>> --- /dev/null
>>> +++ b/tests/spec/glsl-1.30/execution/fs-large-local-array.shader_test
>>> @@ -0,0 +1,43 @@
>>> +# Test correct handling of local-scope declared arrays large enough to
>>> +# typically not fit into first level GPU memory such as its
>>> register file,
>>> +# requiring storing/loading to some device-specific scratch space.
>>> +#
>>> +# One hardware example is R600 where arrays larger than 124*vec4
>>> cannot
>>> +# fit into its GPR register file and has to be spilled to scratch
>>> memory.
>>> +# As of 2017-03-06 this is the largest known register file of any
>>> GPU, so
>>> +# the test uses that as a size to guarantee some form of spilling
>>> on any GPU.
>>> +#
>>> +# GLSL 1.10 specification loosely implies a minimum limit of 64k
>>> elements
>>> +# for an array due to 16 bit minimum precision for integer indexes.
>>> This
>>> +# is higher than R600 scratch supports and likely more than many
>>> other gpus
>>> +# support in practice, so this test restricts itself to testing
>>> "large enough"
>>> +# array size.
>>> +#
>>> +# Later specifications leave this undefined but allow 32 bit
>>> integers for indexing.
>>> +[require]
>>> +GLSL >= 1.30
>>> +
>>> +[vertex shader passthrough]
>>> +
>>> +[fragment shader]
>>> +uniform uint i;
>>> +void main()
>>> +{
>>> +    uint A[130];
>>> +    A[20] = 0u;
>>> +    A[i] = 37u;
>>> +    gl_FragColor.rba = vec3(0.0, 0.0, 1.0);
>>> +    gl_FragColor.g = float(A[20] == 37u);
>>
>> What's the idea behind this comparison? I guess that at some point of
>> your development, when it was not supported, A[20] or A[i] was not
>> properly assigned, or the comparison was not working properly. But it is
>> somewhat non-intuitive, at least to me.
>
> Hmm, how about:
>
>  + # Test that storing and reading from a large local array returns
> expected results
> ...
>
>  +    uint A[130]; // Large enough to require spilling on all known GPUs
>  +    A[20] = 0u; // First store a known value to the element we will
> be reading back
>  +    // Store to a compile-time unknown location, which may alias the
> above value, forcing the compiler to emit the previous store as-is
>  +    // The non-constant indirection also inhibits possible splitting
> or lowering of the array to scalar values by the compiler
>  +    A[i] = 37u; // 37 is a arbitrarily chosen value highly unlikely
> to be found at random in uninitialized scratch space memory
>  +    gl_FragColor.rba = vec3(0.0, 0.0, 1.0);
>  +    gl_FragColor.g = float(A[20] == 37u); // Read back the value

Thanks for all the explanation. I think that it is good to have
something like that. Perhaps it would be more readable a summary at the
test intro (just after "Later specification ..."), but I will let you
decide which is better.

So, with or without that last suggestion, series:
Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>

>
>>
>>> +}
>>> +
>
>  + # First check that storing to a location next to A[20] doesn't
> overwrite its initial value
>
>>> +[test]
>>> +clear color 1.0 0.0 0.0 1.0
>>> +clear
>>> +uniform uint i 19
>>> +draw rect -1 -1 2 2
>>> +probe all rgba 0.0 0.0 0.0 1.0
>>> +
>
>  + # Then check that storing to A[20] results in 37 being read back
>
>>> +clear
>>> +uniform uint i 20
>>> +draw rect -1 -1 2 2
>>> +probe all rgba 0.0 1.0 0.0 1.0
>