ANGLE tests broken by recent glslang change

[ianelliottus]

A recent commit broke ANGLE testing (which pulls/uses glslang downstream). We believe it's this commit (by Will Brown, with 1st line "Implement GL_EXT_vulkan_glsl_relaxed option"): https://chromium.googlesource.com/external/github.com/KhronosGroup/glslang.git/+/ecc9b9149f8f6367e4dc109e1f9ce3635a851968

Here is a commit where our auto-roller pulled glslang and first saw the problem: https://chromium-review.googlesource.com/c/chromium/src/+/2744336

If you click on the red "Tryjobs" you can see a page like this: https://ci.chromium.org/ui/p/chromium/builders/try/linux-swangle-try-x64/2314/overview

And then click on the first "Shard #0 (failed)" link, you'll see this: https://chromium-swarm.appspot.com/task?id=522efcb65413b410#dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocallstages,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocarraysnonspaced,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocarraysofarrays,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocarraysspaced,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocimplicitinsomestages,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocmixwithimplicit,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocmixwithimplicit2,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocmixwithimplicit3,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocmixwithimplicitmax,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocmixwithimplicitmaxarray,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformlocmultipleuniforms,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformloctypesmat,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformloctypesmix,dEQP.KHR_GLES31%2Fcore_explicit_uniform_location_uniformloctypesstructs,dEQP.KHR_GLES31%2Fcore_shader_storage_buffer_object_advancedmatrixvsfs,dEQP.KHR_GLES31%2Fcore_shader_storage_buffer_object_basicatomiccase1vsfs,dEQP.KHR_GLES31%2Fcore_texture_storage_multisample_FunctionalTests_blitting_multisampled_integer_attachment

Here's the failure of one of the tests:

[87/3472] dEQP.KHR_GLES31/core_explicit_uniform_location_uniformlocallstages (FAIL)

[ RUN      ] dEQP.KHR_GLES31/core_explicit_uniform_location_uniformlocallstages
KHR-GLES31.core.explicit_uniform_location.uniform-loc-all-stages
../../third_party/angle/src/tests/deqp_support/angle_deqp_gtest.cpp:53: Failure
Failed
glslang_wrapper_utils.cpp:1059 (LinkProgram): Internal error linking Vulkan shaders:
ERROR: Linking unknown stage stage: Anonymous member name used for global variable or other anonymous member: 
layout( column_major std140 offset=0) uniform highp 4-component vector of float


Stack trace:
#0 0x55de2001aa90 angle::(anonymous namespace)::HandlePlatformError()
#1 0x7fd34aae9e28 angle::LoggingAnnotator::logMessage()
#2 0x7fd34af8a185 gl::LogMessage::~LogMessage()
#3 0x7fd34ae1d010 rx::(anonymous namespace)::LinkProgram()
#4 0x7fd34ae1c6d4 rx::GlslangGetShaderSpirvCode()
#5 0x7fd34ad38ed9 rx::GlslangWrapperVk::GetShaderCode()
#6 0x7fd34ad3cbe5 rx::ShaderInfo::initShaders()
#7 0x7fd34ad4f1f2 rx::ProgramVk::link()
#8 0x7fd34aaf48f9 gl::Program::linkImpl()
#9 0x7fd34aaf3fe2 gl::Program::link()
#10 0x7fd34aaa4e73 gl::Context::linkProgram()
#11 0x7fd34a9cde69 GL_LinkProgram
#12 0x7fd34f2a5354 glu::CallLogWrapper::glLinkProgram()
#13 0x7fd34efe0a19 glcts::(anonymous namespace)::ExplicitUniformLocationCaseBase::CreatePrograms()
#14 0x7fd34efd97e9 glcts::(anonymous namespace)::ExplicitUniformLocationCaseBase::doRun()
#15 0x7fd34efd760b glcts::(anonymous namespace)::ExplicitUniformLocationCaseBase::doRun()
#16 0x7fd34efe6dad glcts::(anonymous namespace)::UniformLocMultipleStages::Run()
#17 0x7fd34eedddc6 deqp::TestSubcase::iterate()
#18 0x7fd34f0fa165 es31cts::TestCaseWrapper::iterate()
#19 0x7fd34f27e816 tcu::RandomOrderExecutor::executeInner()
#20 0x7fd34f27e747 tcu::RandomOrderExecutor::execute()
#21 0x7fd34eddaa50 deqp_libtester_run()
#22 0x55de200124bf angle::(anonymous namespace)::dEQP_KHR_GLES31_Test::TestBody()

../../third_party/angle/src/tests/deqp_support/angle_deqp_gtest.cpp:53: Failure
Failed
glslang_wrapper_utils.cpp:1059 (LinkProgram): Internal error linking Vulkan shaders:
ERROR: Linking unknown stage stage: Anonymous member name used for global variable or other anonymous member: 
layout( column_major std140 offset=0) uniform highp 4-component vector of float


