shaderc icon indicating copy to clipboard operation
shaderc copied to clipboard

Fix test breakage in shaderc_util_compiler

Open zoddicus opened this issue 6 years ago • 7 comments

CompilerTest.HlslFunctionality1Enabled in this test suite is failing, because the third_party/spirv-tools has not been rolled to pick up the SPIR-V 1.4 changes yet, but the test expectations were updated in #608

Specifically OpDecorateStringGOOGLE was changed to OpDecorateString.

zoddicus avatar May 08 '19 15:05 zoddicus

Rolling glslang, spirv-tools, spirv-headers via #614

dneto0 avatar May 08 '19 15:05 dneto0

When rolling forward SPIRV-Tools, SPIRV-Headers, and Glslang, that fails.

When I also roll forward SPIRV-Cross, I get a bunch of failures, including validation failures showing SPIR-V 1.4 modules failing to validate for Vulkan 1.1. Many of those I can fix by forcing vulkan1.1 target environment for .asm. tests.

I still have a few left.

dneto0 avatar May 08 '19 17:05 dneto0

One of the errors looks like a bad test case in SPIRV-Cross: https://github.com/KhronosGroup/SPIRV-Cross/issues/967

dneto0 avatar May 08 '19 17:05 dneto0

Did you run the spirv-cross tests (the actual spirv-cross tests, not the shaderc/spvc/spirv-cross tests)? It seems unlikely (though anything is possible) that there's a spirv-cross rev where their tests don't pass. If it works in spirv-cross it should work in shaderc/spvc/spirv-cross. If not then it's a good bet https://github.com/google/shaderc/blob/master/spvc/test/run_spirv_cross_tests.py needs updating to correctly mimic the behavior of https://github.com/KhronosGroup/SPIRV-Cross/blob/master/test_shaders.py

fjhenigman avatar May 08 '19 18:05 fjhenigman

Good point. I just ran the SPIRV-Cross tests, when it's built with its own older versions of glslang, spirv-tools, and spirv-headers. They pass, with a small handful of unrelated warnings (array of arrays, and precision).

The changelog on test_shaders.py is pretty slim lately and does not seem relevant. So far the two issues @zoddicus and I have identified are independent of SPIRV-Cross passing its own tests. That is,

  • the bad test case I identified above is legitimately invalid GLSL (with the extension) and Glslang is rightly rejecting it.
  • When assembling .asm. cases, we should be explicitly specifying vulkan1.1 target environment (to get SPIR-V 1.3, rather than SPIR-V 1.4)

dneto0 avatar May 08 '19 21:05 dneto0

  • One more bad SPIRV-Cross test case https://github.com/KhronosGroup/SPIRV-Cross/blob/master/shaders/amd/shader_ballot_nonuniform_invocations.invalid.comp This exercises a bug I filed https://github.com/KhronosGroup/glslang/issues/1763 I suspect it's really a bug in SPV_AMD_shader_ballot and we'll end up relaxing the validation rules by saying SPV_AMD_shader_ballot enables the Groups capability.

dneto0 avatar May 08 '19 21:05 dneto0

The last failure is a legitimate validation failure:

WARNING: spirv-cross/shaders/flatten/multi-dimensional.desktop.invalid.flatten_dim.frag:4: '[][]' : Generating SPIR-V array-of-arrays, but Vulkan only supports single array level for this resource

validation failed: UniformConstant OpVariable '57[%uTextures]' has illegal type. From Vulkan spec, section 14.5.2: Variables identified with the UniformConstant storage class are used only as handles to refer to opaque resources. Such variables must be typed as OpTypeImage, OpTypeSampler, OpTypeSampledImage, OpTypeAccelerationStructureNV, or an array of one of these types. %uTextures = OpVariable %_ptr_UniformConstant__arr__arr__arr_52_uint_1_uint_3_uint_2 UniformConstant

This is a new validation check and it's legitimate. Really, Glslang should not be generating such code, but should be flattening the array. (I bet this is known, but should check.)

dneto0 avatar May 08 '19 21:05 dneto0