On 10/28/2015 12:16 PM, Samuel Iglesias Gonsálvez wrote:

On 28/10/15 11:13, Samuel Iglesias Gonsálvez wrote:

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@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)

Sorry, I used a later commit, but with 03c92ff I get the same result.

I debugged this a bit, for me all the index queries with both of these tests hit GL_SHADER_STORAGE_BLOCK in that switch and for all of those name[baselen] == '\0' is true, that's why it passes. And this happens with luck because baselen is length of rname and rname can be longer in some cases than name, so we are actually reading out of bounds with that check :/


Sam

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 <sigles...@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;


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to