Stack trace:
#0 0x55de2001aa90 angle::(anonymous namespace)::HandlePlatformError()
#1 0x7fd34aae9e28 angle::LoggingAnnotator::logMessage()
#2 0x7fd34af8a185 gl::LogMessage::~LogMessage()
#3 0x7fd34ae1d010 rx::(anonymous namespace)::LinkProgram()
#4 0x7fd34ae1c6d4 rx::GlslangGetShaderSpirvCode()
#5 0x7fd34ad38ed9 rx::GlslangWrapperVk::GetShaderCode()
#6 0x7fd34ad3cbe5 rx::ShaderInfo::initShaders()
#7 0x7fd34ad4f1f2 rx::ProgramVk::link()
#8 0x7fd34aaf48f9 gl::Program::linkImpl()
#9 0x7fd34aaf3fe2 gl::Program::link()
#10 0x7fd34aaa4e73 gl::Context::linkProgram()
#11 0x7fd34a9cde69 GL_LinkProgram
#12 0x7fd34f2a5354 glu::CallLogWrapper::glLinkProgram()
#13 0x7fd34efe0ad7 glcts::(anonymous namespace)::ExplicitUniformLocationCaseBase::CreatePrograms()
#14 0x7fd34efd97e9 glcts::(anonymous namespace)::ExplicitUniformLocationCaseBase::doRun()
#15 0x7fd34efd760b glcts::(anonymous namespace)::ExplicitUniformLocationCaseBase::doRun()
#16 0x7fd34efe6dad glcts::(anonymous namespace)::UniformLocMultipleStages::Run()
#17 0x7fd34eedddc6 deqp::TestSubcase::iterate()
#18 0x7fd34f0fa165 es31cts::TestCaseWrapper::iterate()
#19 0x7fd34f27e816 tcu::RandomOrderExecutor::executeInner()
#20 0x7fd34f27e747 tcu::RandomOrderExecutor::execute()
#21 0x7fd34eddaa50 deqp_libtester_run()
#22 0x55de200124bf angle::(anonymous namespace)::dEQP_KHR_GLES31_Test::TestBody()

../../third_party/angle/src/tests/deqp_support/angle_deqp_gtest.cpp:412: Failure
Value of: testSucceeded
  Actual: false
Expected: true
Stack trace:
#0 0x55de2001269e angle::(anonymous namespace)::dEQP_KHR_GLES31_Test::TestBody()

[  FAILED  ] dEQP.KHR_GLES31/core_explicit_uniform_location_uniformlocallstages, where GetParam() = 2130 (66 ms)
[88/3472] dEQP.KHR_GLES31/core_explicit_uniform_location_uniformlocimplicitinsomestages (FAIL)

[ianelliottus]

Attention: @greg-lunarg and @WDog367 . A change by Will appears to have broken our internal tests. Can you look into this soon please?

[greg-lunarg]

@mbechard heads up

[mbechard]

Yep, I'll take a look at this today.

[mbechard]

@ianelliottus sorry, not familiar with the Angle testing framework. Where do I find the source for the shader(s) that are failing?

[greg-lunarg]

@mbechard Can you follow this link?:

https://ci.chromium.org/ui/p/chromium/builders/try/linux-swangle-try-x64/2314/overview

This essentially lists all of failing tests for the OGL / Vulkan CTS (conformance test suite for drivers). The CTS can be found here: https://github.com/KhronosGroup/VK-GL-CTS

I am fairly certain that these will likely fail for any platform's driver, not just ANGLE, so it might be good to verify this on one of your platforms to get a clean run of CTS.

I believe CTS has a method to print the shaders for any test.

[greg-lunarg]

@mbechard Because #2242 is a feature and it is causing CTS regressions, I am going to revert until these are addressed.

[greg-lunarg]

@mbechard Please rebase and address this issue on https://github.com/mbechard/glslang/tree/GL_EXT_vulkan_glsl_relaxed and resubmit when it is ready. Thanks!

[mbechard]

Since the original developer (Will Brown) isn't maintaining this, are you ok with fixes going into new commits so we have a bit of history of this feature?

[mbechard]

Seems like I need to be a Khronos member to be able to gain access to the gitlab required to get VK-GL-CTS going?

[greg-lunarg]

Do you need gitlab access? Isn't https://github.com/KhronosGroup/VK-GL-CTS sufficient?

[mbechard]

Following the instructions, this script is failing:
platform/external/deqp/external/fetch_sources.py
trying to connect to the gitlab.

Seems like this is just one bug that everything is hitting, trying to dig out a shader to reproduce it from those logs but no luck so far.

[greg-lunarg]

