From 82e95a3aa5c41e3cfdf045e22cf131922e529b48 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Sat, 26 Aug 2017 15:47:25 -0600 Subject: [PATCH] SPV: Add auto location mapping of non-opaque non-block uniform variables. Fix #1019. --- Test/baseResults/glspv.frag.out | 3 +- .../spv.looseUniformNoLoc.vert.out | 8 +++++ Test/baseResults/spv.noBuiltInLoc.vert.out | 27 +++++++++++++++-- Test/runtests | 4 ++- Test/spv.looseUniformNoLoc.vert | 15 ++++++++++ Test/spv.noBuiltInLoc.vert | 4 +++ glslang/MachineIndependent/ParseHelper.cpp | 4 +-- glslang/MachineIndependent/iomapper.cpp | 30 +++++++++++++++++-- glslang/Public/ShaderLang.h | 10 +++++-- 9 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 Test/baseResults/spv.looseUniformNoLoc.vert.out create mode 100644 Test/spv.looseUniformNoLoc.vert diff --git a/Test/baseResults/glspv.frag.out b/Test/baseResults/glspv.frag.out index 36afc8317..5622d0182 100755 --- a/Test/baseResults/glspv.frag.out +++ b/Test/baseResults/glspv.frag.out @@ -1,10 +1,9 @@ glspv.frag ERROR: 0:4: '#error' : GL_SPIRV is set ( correct , not an error ) ERROR: 0:6: '#error' : GL_SPIR is 100 -ERROR: 0:14: 'f' : non-opaque uniform variables need a layout(location=L) ERROR: 0:19: 'input_attachment_index' : only allowed when using GLSL for Vulkan ERROR: 0:19: '' : syntax error, unexpected IDENTIFIER, expecting LEFT_BRACE or COMMA or SEMICOLON -ERROR: 5 compilation errors. No code generated. +ERROR: 4 compilation errors. No code generated. SPIR-V is not generated for failed compile or link diff --git a/Test/baseResults/spv.looseUniformNoLoc.vert.out b/Test/baseResults/spv.looseUniformNoLoc.vert.out new file mode 100644 index 000000000..55d1639cb --- /dev/null +++ b/Test/baseResults/spv.looseUniformNoLoc.vert.out @@ -0,0 +1,8 @@ +spv.looseUniformNoLoc.vert +ERROR: spv.looseUniformNoLoc.vert:9: 'uv' : non-opaque uniform variables need a layout(location=L) +ERROR: 1 compilation errors. No code generated. + + +ERROR: Linking vertex stage: Missing entry point: Each stage requires one entry point + +SPIR-V is not generated for failed compile or link diff --git a/Test/baseResults/spv.noBuiltInLoc.vert.out b/Test/baseResults/spv.noBuiltInLoc.vert.out index 3d980bf0f..d6cbeaa87 100644 --- a/Test/baseResults/spv.noBuiltInLoc.vert.out +++ b/Test/baseResults/spv.noBuiltInLoc.vert.out @@ -1,12 +1,12 @@ spv.noBuiltInLoc.vert // Module Version 10000 // Generated by (magic number): 80001 -// Id's are bound by 23 +// Id's are bound by 33 Capability Shader 1: ExtInstImport "GLSL.std.450" MemoryModel Logical GLSL450 - EntryPoint Vertex 4 "main" 9 11 18 + EntryPoint Vertex 4 "main" 9 11 18 31 32 Source GLSL 450 Name 4 "main" Name 9 "bar" @@ -17,6 +17,11 @@ spv.noBuiltInLoc.vert MemberName 16(gl_PerVertex) 2 "gl_ClipDistance" MemberName 16(gl_PerVertex) 3 "gl_CullDistance" Name 18 "" + Name 24 "uv1" + Name 26 "uv2" + Name 29 "uv3" + Name 31 "gl_VertexID" + Name 32 "gl_InstanceID" Decorate 9(bar) Location 0 Decorate 11(foo) Location 0 MemberDecorate 16(gl_PerVertex) 0 BuiltIn Position @@ -24,6 +29,14 @@ spv.noBuiltInLoc.vert MemberDecorate 16(gl_PerVertex) 2 BuiltIn ClipDistance MemberDecorate 16(gl_PerVertex) 3 BuiltIn CullDistance Decorate 16(gl_PerVertex) Block + Decorate 24(uv1) Location 0 + Decorate 24(uv1) DescriptorSet 0 + Decorate 26(uv2) Location 1 + Decorate 26(uv2) DescriptorSet 0 + Decorate 29(uv3) Location 2 + Decorate 29(uv3) DescriptorSet 0 + Decorate 31(gl_VertexID) BuiltIn VertexId + Decorate 32(gl_InstanceID) BuiltIn InstanceId 2: TypeVoid 3: TypeFunction 2 6: TypeFloat 32 @@ -40,6 +53,16 @@ spv.noBuiltInLoc.vert 18: 17(ptr) Variable Output 19: TypeInt 32 1 20: 19(int) Constant 0 + 23: TypePointer UniformConstant 7(fvec4) + 24(uv1): 23(ptr) Variable UniformConstant + 25: TypePointer UniformConstant 6(float) + 26(uv2): 25(ptr) Variable UniformConstant + 27: TypeVector 6(float) 3 + 28: TypePointer UniformConstant 27(fvec3) + 29(uv3): 28(ptr) Variable UniformConstant + 30: TypePointer Input 19(int) + 31(gl_VertexID): 30(ptr) Variable Input +32(gl_InstanceID): 30(ptr) Variable Input 4(main): 2 Function None 3 5: Label 12: 7(fvec4) Load 11(foo) diff --git a/Test/runtests b/Test/runtests index 98cd944db..f0da75f54 100755 --- a/Test/runtests +++ b/Test/runtests @@ -109,8 +109,10 @@ diff -b $BASEDIR/hlsl.explicitDescriptorSet-2.frag.out $TARGETDIR/hlsl.explicitD echo Testing SPV no location $EXE -V -C spv.noLocation.vert > $TARGETDIR/spv.noLocation.vert.out diff -b $BASEDIR/spv.noLocation.vert.out $TARGETDIR/spv.noLocation.vert.out || HASERROR=1 -$EXE -H --aml spv.noBuiltInLoc.vert > $TARGETDIR/spv.noBuiltInLoc.vert.out +$EXE -G -H --aml spv.noBuiltInLoc.vert > $TARGETDIR/spv.noBuiltInLoc.vert.out diff -b $BASEDIR/spv.noBuiltInLoc.vert.out $TARGETDIR/spv.noBuiltInLoc.vert.out || HASERROR=1 +$EXE -G spv.looseUniformNoLoc.vert > $TARGETDIR/spv.looseUniformNoLoc.vert.out +diff -b $BASEDIR/spv.looseUniformNoLoc.vert.out $TARGETDIR/spv.looseUniformNoLoc.vert.out || HASERROR=1 # # Testing debug information diff --git a/Test/spv.looseUniformNoLoc.vert b/Test/spv.looseUniformNoLoc.vert new file mode 100644 index 000000000..e88735944 --- /dev/null +++ b/Test/spv.looseUniformNoLoc.vert @@ -0,0 +1,15 @@ +#version 450 core + +layout(location = 0) +in vec4 foo; + +layout(location = 0) +out vec4 bar; + +uniform vec4 uv; + +void main() +{ + bar = foo; + gl_Position = foo; +} \ No newline at end of file diff --git a/Test/spv.noBuiltInLoc.vert b/Test/spv.noBuiltInLoc.vert index e75489272..5fbe25219 100644 --- a/Test/spv.noBuiltInLoc.vert +++ b/Test/spv.noBuiltInLoc.vert @@ -6,6 +6,10 @@ in vec4 foo; layout(location = 0) out vec4 bar; +uniform vec4 uv1; +uniform float uv2; +uniform vec3 uv3; + void main() { bar = foo; diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 7902db8a1..6decfea72 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -2493,8 +2493,8 @@ void TParseContext::transparentOpaqueCheck(const TSourceLoc& loc, const TType& t // Vulkan doesn't allow transparent uniforms outside of blocks if (spvVersion.vulkan > 0) vulkanRemoved(loc, "non-opaque uniforms outside a block"); - // OpenGL wants locations on these - if (spvVersion.openGl > 0 && !type.getQualifier().hasLocation()) + // OpenGL wants locations on these (unless they are getting automapped) + if (spvVersion.openGl > 0 && !type.getQualifier().hasLocation() && !intermediate.getAutoMapLocations()) error(loc, "non-opaque uniform variables need a layout(location=L)", identifier.c_str(), ""); } } diff --git a/glslang/MachineIndependent/iomapper.cpp b/glslang/MachineIndependent/iomapper.cpp index fc85e9789..59db1adee 100644 --- a/glslang/MachineIndependent/iomapper.cpp +++ b/glslang/MachineIndependent/iomapper.cpp @@ -253,10 +253,14 @@ struct TResolverUniformAdaptor ent.newBinding = -1; ent.newSet = -1; ent.newIndex = -1; - const bool isValid = resolver.validateBinding(stage, ent.symbol->getName().c_str(), ent.symbol->getType(), ent.live); + const bool isValid = resolver.validateBinding(stage, ent.symbol->getName().c_str(), ent.symbol->getType(), + ent.live); if (isValid) { - ent.newBinding = resolver.resolveBinding(stage, ent.symbol->getName().c_str(), ent.symbol->getType(), ent.live); + ent.newBinding = resolver.resolveBinding(stage, ent.symbol->getName().c_str(), ent.symbol->getType(), + ent.live); ent.newSet = resolver.resolveSet(stage, ent.symbol->getName().c_str(), ent.symbol->getType(), ent.live); + ent.newLocation = resolver.resolveUniformLocation(stage, ent.symbol->getName().c_str(), + ent.symbol->getType(), ent.live); if (ent.newBinding != -1) { if (ent.newBinding >= int(TQualifier::layoutBindingEnd)) { @@ -356,6 +360,7 @@ struct TDefaultIoResolverBase : public glslang::TIoMapResolver std::vector baseResourceSetBinding; bool doAutoBindingMapping; bool doAutoLocationMapping; + int nextUniformLocation; typedef std::vector TSlotSet; typedef std::unordered_map TSlotSetMap; TSlotSetMap slots; @@ -411,7 +416,27 @@ struct TDefaultIoResolverBase : public glslang::TIoMapResolver return 0; } + int resolveUniformLocation(EShLanguage /*stage*/, const char* /*name*/, const glslang::TType& type, bool is_live) override + { + // kick out of not doing this + if (!doAutoLocationMapping) + return -1; + // no locations added if already present, a built-in variable, a block, or an opaque + if (type.getQualifier().hasLocation() || type.isBuiltIn() || + type.getBasicType() == EbtBlock || type.containsOpaque()) + return -1; + + // no locations on blocks of built-in variables + if (type.isStruct()) { + if (type.getStruct()->size() < 1) + return -1; + if ((*type.getStruct())[0].type->isBuiltIn()) + return -1; + } + + return nextUniformLocation++; + } bool validateInOut(EShLanguage /*stage*/, const char* /*name*/, const TType& /*type*/, bool /*is_live*/) override { return true; @@ -700,6 +725,7 @@ bool TIoMapper::addStage(EShLanguage stage, TIntermediate &intermediate, TInfoSi resolverBase->baseResourceSetBinding = intermediate.getResourceSetBinding(); resolverBase->doAutoBindingMapping = intermediate.getAutoMapBindings(); resolverBase->doAutoLocationMapping = intermediate.getAutoMapLocations(); + resolverBase->nextUniformLocation = 0; resolver = resolverBase; } diff --git a/glslang/Public/ShaderLang.h b/glslang/Public/ShaderLang.h index b968967d2..b36e384fa 100644 --- a/glslang/Public/ShaderLang.h +++ b/glslang/Public/ShaderLang.h @@ -533,9 +533,10 @@ class TIoMapper; // Allows to customize the binding layout after linking. // All used uniform variables will invoke at least validateBinding. -// If validateBinding returned true then the other resolveBinding -// and resolveSet are invoked to resolve the binding and descriptor -// set index respectively. +// If validateBinding returned true then the other resolveBinding, +// resolveSet, and resolveLocation are invoked to resolve the binding +// and descriptor set index respectively. +// // Invocations happen in a particular order: // 1) all shader inputs // 2) all shader outputs @@ -567,6 +568,9 @@ public: // Should return a value >= 0 if the current set should be overridden. // Return -1 if the current set (including no set) should be kept. virtual int resolveSet(EShLanguage stage, const char* name, const TType& type, bool is_live) = 0; + // Should return a value >= 0 if the current location should be overridden. + // Return -1 if the current location (including no location) should be kept. + virtual int resolveUniformLocation(EShLanguage stage, const char* name, const TType& type, bool is_live) = 0; // Should return true if the resulting/current setup would be okay. // Basic idea is to do aliasing checks and reject invalid semantic names. virtual bool validateInOut(EShLanguage stage, const char* name, const TType& type, bool is_live) = 0;