Add Compute Shared Memory Size Validation Add tracking of shared memory declarations in compute shaders. Test: angle_deqp_gles31_tests --gtest_filter=dEQP.GLES31/functional_debug_negative_coverage_callbacks_compute_exceed_shared_memory_size_limit Bug: 4173 Change-Id: If2a86d467a82f73fa5b2ee0ced752701acfe1872 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1934653 Commit-Queue: Courtney Goeltzenleuchter <courtneygo@google.com> Reviewed-by: Geoff Lang <geofflang@chromium.org>
diff --git a/include/GLSLANG/ShaderLang.h b/include/GLSLANG/ShaderLang.h index 964e937..22fdf33 100644 --- a/include/GLSLANG/ShaderLang.h +++ b/include/GLSLANG/ShaderLang.h
@@ -26,7 +26,7 @@ // Version number for shader translation API. // It is incremented every time the API changes. -#define ANGLE_SH_VERSION 218 +#define ANGLE_SH_VERSION 219 enum ShShaderSpec { @@ -693,6 +693,7 @@ GLenum GetGeometryShaderOutputPrimitiveType(const ShHandle handle); int GetGeometryShaderInvocations(const ShHandle handle); int GetGeometryShaderMaxVertices(const ShHandle handle); +unsigned int GetShaderSharedMemorySize(const ShHandle handle); // // Helper function to identify specs that are based on the WebGL spec.
diff --git a/include/GLSLANG/ShaderVars.h b/include/GLSLANG/ShaderVars.h index e2a67a2..52f6ad0 100644 --- a/include/GLSLANG/ShaderVars.h +++ b/include/GLSLANG/ShaderVars.h
@@ -100,6 +100,8 @@ // ARRAY_SIZE value that can be queried through the API. unsigned int getBasicTypeElementCount() const; + unsigned int getExternalSize() const; + bool isStruct() const { return !fields.empty(); } // All of the shader's variables are described using nested data
diff --git a/src/common/utilities.cpp b/src/common/utilities.cpp index 45b6258..2345148 100644 --- a/src/common/utilities.cpp +++ b/src/common/utilities.cpp
@@ -8,6 +8,7 @@ #include "common/utilities.h" #include <GLSLANG/ShaderVars.h> +#include "GLES3/gl3.h" #include "common/mathutil.h" #include "common/platform.h"
diff --git a/src/compiler/translator/CollectVariables.cpp b/src/compiler/translator/CollectVariables.cpp index 08b53e3..4a5800e 100644 --- a/src/compiler/translator/CollectVariables.cpp +++ b/src/compiler/translator/CollectVariables.cpp
@@ -100,7 +100,7 @@ } // Traverses the intermediate tree to collect all attributes, uniforms, varyings, fragment outputs, -// and interface blocks. +// shared data and interface blocks. class CollectVariablesTraverser : public TIntermTraverser { public: @@ -109,6 +109,7 @@ std::vector<ShaderVariable> *uniforms, std::vector<ShaderVariable> *inputVaryings, std::vector<ShaderVariable> *outputVaryings, + std::vector<ShaderVariable> *sharedVariables, std::vector<InterfaceBlock> *uniformBlocks, std::vector<InterfaceBlock> *shaderStorageBlocks, std::vector<InterfaceBlock> *inBlocks, @@ -160,6 +161,7 @@ std::vector<ShaderVariable> *mUniforms; std::vector<ShaderVariable> *mInputVaryings; std::vector<ShaderVariable> *mOutputVaryings; + std::vector<ShaderVariable> *mSharedVariables; std::vector<InterfaceBlock> *mUniformBlocks; std::vector<InterfaceBlock> *mShaderStorageBlocks; std::vector<InterfaceBlock> *mInBlocks; @@ -209,6 +211,9 @@ bool mPrimitiveIDAdded; bool mLayerAdded; + // Shared memory variables + bool mSharedVariableAdded; + ShHashFunction64 mHashFunction; GLenum mShaderType; @@ -221,6 +226,7 @@ std::vector<sh::ShaderVariable> *uniforms, std::vector<sh::ShaderVariable> *inputVaryings, std::vector<sh::ShaderVariable> *outputVaryings, + std::vector<sh::ShaderVariable> *sharedVariables, std::vector<sh::InterfaceBlock> *uniformBlocks, std::vector<sh::InterfaceBlock> *shaderStorageBlocks, std::vector<sh::InterfaceBlock> *inBlocks, @@ -234,6 +240,7 @@ mUniforms(uniforms), mInputVaryings(inputVaryings), mOutputVaryings(outputVaryings), + mSharedVariables(sharedVariables), mUniformBlocks(uniformBlocks), mShaderStorageBlocks(shaderStorageBlocks), mInBlocks(inBlocks), @@ -266,6 +273,7 @@ mInvocationIDAdded(false), mPrimitiveIDAdded(false), mLayerAdded(false), + mSharedVariableAdded(false), mHashFunction(hashFunction), mShaderType(shaderType), mExtensionBehavior(extensionBehavior) @@ -283,12 +291,8 @@ info->name = variable.name().data(); info->mappedName = variable.name().data(); - info->type = GLVariableType(type); - info->precision = GLVariablePrecision(type); - if (auto *arraySizes = type.getArraySizes()) - { - info->arraySizes.assign(arraySizes->begin(), arraySizes->end()); - } + + setFieldOrVariableProperties(type, true, info); } void CollectVariablesTraverser::recordBuiltInVaryingUsed(const TVariable &variable, @@ -300,9 +304,9 @@ { ShaderVariable info; setBuiltInInfoFromSymbol(variable, &info); - info.staticUse = true; info.active = true; info.isInvariant = mSymbolTable->isVaryingInvariant(variable); + varyings->push_back(info); (*addedFlag) = true; } @@ -315,8 +319,7 @@ { ShaderVariable info; setBuiltInInfoFromSymbol(variable, &info); - info.staticUse = true; - info.active = true; + info.active = true; mOutputVariables->push_back(info); (*addedFlag) = true; } @@ -329,9 +332,8 @@ { ShaderVariable info; setBuiltInInfoFromSymbol(variable, &info); - info.staticUse = true; - info.active = true; - info.location = -1; + info.active = true; + info.location = -1; mAttribs->push_back(info); (*addedFlag) = true; } @@ -548,8 +550,7 @@ ASSERT(info.arraySizes.size() == 1u); info.arraySizes.back() = 1u; } - info.staticUse = true; - info.active = true; + info.active = true; mOutputVariables->push_back(info); mFragDataAdded = true; } @@ -601,6 +602,13 @@ IsExtensionEnabled(mExtensionBehavior, TExtension::OVR_multiview))); } break; + case EvqShared: + if (mShaderType == GL_COMPUTE_SHADER) + { + recordBuiltInVaryingUsed(symbol->variable(), &mSharedVariableAdded, + mSharedVariables); + } + break; default: break; } @@ -986,6 +994,7 @@ std::vector<ShaderVariable> *uniforms, std::vector<ShaderVariable> *inputVaryings, std::vector<ShaderVariable> *outputVaryings, + std::vector<ShaderVariable> *sharedVariables, std::vector<InterfaceBlock> *uniformBlocks, std::vector<InterfaceBlock> *shaderStorageBlocks, std::vector<InterfaceBlock> *inBlocks, @@ -995,8 +1004,9 @@ const TExtensionBehavior &extensionBehavior) { CollectVariablesTraverser collect(attributes, outputVariables, uniforms, inputVaryings, - outputVaryings, uniformBlocks, shaderStorageBlocks, inBlocks, - hashFunction, symbolTable, shaderType, extensionBehavior); + outputVaryings, sharedVariables, uniformBlocks, + shaderStorageBlocks, inBlocks, hashFunction, symbolTable, + shaderType, extensionBehavior); root->traverse(&collect); }
diff --git a/src/compiler/translator/CollectVariables.h b/src/compiler/translator/CollectVariables.h index 7d0cef5..e0d80b1 100644 --- a/src/compiler/translator/CollectVariables.h +++ b/src/compiler/translator/CollectVariables.h
@@ -24,6 +24,7 @@ std::vector<ShaderVariable> *uniforms, std::vector<ShaderVariable> *inputVaryings, std::vector<ShaderVariable> *outputVaryings, + std::vector<ShaderVariable> *sharedVariables, std::vector<InterfaceBlock> *uniformBlocks, std::vector<InterfaceBlock> *shaderStorageBlocks, std::vector<InterfaceBlock> *inBlocks,
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp index a5f0976..97e61fe 100644 --- a/src/compiler/translator/Compiler.cpp +++ b/src/compiler/translator/Compiler.cpp
@@ -487,6 +487,17 @@ } } +unsigned int TCompiler::getSharedMemorySize() const +{ + unsigned int sharedMemSize = 0; + for (const sh::ShaderVariable &var : mSharedVariables) + { + sharedMemSize += var.getExternalSize(); + } + + return sharedMemSize; +} + bool TCompiler::validateAST(TIntermNode *root) { if ((mCompileOptions & SH_VALIDATE_AST) != 0) @@ -767,8 +778,9 @@ { ASSERT(!mVariablesCollected); CollectVariables(root, &mAttributes, &mOutputVariables, &mUniforms, &mInputVaryings, - &mOutputVaryings, &mUniformBlocks, &mShaderStorageBlocks, &mInBlocks, - mResources.HashFunction, &mSymbolTable, mShaderType, mExtensionBehavior); + &mOutputVaryings, &mSharedVariables, &mUniformBlocks, + &mShaderStorageBlocks, &mInBlocks, mResources.HashFunction, &mSymbolTable, + mShaderType, mExtensionBehavior); collectInterfaceBlocks(); mVariablesCollected = true; if (compileOptions & SH_USE_UNUSED_STANDARD_SHARED_BLOCKS) @@ -1113,6 +1125,7 @@ mUniforms.clear(); mInputVaryings.clear(); mOutputVaryings.clear(); + mSharedVariables.clear(); mInterfaceBlocks.clear(); mUniformBlocks.clear(); mShaderStorageBlocks.clear();
diff --git a/src/compiler/translator/Compiler.h b/src/compiler/translator/Compiler.h index 8b8a546..c615511 100644 --- a/src/compiler/translator/Compiler.h +++ b/src/compiler/translator/Compiler.h
@@ -141,6 +141,9 @@ return mGeometryShaderOutputPrimitiveType; } + unsigned int getStructSize(const ShaderVariable &var) const; + unsigned int getSharedMemorySize() const; + sh::GLenum getShaderType() const { return mShaderType; } bool validateAST(TIntermNode *root); @@ -175,6 +178,7 @@ std::vector<sh::ShaderVariable> mUniforms; std::vector<sh::ShaderVariable> mInputVaryings; std::vector<sh::ShaderVariable> mOutputVaryings; + std::vector<sh::ShaderVariable> mSharedVariables; std::vector<sh::InterfaceBlock> mInterfaceBlocks; std::vector<sh::InterfaceBlock> mUniformBlocks; std::vector<sh::InterfaceBlock> mShaderStorageBlocks;
diff --git a/src/compiler/translator/ShaderLang.cpp b/src/compiler/translator/ShaderLang.cpp index f4b7b9d..e4c51e5 100644 --- a/src/compiler/translator/ShaderLang.cpp +++ b/src/compiler/translator/ShaderLang.cpp
@@ -671,4 +671,16 @@ return maxVertices; } +unsigned int GetShaderSharedMemorySize(const ShHandle handle) +{ + ASSERT(handle); + + TShHandleBase *base = static_cast<TShHandleBase *>(handle); + TCompiler *compiler = base->getAsCompiler(); + ASSERT(compiler); + + unsigned int sharedMemorySize = compiler->getSharedMemorySize(); + return sharedMemorySize; +} + } // namespace sh
diff --git a/src/compiler/translator/ShaderVars.cpp b/src/compiler/translator/ShaderVars.cpp index 3cebc70..f159b3d 100644 --- a/src/compiler/translator/ShaderVars.cpp +++ b/src/compiler/translator/ShaderVars.cpp
@@ -181,6 +181,29 @@ return 1u; } +unsigned int ShaderVariable::getExternalSize() const +{ + unsigned int memorySize = 0; + + if (isStruct()) + { + // Have a structure, need to compute the structure size. + for (const auto &field : fields) + { + memorySize += field.getArraySizeProduct() * field.getExternalSize(); + } + } + else + { + memorySize += gl::VariableExternalSize(type); + } + + // multiply by array size to get total memory size of this variable / struct. + memorySize *= getArraySizeProduct(); + + return memorySize; +} + bool ShaderVariable::findInfoByMappedName(const std::string &mappedFullName, const ShaderVariable **leafVar, std::string *originalFullName) const
diff --git a/src/libANGLE/Shader.cpp b/src/libANGLE/Shader.cpp index 4a41549..04b0dc0 100644 --- a/src/libANGLE/Shader.cpp +++ b/src/libANGLE/Shader.cpp
@@ -352,6 +352,8 @@ mCurrentMaxComputeWorkGroupInvocations = static_cast<GLuint>(context->getCaps().maxComputeWorkGroupInvocations); + mMaxComputeSharedMemory = context->getCaps().maxComputeSharedMemorySize; + ASSERT(mBoundCompiler.get()); ShCompilerInstance compilerInstance = mBoundCompiler->getInstance(mState.mShaderType); ShHandle compilerHandle = compilerInstance.getHandle(); @@ -461,6 +463,14 @@ return; } } + + unsigned int sharedMemSize = sh::GetShaderSharedMemorySize(compilerHandle); + if (sharedMemSize > mMaxComputeSharedMemory) + { + WARN() << std::endl << "Exceeded maximum shared memory size"; + mState.mCompileStatus = CompileStatus::NOT_COMPILED; + return; + } break; } case ShaderType::Vertex:
diff --git a/src/libANGLE/Shader.h b/src/libANGLE/Shader.h index ed4e73b..707ac89 100644 --- a/src/libANGLE/Shader.h +++ b/src/libANGLE/Shader.h
@@ -217,6 +217,7 @@ ShaderProgramManager *mResourceManager; GLuint mCurrentMaxComputeWorkGroupInvocations; + unsigned int mMaxComputeSharedMemory; }; bool CompareShaderVar(const sh::ShaderVariable &x, const sh::ShaderVariable &y);
diff --git a/src/tests/deqp_support/deqp_gles31_test_expectations.txt b/src/tests/deqp_support/deqp_gles31_test_expectations.txt index 411b0b1..c07d340 100644 --- a/src/tests/deqp_support/deqp_gles31_test_expectations.txt +++ b/src/tests/deqp_support/deqp_gles31_test_expectations.txt
@@ -565,9 +565,6 @@ 2324 : dEQP-GLES31.functional.debug.negative_coverage.get_error.vertex_array.draw_range_elements_incomplete_primitive = FAIL // These tests are failing because of compile errors with SSBOs in compute shaders. -1951 D3D11 : dEQP-GLES31.functional.debug.negative_coverage.callbacks.compute.exceed_shared_memory_size_limit = FAIL -1951 D3D11 : dEQP-GLES31.functional.debug.negative_coverage.get_error.compute.exceed_shared_memory_size_limit = FAIL -1951 D3D11 : dEQP-GLES31.functional.debug.negative_coverage.log.compute.exceed_shared_memory_size_limit = FAIL 1442 D3D11 : dEQP-GLES31.functional.stencil_texturing.* = SKIP // TODO(xinghua.cao@intel.com): FAIL expectation instead of SKIP should be sufficient for OpenGL, but the @@ -643,7 +640,6 @@ 3579 ANDROID VULKAN : dEQP-GLES31.functional.fbo.no_attachments.* = SKIP // Debug: -3590 VULKAN : dEQP-GLES31.functional.debug.negative_coverage.*exceed_shared_memory_size_limit = FAIL 3590 SWIFTSHADER : dEQP-GLES31.functional.debug.negative_coverage.get_error.buffer.framebuffer_texture2d = FAIL // Stencil textures (some missing support for base level):