Re: [Mesa-dev] [PATCH v2 0/3] Fix for PerVertexValidation CTS test

[Kenneth Graunke]

On Friday, October 27, 2017 12:28:16 PM PDT Neil Roberts wrote:
> Hi,
> 
> Here is a v2 of Eduardo’s patches to fix the PerVertexValidation CTS
> test. They were originally posted here:
> 
> https://lists.freedesktop.org/archives/mesa-dev/2017-March/146667.html

Actually, they're even older than that :)

https://lists.freedesktop.org/archives/mesa-dev/2017-January/142386.html

It looks like Timothy was concerned about enforcing restrictions on
unused varyings that get eliminated, and suggested that we file a spec
bug.  So, Eduardo did:

https://gitlab.khronos.org/Tracker/vk-gl-cts/issues/137

The response was that the test is correct as is, and we should indeed
make this change.  So, thanks for picking this up!

> The patches needed a non-trivial rebase. The previous discussion was
> left with a note saying that there are more cases that should fail but
> that weren’t handled by the patches and aren’t checked by the CTS
> test. Ie, if the consuming shader doesn’t reference any of the members
> of gl_PerVertex then it wouldn’t find the block redefinition. This v2
> addresses that by directly fetching the glsl_type for the gl_PerVertex
> block from the symbol table rather waiting for an instruction which
> references it.
> 
> I’ve tested it with Piglit and the public CTS repo and it doesn’t
> cause any regressions, and of course it fixes the CTS test.
> 
> It might be nice to add a Piglit or CTS test to check this missing
> case.
> 
> - Neil

Series is:
Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 0/3] Fix for PerVertexValidation CTS test

[Neil Roberts]

Hi,

Here is a v2 of Eduardo’s patches to fix the PerVertexValidation CTS
test. They were originally posted here:

https://lists.freedesktop.org/archives/mesa-dev/2017-March/146667.html

The patches needed a non-trivial rebase. The previous discussion was
left with a note saying that there are more cases that should fail but
that weren’t handled by the patches and aren’t checked by the CTS
test. Ie, if the consuming shader doesn’t reference any of the members
of gl_PerVertex then it wouldn’t find the block redefinition. This v2
addresses that by directly fetching the glsl_type for the gl_PerVertex
block from the symbol table rather waiting for an instruction which
references it.

I’ve tested it with Piglit and the public CTS repo and it doesn’t
cause any regressions, and of course it fixes the CTS test.

It might be nice to add a Piglit or CTS test to check this missing
case.

- Neil

Eduardo Lima Mitev (3):
  glsl_parser_extra: Add utility to copy symbols between symbol tables
  glsl: Use the utility function to copy symbols between symbol tables
  glsl/linker: Check that re-declared, inter-shader built-in blocks
match

 src/compiler/glsl/glsl_parser_extras.cpp| 68 +
 src/compiler/glsl/glsl_parser_extras.h  |  5 +++
 src/compiler/glsl/link_interface_blocks.cpp | 29 
 src/compiler/glsl/linker.cpp| 16 ++-
 4 files changed, 87 insertions(+), 31 deletions(-)

-- 
2.9.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev