[Mesa-dev] [PATCH] main: fix basename match's check if it's an array or struct

Wed Oct 28 03:13:13 PDT 2015

On 28/10/15 10:31, Tapani Pälli wrote:
> On 10/28/2015 09:09 AM, Samuel Iglesias Gonsálvez wrote:
>>
>> On 28/10/15 06:53, Tapani Pälli wrote:
>>> On 10/27/2015 04:04 PM, Samuel Iglesias Gonsalvez wrote:
>>>> Commit 4565b6f did not update the basename match's check for
>>>> the case that string would exactly match the name of the
>>>> variable if the suffix "[0]" were appended to it.
>>>>
>>>> Fixes two dEQP-GLES31 tests:
>>>>
>>>> dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array
>>>>
>>>>
>>>> dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element
>>>>
>>>>
>>> These tests are passing already with commit 4565b6f right? I think the
>>> 'found' boolean already takes care of this, I need to step through again
>>> to make sure though.
>> No, they are failing in those cases because 'found' is true but then for
>> either Uniform block or shader storage block, we check if the name is an
>> array or struct:
>>
>>         if (found) {
>>           switch (programInterface) {
>>           case GL_UNIFORM_BLOCK:
>>           case GL_SHADER_STORAGE_BLOCK:
>>              /* Basename match, check if array or struct. */
>>              if (name[baselen] == '\0' ||
>>                  name[baselen] == '[' ||
>>                  name[baselen] == '.') {
>>              [...]
>>
>> As 'found' can be true because of two different cases, we need to take
>> care of both in this 'if' condition.
>>
>> $ ./deqp-gles31 -n
>> dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array*
>>
>> dEQP Core git-deb902685b9309a64b5e08c48157d3117cf27750 (0xdeb90268)
>> starting..
>>    target implementation = 'Default'
>>
>> Test case
>> 'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array'..
>>
>> Compute shader compile time = 16.732000 ms
>> Link time = 14.251000 ms
>> Test case duration in microseconds = 43993 us
>>    Fail (GetProgramResourceIndex returned unexpected values)
>>
>> Test case
>> 'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element'..
>>
>> Compute shader compile time = 1.114000 ms
>> Link time = 2.714000 ms
>> Test case duration in microseconds = 8858 us
>>    Fail (GetProgramResourceIndex returned unexpected values)
>>
>> DONE!
>>
>> Test run totals:
>>    Passed:        0/2 (0.0%)
>>    Failed:        2/2 (100.0%)
>>    Not supported: 0/2 (0.0%)
>>    Warnings:      0/2 (0.0%)
>>
>> $ glxinfo | grep git
>>
>>
>> OpenGL core profile version string: 3.3 (Core Profile) Mesa 11.1.0-devel
>> (git-4565b6f)
>> OpenGL version string: 3.0 Mesa 11.1.0-devel (git-4565b6f)
>> OpenGL ES profile version string: OpenGL ES 3.1 Mesa 11.1.0-devel
>> (git-4565b6f)
> 
> That is strange, for me these pass fine, I can see that our deqp version
> differ:
> 
> [tpalli at tpalli-mobl2 gles31]$ mesa_run ./deqp-gles31
> --deqp-case=dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array*
> 
> dEQP Core git-7d804783aef83f8f2784d99812d98bcfcf21e95f (0x7d804783)
> starting..
>   target implementation = 'Default'
> 
> Test case
> 'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array'..
> 
> Compute shader compile time = 4.585000 ms
> Link time = 3.678000 ms
> Test case duration in microseconds = 12021 us
>   Pass (Pass)
> 
> Test case
> 'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element'..
> 
> Compute shader compile time = 0.822000 ms
> Link time = 1.225000 ms
> Test case duration in microseconds = 3682 us
>   Pass (Pass)
> 
> DONE!
> 
> Test run totals:
>   Passed:        2/2 (100.0%)
>   Failed:        0/2 (0.0%)
>   Not supported: 0/2 (0.0%)
>   Warnings:      0/2 (0.0%)
> 
> 
> 
> This is with
> 
> MESA_EXTENSION_OVERRIDE=GL_ARB_compute_shader
> MESA_GLES_VERSION_OVERRIDE=3.1
> 
> and current Mesa head (03c92ff), tested on Ivybridge laptop.
> 

I checked out the same commits for both dEQP and Mesa. Run it on HSW
laptop with MESA_EXTENSION_OVERRIDE=GL_ARB_compute_shader and
MESA_GLES_VERSION_OVERRIDE=3.1. Both tests failed for me.

$ ./deqp-gles31 -n
dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array*
dEQP Core git-7d804783aef83f8f2784d99812d98bcfcf21e95f (0x7d804783)
starting..
  target implementation = 'Default'

Test case
'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array'..
Compute shader compile time = 10.610000 ms
Link time = 8.145000 ms
Test case duration in microseconds = 26574 us
  Fail (GetProgramResourceIndex returned unexpected values)

Test case
'dEQP-GLES31.functional.program_interface_query.shader_storage_block.resource_list.block_array_single_element'..
Compute shader compile time = 1.038000 ms
Link time = 2.763000 ms
Test case duration in microseconds = 7789 us
  Fail (GetProgramResourceIndex returned unexpected values)

DONE!

Test run totals:
  Passed:        0/2 (0.0%)
  Failed:        2/2 (100.0%)
  Not supported: 0/2 (0.0%)
  Warnings:      0/2 (0.0%)
$ glxinfo | grep git
OpenGL core profile version string: 3.3 (Core Profile) Mesa 11.1.0-devel
(git-0325f68)
OpenGL version string: 3.0 Mesa 11.1.0-devel (git-0325f68)
OpenGL ES profile version string: OpenGL ES 3.1 Mesa 11.1.0-devel
(git-0325f68)

Verify you don't have uncommitted local changes that can modify this
result. If not, this is very strange indeed (or maybe HSW is doing
something different when adding the resources).

Sam

> 
>>
>>   This function is becoming a bit of a pain because
>>> different resources have different naming schemes, I'll have to see if
>>> it could be somehow refactored simpler.
>>>
>> OK, thanks.
>>
>> Sam
>>
>>>> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>>> ---
>>>>    src/mesa/main/shader_query.cpp | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/mesa/main/shader_query.cpp
>>>> b/src/mesa/main/shader_query.cpp
>>>> index 59ec3d7..6cc91de 100644
>>>> --- a/src/mesa/main/shader_query.cpp
>>>> +++ b/src/mesa/main/shader_query.cpp
>>>> @@ -592,7 +592,8 @@ _mesa_program_resource_find_name(struct
>>>> gl_shader_program *shProg,
>>>>                /* Basename match, check if array or struct. */
>>>>                if (name[baselen] == '\0' ||
>>>>                    name[baselen] == '[' ||
>>>> -                name[baselen] == '.') {
>>>> +                name[baselen] == '.' ||
>>>> +                rname_has_array_index_zero) {
>>>>                   return res;
>>>>                }
>>>>                break;
>>>
> 
> 