From 1c6280646873d3267727fdecfd8e5a65b266e70b Mon Sep 17 00:00:00 2001 From: Jeremy Hayes Date: Mon, 22 Feb 2021 18:21:25 -0700 Subject: [PATCH] Require fixed workgroup size declaration Fix 2479. --- .../negativeWorkGroupSize.comp.out | 69 +++++++++++++++++++ Test/negativeWorkGroupSize.comp | 12 ++++ glslang/MachineIndependent/ParseHelper.cpp | 5 ++ .../MachineIndependent/localintermediate.h | 12 ++++ gtests/AST.FromFile.cpp | 1 + 5 files changed, 99 insertions(+) create mode 100644 Test/baseResults/negativeWorkGroupSize.comp.out create mode 100644 Test/negativeWorkGroupSize.comp diff --git a/Test/baseResults/negativeWorkGroupSize.comp.out b/Test/baseResults/negativeWorkGroupSize.comp.out new file mode 100644 index 00000000..7ae37985 --- /dev/null +++ b/Test/baseResults/negativeWorkGroupSize.comp.out @@ -0,0 +1,69 @@ +negativeWorkGroupSize.comp +ERROR: 0:4: 'initializer' : can't read from gl_WorkGroupSize before a fixed workgroup size has been declared +ERROR: 1 compilation errors. No code generated. + + +Shader version: 460 +local_size = (64, 1, 1) +ERROR: node is still EOpNull! +0:3 Function Definition: fn( ( global void) +0:3 Function Parameters: +0:4 Sequence +0:4 Sequence +0:4 move second child to first child ( temp 3-component vector of uint) +0:4 'wgs' ( temp 3-component vector of uint) +0:4 Constant: +0:4 1 (const uint) +0:4 1 (const uint) +0:4 1 (const uint) +0:9 Function Definition: main( ( global void) +0:9 Function Parameters: +0:10 Sequence +0:10 Function Call: fn( ( global void) +0:11 Sequence +0:11 move second child to first child ( temp 3-component vector of uint) +0:11 'wgs' ( temp 3-component vector of uint) +0:11 Constant: +0:11 64 (const uint) +0:11 1 (const uint) +0:11 1 (const uint) +0:? Linker Objects +0:? 'gl_WorkGroupSize' ( const 3-component vector of uint WorkGroupSize) +0:? 64 (const uint) +0:? 1 (const uint) +0:? 1 (const uint) + + +Linked compute stage: + + +Shader version: 460 +local_size = (64, 1, 1) +ERROR: node is still EOpNull! +0:3 Function Definition: fn( ( global void) +0:3 Function Parameters: +0:4 Sequence +0:4 Sequence +0:4 move second child to first child ( temp 3-component vector of uint) +0:4 'wgs' ( temp 3-component vector of uint) +0:4 Constant: +0:4 1 (const uint) +0:4 1 (const uint) +0:4 1 (const uint) +0:9 Function Definition: main( ( global void) +0:9 Function Parameters: +0:10 Sequence +0:10 Function Call: fn( ( global void) +0:11 Sequence +0:11 move second child to first child ( temp 3-component vector of uint) +0:11 'wgs' ( temp 3-component vector of uint) +0:11 Constant: +0:11 64 (const uint) +0:11 1 (const uint) +0:11 1 (const uint) +0:? Linker Objects +0:? 'gl_WorkGroupSize' ( const 3-component vector of uint WorkGroupSize) +0:? 64 (const uint) +0:? 1 (const uint) +0:? 1 (const uint) + diff --git a/Test/negativeWorkGroupSize.comp b/Test/negativeWorkGroupSize.comp new file mode 100644 index 00000000..1f007d0c --- /dev/null +++ b/Test/negativeWorkGroupSize.comp @@ -0,0 +1,12 @@ +#version 460 + +void fn(){ + uvec3 wgs = gl_WorkGroupSize; // error: fixed workgroup size has not been declared +} + +layout(local_size_x = 64) in; // declare workgroup size + +void main(){ + fn(); + uvec3 wgs = gl_WorkGroupSize; // valid +} diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 88c278af..ea530cef 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -2756,6 +2756,11 @@ void TParseContext::rValueErrorCheck(const TSourceLoc& loc, const char* op, TInt if (!(symNode && symNode->getQualifier().isWriteOnly())) // base class checks if (symNode && symNode->getQualifier().isExplicitInterpolation()) error(loc, "can't read from explicitly-interpolated object: ", op, symNode->getName().c_str()); + + // local_size_{xyz} must be assigned or specialized before gl_WorkGroupSize can be assigned. + if(node->getQualifier().builtIn == EbvWorkGroupSize && + !(intermediate.isLocalSizeSet() || intermediate.isLocalSizeSpecialized())) + error(loc, "can't read from gl_WorkGroupSize before a fixed workgroup size has been declared", op, ""); } // diff --git a/glslang/MachineIndependent/localintermediate.h b/glslang/MachineIndependent/localintermediate.h index 1db3d1c3..9fe684a3 100644 --- a/glslang/MachineIndependent/localintermediate.h +++ b/glslang/MachineIndependent/localintermediate.h @@ -550,6 +550,11 @@ public: return true; } unsigned int getLocalSize(int dim) const { return localSize[dim]; } + bool isLocalSizeSet() const + { + // Return true if any component has been set (i.e. any component is not default). + return localSizeNotDefault[0] || localSizeNotDefault[1] || localSizeNotDefault[2]; + } bool setLocalSizeSpecId(int dim, int id) { if (localSizeSpecId[dim] != TQualifier::layoutNotSet) @@ -558,6 +563,13 @@ public: return true; } int getLocalSizeSpecId(int dim) const { return localSizeSpecId[dim]; } + bool isLocalSizeSpecialized() const + { + // Return true if any component has been specialized. + return localSizeSpecId[0] != TQualifier::layoutNotSet || + localSizeSpecId[1] != TQualifier::layoutNotSet || + localSizeSpecId[2] != TQualifier::layoutNotSet; + } #ifdef GLSLANG_WEB void output(TInfoSink&, bool tree) { } diff --git a/gtests/AST.FromFile.cpp b/gtests/AST.FromFile.cpp index dc7fea38..8885b291 100644 --- a/gtests/AST.FromFile.cpp +++ b/gtests/AST.FromFile.cpp @@ -280,6 +280,7 @@ INSTANTIATE_TEST_SUITE_P( "glsl.es320.subgroupVote.comp", "terminate.frag", "terminate.vert", + "negativeWorkGroupSize.comp", })), FileNameAsCustomTestSuffix );