From d570b2b05b49453881c54069c417410e454d4850 Mon Sep 17 00:00:00 2001 From: Graeme Leese Date: Tue, 4 Oct 2022 16:16:52 +0100 Subject: [PATCH 1/2] Change Volatile generation for HelperInvocation For SPIR-V 1.6 HelperInvocation accesses need to be volatile to avoid undefined values when shaders execute 'demote'. Previously this was always decorated on the gl_HelperInvocation variable, but this is not valid when the Vulkan memory model is in use. When the memory model is enabled, stop decorating the variable declaration and apply the memory semantic to access chain loads instead. Fixes #3042 --- SPIRV/GlslangToSpv.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index ccb4602b..74053c17 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -4741,6 +4741,16 @@ spv::Id TGlslangToSpvTraverser::accessChainLoad(const glslang::TType& type) spv::Builder::AccessChain::CoherentFlags coherentFlags = builder.getAccessChain().coherentFlags; coherentFlags |= TranslateCoherent(type); + spv::MemoryAccessMask accessMask = spv::MemoryAccessMask(TranslateMemoryAccess(coherentFlags) & ~spv::MemoryAccessMakePointerAvailableKHRMask); + // If the value being loaded is HelperInvocation, SPIR-V 1.6 is being generated (so that + // SPV_EXT_demote_to_helper_invocation is in core) and the memory model is in use, add + // the Volatile MemoryAccess semantic. + if (type.getQualifier().builtIn == glslang::EbvHelperInvocation && + glslangIntermediate->usingVulkanMemoryModel() && + glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_6) { + accessMask = spv::MemoryAccessMask(accessMask | spv::MemoryAccessVolatileMask); + } + unsigned int alignment = builder.getAccessChain().alignment; alignment |= type.getBufferReferenceAlignment(); @@ -4748,7 +4758,7 @@ spv::Id TGlslangToSpvTraverser::accessChainLoad(const glslang::TType& type) TranslateNonUniformDecoration(builder.getAccessChain().coherentFlags), TranslateNonUniformDecoration(type.getQualifier()), nominalTypeId, - spv::MemoryAccessMask(TranslateMemoryAccess(coherentFlags) & ~spv::MemoryAccessMakePointerAvailableKHRMask), + accessMask, TranslateMemoryScope(coherentFlags), alignment); @@ -8968,6 +8978,7 @@ spv::Id TGlslangToSpvTraverser::getSymbolId(const glslang::TIntermSymbol* symbol // Add volatile decoration to HelperInvocation for spirv1.6 and beyond if (builtIn == spv::BuiltInHelperInvocation && + !glslangIntermediate->usingVulkanMemoryModel() && glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_6) { builder.addDecoration(id, spv::DecorationVolatile); } From 5c352476c7c700eae948c8e68181ec9f5f4f3710 Mon Sep 17 00:00:00 2001 From: Graeme Leese Date: Fri, 14 Oct 2022 15:03:14 +0100 Subject: [PATCH 2/2] Test for spv1.6 + memory model HelperInvocation This should generate a Volatile tag on the access, not on the variable itself. --- ...spv.1.6.helperInvocation.memmodel.frag.out | 48 +++++++++++++++++++ Test/spv.1.6.helperInvocation.memmodel.frag | 16 +++++++ gtests/Spv.FromFile.cpp | 1 + 3 files changed, 65 insertions(+) create mode 100644 Test/baseResults/spv.1.6.helperInvocation.memmodel.frag.out create mode 100644 Test/spv.1.6.helperInvocation.memmodel.frag diff --git a/Test/baseResults/spv.1.6.helperInvocation.memmodel.frag.out b/Test/baseResults/spv.1.6.helperInvocation.memmodel.frag.out new file mode 100644 index 00000000..fea4e458 --- /dev/null +++ b/Test/baseResults/spv.1.6.helperInvocation.memmodel.frag.out @@ -0,0 +1,48 @@ +spv.1.6.helperInvocation.memmodel.frag +// Module Version 10600 +// Generated by (magic number): 8000b +// Id's are bound by 21 + + Capability Shader + Capability VulkanMemoryModelKHR + Capability DemoteToHelperInvocationEXT + Extension "SPV_EXT_demote_to_helper_invocation" + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical VulkanKHR + EntryPoint Fragment 4 "main" 9 14 + ExecutionMode 4 OriginUpperLeft + Source ESSL 310 + SourceExtension "GL_EXT_demote_to_helper_invocation" + Name 4 "main" + Name 7 "B" + MemberName 7(B) 0 "o" + Name 9 "" + Name 14 "gl_HelperInvocation" + MemberDecorate 7(B) 0 Offset 0 + Decorate 7(B) Block + Decorate 9 DescriptorSet 0 + Decorate 9 Binding 0 + Decorate 14(gl_HelperInvocation) BuiltIn HelperInvocation + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeFloat 32 + 7(B): TypeStruct 6(float) + 8: TypePointer StorageBuffer 7(B) + 9: 8(ptr) Variable StorageBuffer + 10: TypeInt 32 1 + 11: 10(int) Constant 0 + 12: TypeBool + 13: TypePointer Input 12(bool) +14(gl_HelperInvocation): 13(ptr) Variable Input + 16: 6(float) Constant 1065353216 + 17: 6(float) Constant 0 + 19: TypePointer StorageBuffer 6(float) + 4(main): 2 Function None 3 + 5: Label + DemoteToHelperInvocationEXT + 15: 12(bool) Load 14(gl_HelperInvocation) Volatile + 18: 6(float) Select 15 16 17 + 20: 19(ptr) AccessChain 9 11 + Store 20 18 + Return + FunctionEnd diff --git a/Test/spv.1.6.helperInvocation.memmodel.frag b/Test/spv.1.6.helperInvocation.memmodel.frag new file mode 100644 index 00000000..b727f8e0 --- /dev/null +++ b/Test/spv.1.6.helperInvocation.memmodel.frag @@ -0,0 +1,16 @@ +#version 310 es + +#pragma use_vulkan_memory_model + +#extension GL_EXT_demote_to_helper_invocation : require + +precision highp float; + +layout (set=0, binding=0) buffer B { + float o; +}; + +void main() { + demote; + o = gl_HelperInvocation ? 1.0 : 0.0; +} diff --git a/gtests/Spv.FromFile.cpp b/gtests/Spv.FromFile.cpp index 93e364d9..e1b7a257 100644 --- a/gtests/Spv.FromFile.cpp +++ b/gtests/Spv.FromFile.cpp @@ -660,6 +660,7 @@ INSTANTIATE_TEST_SUITE_P( ::testing::ValuesIn(std::vector({ "spv.1.6.conditionalDiscard.frag", "spv.1.6.helperInvocation.frag", + "spv.1.6.helperInvocation.memmodel.frag", "spv.1.6.specConstant.comp", "spv.1.6.samplerBuffer.frag", "spv.1.6.separate.frag",