Commit 33a00efd by Courtney Goeltzenleuchter Committed by Commit Bot

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: 's avatarGeoff Lang <geofflang@chromium.org>
parent 5f3456e3
......@@ -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 GetGeometryShaderInputPrimitiveType(const ShHandle handle);
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.
......
......@@ -100,6 +100,8 @@ struct ShaderVariable
// 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
......
......@@ -8,6 +8,7 @@
#include "common/utilities.h"
#include <GLSLANG/ShaderVars.h>
#include "GLES3/gl3.h"
#include "common/mathutil.h"
#include "common/platform.h"
......
......@@ -100,7 +100,7 @@ ShaderVariable *FindVariableInInterfaceBlock(const ImmutableString &name,
}
// 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 @@ class CollectVariablesTraverser : public TIntermTraverser
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 @@ class CollectVariablesTraverser : public TIntermTraverser
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 @@ class CollectVariablesTraverser : public TIntermTraverser
bool mPrimitiveIDAdded;
bool mLayerAdded;
// Shared memory variables
bool mSharedVariableAdded;
ShHashFunction64 mHashFunction;
GLenum mShaderType;
......@@ -221,6 +226,7 @@ CollectVariablesTraverser::CollectVariablesTraverser(
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 @@ CollectVariablesTraverser::CollectVariablesTraverser(
mUniforms(uniforms),
mInputVaryings(inputVaryings),
mOutputVaryings(outputVaryings),
mSharedVariables(sharedVariables),
mUniformBlocks(uniformBlocks),
mShaderStorageBlocks(shaderStorageBlocks),
mInBlocks(inBlocks),
......@@ -266,6 +273,7 @@ CollectVariablesTraverser::CollectVariablesTraverser(
mInvocationIDAdded(false),
mPrimitiveIDAdded(false),
mLayerAdded(false),
mSharedVariableAdded(false),
mHashFunction(hashFunction),
mShaderType(shaderType),
mExtensionBehavior(extensionBehavior)
......@@ -283,12 +291,8 @@ void CollectVariablesTraverser::setBuiltInInfoFromSymbol(const TVariable &variab
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 @@ void CollectVariablesTraverser::recordBuiltInVaryingUsed(const TVariable &variab
{
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 @@ void CollectVariablesTraverser::recordBuiltInFragmentOutputUsed(const TVariable
{
ShaderVariable info;
setBuiltInInfoFromSymbol(variable, &info);
info.staticUse = true;
info.active = true;
info.active = true;
mOutputVariables->push_back(info);
(*addedFlag) = true;
}
......@@ -329,9 +332,8 @@ void CollectVariablesTraverser::recordBuiltInAttributeUsed(const TVariable &vari
{
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 @@ void CollectVariablesTraverser::visitSymbol(TIntermSymbol *symbol)
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 @@ void CollectVariablesTraverser::visitSymbol(TIntermSymbol *symbol)
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 @@ void CollectVariables(TIntermBlock *root,
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 @@ void CollectVariables(TIntermBlock *root,
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);
}
......
......@@ -24,6 +24,7 @@ void CollectVariables(TIntermBlock *root,
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,
......
......@@ -487,6 +487,17 @@ void TCompiler::setASTMetadata(const TParseContext &parseContext)
}
}
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 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
{
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 @@ void TCompiler::clearResults()
mUniforms.clear();
mInputVaryings.clear();
mOutputVaryings.clear();
mSharedVariables.clear();
mInterfaceBlocks.clear();
mUniformBlocks.clear();
mShaderStorageBlocks.clear();
......
......@@ -141,6 +141,9 @@ class TCompiler : public TShHandleBase
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 @@ class TCompiler : public TShHandleBase
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;
......
......@@ -671,4 +671,16 @@ int GetGeometryShaderMaxVertices(const ShHandle handle)
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
......@@ -181,6 +181,29 @@ unsigned int ShaderVariable::getBasicTypeElementCount() const
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
......
......@@ -352,6 +352,8 @@ void Shader::compile(const Context *context)
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 @@ void Shader::resolveCompile()
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:
......
......@@ -217,6 +217,7 @@ class Shader final : angle::NonCopyable, public LabeledObject
ShaderProgramManager *mResourceManager;
GLuint mCurrentMaxComputeWorkGroupInvocations;
unsigned int mMaxComputeSharedMemory;
};
bool CompareShaderVar(const sh::ShaderVariable &x, const sh::ShaderVariable &y);
......
......@@ -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):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment