From 5340752190843cafab8a2ee94bff5f1a3b743839 Mon Sep 17 00:00:00 2001 From: Malcolm Bechard Date: Wed, 17 Mar 2021 18:47:13 -0400 Subject: [PATCH] Fix issue with remapping global uniform blocks Avoid adding global uniform blocks to stages that don't already have it. Otherwise multiple stages point to the same block object, and a remapping that occurs later on will change the mapping on multiple stages. --- .../baseResults/vk.relaxed.changeSet.vert.out | 281 ++++++++++++++++++ Test/vk.relaxed.changeSet.frag | 13 + Test/vk.relaxed.changeSet.vert | 16 + glslang/MachineIndependent/ShaderLang.cpp | 7 +- glslang/MachineIndependent/linkValidate.cpp | 10 +- .../MachineIndependent/localintermediate.h | 2 +- gtests/VkRelaxed.FromFile.cpp | 9 + 7 files changed, 332 insertions(+), 6 deletions(-) create mode 100755 Test/baseResults/vk.relaxed.changeSet.vert.out create mode 100755 Test/vk.relaxed.changeSet.frag create mode 100755 Test/vk.relaxed.changeSet.vert diff --git a/Test/baseResults/vk.relaxed.changeSet.vert.out b/Test/baseResults/vk.relaxed.changeSet.vert.out new file mode 100755 index 000000000..f6bce292b --- /dev/null +++ b/Test/baseResults/vk.relaxed.changeSet.vert.out @@ -0,0 +1,281 @@ +vk.relaxed.changeSet.vert +Shader version: 460 +0:? Sequence +0:11 Function Definition: main( ( global void) +0:11 Function Parameters: +0:13 Sequence +0:13 move second child to first child ( temp highp 4-component vector of float) +0:13 'Color' ( smooth out highp 4-component vector of float) +0:13 'aColor' ( in highp 4-component vector of float) +0:14 move second child to first child ( temp highp 2-component vector of float) +0:14 'UV' ( smooth out highp 2-component vector of float) +0:14 'aUV' ( in highp 2-component vector of float) +0:15 move second child to first child ( temp highp 4-component vector of float) +0:15 gl_Position: direct index for structure ( gl_Position highp 4-component vector of float Position) +0:15 'anon@1' ( out block{ gl_Position 4-component vector of float Position gl_Position, gl_PointSize float PointSize gl_PointSize, out unsized 1-element array of float ClipDistance gl_ClipDistance, out unsized 1-element array of float CullDistance gl_CullDistance}) +0:15 Constant: +0:15 0 (const uint) +0:15 matrix-times-vector ( temp highp 4-component vector of float) +0:15 projectionMatrix: direct index for structure ( uniform highp 4X4 matrix of float) +0:15 'anon@0' (layout( column_major std140) uniform block{ uniform highp 4X4 matrix of float projectionMatrix}) +0:15 Constant: +0:15 0 (const uint) +0:15 Construct vec4 ( temp highp 4-component vector of float) +0:15 'aPos' ( in highp 2-component vector of float) +0:15 Constant: +0:15 0.000000 +0:15 Constant: +0:15 1.000000 +0:? Linker Objects +0:? 'aPos' ( in highp 2-component vector of float) +0:? 'aUV' ( in highp 2-component vector of float) +0:? 'aColor' ( in highp 4-component vector of float) +0:? 'anon@0' (layout( column_major std140) uniform block{ uniform highp 4X4 matrix of float projectionMatrix}) +0:? 'Color' ( smooth out highp 4-component vector of float) +0:? 'UV' ( smooth out highp 2-component vector of float) +0:? 'anon@1' ( out block{ gl_Position 4-component vector of float Position gl_Position, gl_PointSize float PointSize gl_PointSize, out unsized 1-element array of float ClipDistance gl_ClipDistance, out unsized 1-element array of float CullDistance gl_CullDistance}) +0:? 'gl_VertexID' ( in int VertexIndex) +0:? 'gl_InstanceID' ( in int InstanceIndex) + +vk.relaxed.changeSet.frag +Shader version: 460 +gl_FragCoord origin is upper left +0:? Sequence +0:10 Function Definition: main( ( global void) +0:10 Function Parameters: +0:12 Sequence +0:12 move second child to first child ( temp highp 4-component vector of float) +0:12 'fragColor' (layout( location=0) out highp 4-component vector of float) +0:12 vector-scale ( temp highp 4-component vector of float) +0:12 'Color' ( smooth in highp 4-component vector of float) +0:12 direct index ( temp highp float) +0:12 texture ( global highp 4-component vector of float) +0:12 'sTexture' ( uniform highp sampler2D) +0:12 vector swizzle ( temp highp 2-component vector of float) +0:12 'UV' ( smooth in highp 2-component vector of float) +0:12 Sequence +0:12 Constant: +0:12 0 (const int) +0:12 Constant: +0:12 1 (const int) +0:12 Constant: +0:12 0 (const int) +0:? Linker Objects +0:? 'fragColor' (layout( location=0) out highp 4-component vector of float) +0:? 'sTexture' ( uniform highp sampler2D) +0:? 'Color' ( smooth in highp 4-component vector of float) +0:? 'UV' ( smooth in highp 2-component vector of float) + + +Linked vertex stage: + + +Linked fragment stage: + + +Shader version: 460 +0:? Sequence +0:11 Function Definition: main( ( global void) +0:11 Function Parameters: +0:13 Sequence +0:13 move second child to first child ( temp highp 4-component vector of float) +0:13 'Color' ( smooth out highp 4-component vector of float) +0:13 'aColor' ( in highp 4-component vector of float) +0:14 move second child to first child ( temp highp 2-component vector of float) +0:14 'UV' ( smooth out highp 2-component vector of float) +0:14 'aUV' ( in highp 2-component vector of float) +0:15 move second child to first child ( temp highp 4-component vector of float) +0:15 gl_Position: direct index for structure ( gl_Position highp 4-component vector of float Position) +0:15 'anon@1' ( out block{ gl_Position 4-component vector of float Position gl_Position, gl_PointSize float PointSize gl_PointSize, out 1-element array of float ClipDistance gl_ClipDistance, out 1-element array of float CullDistance gl_CullDistance}) +0:15 Constant: +0:15 0 (const uint) +0:15 matrix-times-vector ( temp highp 4-component vector of float) +0:15 projectionMatrix: direct index for structure ( uniform highp 4X4 matrix of float) +0:15 'anon@0' (layout( column_major std140) uniform block{ uniform highp 4X4 matrix of float projectionMatrix}) +0:15 Constant: +0:15 0 (const uint) +0:15 Construct vec4 ( temp highp 4-component vector of float) +0:15 'aPos' ( in highp 2-component vector of float) +0:15 Constant: +0:15 0.000000 +0:15 Constant: +0:15 1.000000 +0:? Linker Objects +0:? 'aPos' ( in highp 2-component vector of float) +0:? 'aUV' ( in highp 2-component vector of float) +0:? 'aColor' ( in highp 4-component vector of float) +0:? 'anon@0' (layout( column_major std140) uniform block{ uniform highp 4X4 matrix of float projectionMatrix}) +0:? 'Color' ( smooth out highp 4-component vector of float) +0:? 'UV' ( smooth out highp 2-component vector of float) +0:? 'anon@1' ( out block{ gl_Position 4-component vector of float Position gl_Position, gl_PointSize float PointSize gl_PointSize, out 1-element array of float ClipDistance gl_ClipDistance, out 1-element array of float CullDistance gl_CullDistance}) +0:? 'gl_VertexID' ( in int VertexIndex) +0:? 'gl_InstanceID' ( in int InstanceIndex) +Shader version: 460 +gl_FragCoord origin is upper left +0:? Sequence +0:10 Function Definition: main( ( global void) +0:10 Function Parameters: +0:12 Sequence +0:12 move second child to first child ( temp highp 4-component vector of float) +0:12 'fragColor' (layout( location=0) out highp 4-component vector of float) +0:12 vector-scale ( temp highp 4-component vector of float) +0:12 'Color' ( smooth in highp 4-component vector of float) +0:12 direct index ( temp highp float) +0:12 texture ( global highp 4-component vector of float) +0:12 'sTexture' ( uniform highp sampler2D) +0:12 vector swizzle ( temp highp 2-component vector of float) +0:12 'UV' ( smooth in highp 2-component vector of float) +0:12 Sequence +0:12 Constant: +0:12 0 (const int) +0:12 Constant: +0:12 1 (const int) +0:12 Constant: +0:12 0 (const int) +0:? Linker Objects +0:? 'fragColor' (layout( location=0) out highp 4-component vector of float) +0:? 'sTexture' ( uniform highp sampler2D) +0:? 'Color' ( smooth in highp 4-component vector of float) +0:? 'UV' ( smooth in highp 2-component vector of float) + +// Module Version 10000 +// Generated by (magic number): 8000a +// Id's are bound by 46 + + Capability Shader + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical GLSL450 + EntryPoint Vertex 4 "main" 9 11 15 17 24 34 44 45 + Source GLSL 460 + Name 4 "main" + Name 9 "Color" + Name 11 "aColor" + Name 15 "UV" + Name 17 "aUV" + Name 22 "gl_PerVertex" + MemberName 22(gl_PerVertex) 0 "gl_Position" + MemberName 22(gl_PerVertex) 1 "gl_PointSize" + MemberName 22(gl_PerVertex) 2 "gl_ClipDistance" + MemberName 22(gl_PerVertex) 3 "gl_CullDistance" + Name 24 "" + Name 28 "gl_DefaultUniformBlock" + MemberName 28(gl_DefaultUniformBlock) 0 "projectionMatrix" + Name 30 "" + Name 34 "aPos" + Name 44 "gl_VertexID" + Name 45 "gl_InstanceID" + Decorate 9(Color) Location 0 + Decorate 11(aColor) Location 2 + Decorate 15(UV) Location 1 + Decorate 17(aUV) Location 1 + MemberDecorate 22(gl_PerVertex) 0 BuiltIn Position + MemberDecorate 22(gl_PerVertex) 1 BuiltIn PointSize + MemberDecorate 22(gl_PerVertex) 2 BuiltIn ClipDistance + MemberDecorate 22(gl_PerVertex) 3 BuiltIn CullDistance + Decorate 22(gl_PerVertex) Block + MemberDecorate 28(gl_DefaultUniformBlock) 0 ColMajor + MemberDecorate 28(gl_DefaultUniformBlock) 0 Offset 0 + MemberDecorate 28(gl_DefaultUniformBlock) 0 MatrixStride 16 + Decorate 28(gl_DefaultUniformBlock) Block + Decorate 30 DescriptorSet 0 + Decorate 30 Binding 0 + Decorate 34(aPos) Location 0 + Decorate 44(gl_VertexID) BuiltIn VertexIndex + Decorate 45(gl_InstanceID) BuiltIn InstanceIndex + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeFloat 32 + 7: TypeVector 6(float) 4 + 8: TypePointer Output 7(fvec4) + 9(Color): 8(ptr) Variable Output + 10: TypePointer Input 7(fvec4) + 11(aColor): 10(ptr) Variable Input + 13: TypeVector 6(float) 2 + 14: TypePointer Output 13(fvec2) + 15(UV): 14(ptr) Variable Output + 16: TypePointer Input 13(fvec2) + 17(aUV): 16(ptr) Variable Input + 19: TypeInt 32 0 + 20: 19(int) Constant 1 + 21: TypeArray 6(float) 20 +22(gl_PerVertex): TypeStruct 7(fvec4) 6(float) 21 21 + 23: TypePointer Output 22(gl_PerVertex) + 24: 23(ptr) Variable Output + 25: TypeInt 32 1 + 26: 25(int) Constant 0 + 27: TypeMatrix 7(fvec4) 4 +28(gl_DefaultUniformBlock): TypeStruct 27 + 29: TypePointer Uniform 28(gl_DefaultUniformBlock) + 30: 29(ptr) Variable Uniform + 31: TypePointer Uniform 27 + 34(aPos): 16(ptr) Variable Input + 36: 6(float) Constant 0 + 37: 6(float) Constant 1065353216 + 43: TypePointer Input 25(int) + 44(gl_VertexID): 43(ptr) Variable Input +45(gl_InstanceID): 43(ptr) Variable Input + 4(main): 2 Function None 3 + 5: Label + 12: 7(fvec4) Load 11(aColor) + Store 9(Color) 12 + 18: 13(fvec2) Load 17(aUV) + Store 15(UV) 18 + 32: 31(ptr) AccessChain 30 26 + 33: 27 Load 32 + 35: 13(fvec2) Load 34(aPos) + 38: 6(float) CompositeExtract 35 0 + 39: 6(float) CompositeExtract 35 1 + 40: 7(fvec4) CompositeConstruct 38 39 36 37 + 41: 7(fvec4) MatrixTimesVector 33 40 + 42: 8(ptr) AccessChain 24 26 + Store 42 41 + Return + FunctionEnd +// Module Version 10000 +// Generated by (magic number): 8000a +// Id's are bound by 27 + + Capability Shader + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical GLSL450 + EntryPoint Fragment 4 "main" 9 11 20 + ExecutionMode 4 OriginUpperLeft + Source GLSL 460 + Name 4 "main" + Name 9 "fragColor" + Name 11 "Color" + Name 16 "sTexture" + Name 20 "UV" + Decorate 9(fragColor) Location 0 + Decorate 11(Color) Location 0 + Decorate 16(sTexture) DescriptorSet 1 + Decorate 16(sTexture) Binding 0 + Decorate 20(UV) Location 1 + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeFloat 32 + 7: TypeVector 6(float) 4 + 8: TypePointer Output 7(fvec4) + 9(fragColor): 8(ptr) Variable Output + 10: TypePointer Input 7(fvec4) + 11(Color): 10(ptr) Variable Input + 13: TypeImage 6(float) 2D sampled format:Unknown + 14: TypeSampledImage 13 + 15: TypePointer UniformConstant 14 + 16(sTexture): 15(ptr) Variable UniformConstant + 18: TypeVector 6(float) 2 + 19: TypePointer Input 18(fvec2) + 20(UV): 19(ptr) Variable Input + 23: TypeInt 32 0 + 24: 23(int) Constant 0 + 4(main): 2 Function None 3 + 5: Label + 12: 7(fvec4) Load 11(Color) + 17: 14 Load 16(sTexture) + 21: 18(fvec2) Load 20(UV) + 22: 7(fvec4) ImageSampleImplicitLod 17 21 + 25: 6(float) CompositeExtract 22 0 + 26: 7(fvec4) VectorTimesScalar 12 25 + Store 9(fragColor) 26 + Return + FunctionEnd diff --git a/Test/vk.relaxed.changeSet.frag b/Test/vk.relaxed.changeSet.frag new file mode 100755 index 000000000..e2f2403a6 --- /dev/null +++ b/Test/vk.relaxed.changeSet.frag @@ -0,0 +1,13 @@ +#version 460 + +layout(location = 0) out vec4 fragColor; + +uniform sampler2D sTexture; + +in vec4 Color; +in vec2 UV; + +void main() +{ + fragColor = Color * texture(sTexture, UV.st).r; +} diff --git a/Test/vk.relaxed.changeSet.vert b/Test/vk.relaxed.changeSet.vert new file mode 100755 index 000000000..419adba36 --- /dev/null +++ b/Test/vk.relaxed.changeSet.vert @@ -0,0 +1,16 @@ +#version 460 + +in vec2 aPos; +in vec2 aUV; +in vec4 aColor; +uniform mat4 projectionMatrix; + +out vec4 Color; +out vec2 UV; + +void main() +{ + Color = aColor; + UV = aUV; + gl_Position = projectionMatrix * vec4(aPos, 0, 1); +} diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp index e85196c44..d02eae6fc 100644 --- a/glslang/MachineIndependent/ShaderLang.cpp +++ b/glslang/MachineIndependent/ShaderLang.cpp @@ -2124,7 +2124,12 @@ bool TProgram::crossStageCheck(EShMessages) { // copy final definition of global block back into each stage for (unsigned int i = 0; i < activeStages.size(); ++i) { - activeStages[i]->mergeGlobalUniformBlocks(*infoSink, uniforms); + // We only want to merge into already existing global uniform blocks. + // A stage that doesn't already know about the global doesn't care about it's content. + // Otherwise we end up pointing to the same object between different stages + // and that will break binding/set remappings + bool mergeExistingOnly = true; + activeStages[i]->mergeGlobalUniformBlocks(*infoSink, uniforms, mergeExistingOnly); } // compare cross stage symbols for each stage boundary diff --git a/glslang/MachineIndependent/linkValidate.cpp b/glslang/MachineIndependent/linkValidate.cpp index 789dc3c30..33af7bbe0 100644 --- a/glslang/MachineIndependent/linkValidate.cpp +++ b/glslang/MachineIndependent/linkValidate.cpp @@ -108,7 +108,8 @@ void TIntermediate::mergeUniformObjects(TInfoSink& infoSink, TIntermediate& unit unitLinkerObjects.resize(end - unitLinkerObjects.begin()); // merge uniforms and do error checking - mergeGlobalUniformBlocks(infoSink, unit); + bool mergeExistingOnly = false; + mergeGlobalUniformBlocks(infoSink, unit, mergeExistingOnly); mergeLinkerObjects(infoSink, linkerObjects, unitLinkerObjects, unit.getStage()); } @@ -362,7 +363,8 @@ void TIntermediate::mergeTrees(TInfoSink& infoSink, TIntermediate& unit) remapIds(idMaps, idShift + 1, unit); mergeBodies(infoSink, globals, unitGlobals); - mergeGlobalUniformBlocks(infoSink, unit); + bool mergeExistingOnly = false; + mergeGlobalUniformBlocks(infoSink, unit, mergeExistingOnly); mergeLinkerObjects(infoSink, linkerObjects, unitLinkerObjects, unit.getStage()); ioAccessed.insert(unit.ioAccessed.begin(), unit.ioAccessed.end()); } @@ -525,7 +527,7 @@ static inline bool isSameInterface(TIntermSymbol* symbol, EShLanguage stage, TIn // merge the members of different stages to allow them to be linked properly // as a single block // -void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit) +void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit, bool mergeExistingOnly) { TIntermSequence& linkerObjects = findLinkerObjects()->getSequence(); TIntermSequence& unitLinkerObjects = unit.findLinkerObjects()->getSequence(); @@ -552,7 +554,7 @@ void TIntermediate::mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& auto itUnitBlock = unitDefaultBlocks.begin(); for (; itUnitBlock != unitDefaultBlocks.end(); itUnitBlock++) { - bool add = true; + bool add = !mergeExistingOnly; auto itBlock = defaultBlocks.begin(); for (; itBlock != defaultBlocks.end(); itBlock++) { diff --git a/glslang/MachineIndependent/localintermediate.h b/glslang/MachineIndependent/localintermediate.h index 9bfa35cc8..c9a1d8114 100644 --- a/glslang/MachineIndependent/localintermediate.h +++ b/glslang/MachineIndependent/localintermediate.h @@ -915,7 +915,7 @@ public: void merge(TInfoSink&, TIntermediate&); void finalCheck(TInfoSink&, bool keepUncalled); - void mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit); + void mergeGlobalUniformBlocks(TInfoSink& infoSink, TIntermediate& unit, bool mergeExistingOnly); void mergeUniformObjects(TInfoSink& infoSink, TIntermediate& unit); void checkStageIO(TInfoSink&, TIntermediate&); diff --git a/gtests/VkRelaxed.FromFile.cpp b/gtests/VkRelaxed.FromFile.cpp index d791d6cc6..32e3c29b8 100644 --- a/gtests/VkRelaxed.FromFile.cpp +++ b/gtests/VkRelaxed.FromFile.cpp @@ -48,6 +48,7 @@ namespace { struct vkRelaxedData { std::vector fileNames; + std::vector> resourceSetBindings; }; using VulkanRelaxedTest = GlslangTest <::testing::TestWithParam>; @@ -191,6 +192,7 @@ bool verifyIOMapping(std::string& linkingError, glslang::TProgram& program) { TEST_P(VulkanRelaxedTest, FromFile) { const auto& fileNames = GetParam().fileNames; + const auto& resourceSetBindings = GetParam().resourceSetBindings; Semantics semantics = Semantics::Vulkan; const size_t fileCount = fileNames.size(); const EShMessages controls = DeriveOptions(Source::GLSL, semantics, Target::BothASTAndSpv); @@ -230,6 +232,12 @@ TEST_P(VulkanRelaxedTest, FromFile) result.linkingOutput = program.getInfoLog(); result.linkingError = program.getInfoDebugLog(); + if (!resourceSetBindings.empty()) { + assert(resourceSetBindings.size() == fileNames.size()); + for (int i = 0; i < shaders.size(); i++) + shaders[i]->setResourceSetBinding(resourceSetBindings[i]); + } + unsigned int stage = 0; glslang::TIntermediate* firstIntermediate = nullptr; while (!program.getIntermediate((EShLanguage)stage) && stage < EShLangCount) { stage++; } @@ -287,6 +295,7 @@ INSTANTIATE_TEST_SUITE_P( {{"vk.relaxed.link1.frag", "vk.relaxed.link2.frag"}}, {{"vk.relaxed.stagelink.vert", "vk.relaxed.stagelink.frag"}}, {{"vk.relaxed.errorcheck.vert", "vk.relaxed.errorcheck.frag"}}, + {{"vk.relaxed.changeSet.vert", "vk.relaxed.changeSet.frag" }, { {"0"}, {"1"} } }, })) ); // clang-format on