Gah! I have gitlab access so I never noticed. This might be a mistake. It seems to make little sense to have a non-buildable project on github. It is likely someone broke fetch_sources.py with a recent (?) merge.

[greg-lunarg]

By my reading, you should not need gitlab permission to do what you want to do. Can you tell which repo is causing the fetch to fail?

[greg-lunarg]

zlib, libpng, glslang, spirv-headers, or spirv-tools?

[mbechard]

Ah sorry, got turned around between all the repos I'm trying to get going. I think my Windows command shell autocomplete picked up the fetch_kc_cts.py script instead of fetch_sources.py

[mbechard]

So I tracked down the issue. The new commit is flagging something as an error that glslang was previously allowing as legal. I think the new behavior is correct, and a lot of these Angle test cases are actually invalid GLSL. I know... hear me out.

The test cases where this newly errors are shaders where the vertex shader and pixel shader declare different anonymous uniform blocks that contain the same member names, which I think is illegal. E.g the test dEQP.GLES3/functional_shaders_linkage_uniform_struct_partial_vec4_vec3
The vertex shader declares:
layout(set=0, binding=0, std140) uniform defaultUniformsVS { _uStruct _uval; };
And in the pixel shader declares:
layout(set=0, binding=0, std140) uniform defaultUniformsFS { _uStruct _uval; float _uref_out0; };

_uval is delcared at global scope in more than one spot, but refers to a different object. Which the GLSL spec does not allow for:

Within a shader interface, all declarations of the same global name must be for the same object and
must match in type and in whether they declare a variable or member of a block with no instance
name. The API also needs this name to uniquely identify an object in the shader interface. It is a
link-time error if any particular shader interface contains
• two different blocks, each having no instance name, and each having a member of the same
name

If you try to do this on Nvidia's OpenGL GLSL compiler you get the error;

error: uniform buffer variable "vv" already belongs to interface block "testPS" and cannot also belong to interface block "testVS"

Not sure what the path forward is here, open to suggestions. Does this new behavior just break tests, or does Angle/Chromium depend on glslang allowing this behavior? @ianelliottus
I can certainly remove this check and make this legal again, but if so then the GLSL spec should be updated to match the behavior of GLSLang.

[mbechard]

I'll clarify, I think the code that is failing is code added by the Angle ESSL ->GLSL translator, and not present in the default VK-GL-CTS tests. Trying to get VK-GL-CTS working with those GLES tests to verify though.

[mbechard]

Yeah, the core issue here is Angle's ESSL -> Vulkan Compatible GLSL translator isn't creating legal GLSL, imo. When it's converting default uniforms into a uniform block for Vulkan compliance, these new blocks are the issue.
GL_EXT_vulkan_glsl_relaxed handles turning default uniforms into blocks as well, so it's possible an add-on extension to GL_EXT_vulkan_glsl_relaxed to be added to accept ESSL shaders as input, possibly removing the need for Angle's ESSL->Vulkan Compatible GLSL translator.

[greg-lunarg]

Thanks for looking into this @mbechard. I agree that the case(s) you have called out are not valid OGL.

I am sorry I was too hasty in reverting your original PR. I misunderstood the situation. However, to avoid being too hasty again, I will give @ianelliottus a few more hours to respond to your findings before I revert the revert.

Since the current understanding is that the incorrectness reported by this issue is not present in glslang but in the ANGLE driver, I will leave this issue closed now and after reverting the revert.

[ianelliottus]

Just saw this ...

@mbechard I'm asking our more compiler-oriented engineers to look at this discussion.

ANGLE supports OpenGL ES, not desktop OpenGL. For the shaders you referenced above, are these shaders provided by the test?

[mbechard]

Yes, I pulled that code directly out of the Angle test cases by putting a breakpoint and looking at the shader code that is fed into GLSLang during the test-run.

The issue has nothing to do with Desktop OpenGL. It's how Angle converts ESSL into Vulkan compliant GLSL to turn into SPIR-V for Vulkan. The generated Vulkan compliant GLSL isn't legal GLSL technically, although GLSLang accepted it in the past.

[ianelliottus]

Quick internal chats are happening and we're investigating an ANGLE change. I'll try to keep you posted.

[mbechard]

Sounds good, happy to look at any other test cases that are failing for other reasons too.

[ianelliottus]

We have a fix in progress.

FYI: We're not going to link these glsl shaders (in case you're interested: https://chromium-review.googlesource.com/c/angle/angle/+/2753096)

[mbechard]

That's a shame, was good to have those test cases occurring. It would seem that making the ESSL->GLSL translator output valid code is more future proof.

[greg-lunarg]

@ianelliottus Please let us know as soon as the coast is clear.

[ianelliottus]

Sorry, the coast is clear. Thanks!

[greg-lunarg]

Thanks! Reverting the revert.