Add GLSL support for runtime-sized arrays in SSBOs
The GLSL parser now allows a runtime-sized array as the last member in
a shader storage block. Clamping indexing against the memory bounds is
done by determining the array length at runtime.
Runtime-sized arrays are used in dEQP tests for many compute shader
tests, so these now work on the OpenGL backend.
BUG=angleproject:1951
TEST=angle_unittests,
dEQP-GLES31.functional.shaders.linkage.shader_storage_block.*
dEQP-GLES31.functional.shaders.builtin_functions.*compute*
Change-Id: Ibecca24623ca8e4723af6f0e0421fe9711ea828d
Reviewed-on: https://chromium-review.googlesource.com/787976
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index 1d92ee0..a57ffcb 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -1390,7 +1390,8 @@
if (mOp == EOpArrayLength)
{
- if (mOperand->hasSideEffects())
+ // The size of runtime-sized arrays may only be determined at runtime.
+ if (mOperand->hasSideEffects() || mOperand->getType().isUnsizedArray())
{
return this;
}
diff --git a/src/compiler/translator/OutputGLSLBase.cpp b/src/compiler/translator/OutputGLSLBase.cpp
index 9a47287..edaf2eb 100644
--- a/src/compiler/translator/OutputGLSLBase.cpp
+++ b/src/compiler/translator/OutputGLSLBase.cpp
@@ -526,24 +526,42 @@
}
else if (visit == PostVisit)
{
- int maxSize;
TIntermTyped *left = node->getLeft();
TType leftType = left->getType();
- if (left->isArray())
- {
- // The shader will fail validation if the array length is not > 0.
- maxSize = static_cast<int>(leftType.getOutermostArraySize()) - 1;
- }
- else
- {
- maxSize = leftType.getNominalSize() - 1;
- }
-
if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
- out << "), 0.0, float(" << maxSize << ")))]";
+ out << "), 0.0, float(";
else
- out << ", 0, " << maxSize << ")]";
+ out << ", 0, ";
+
+ if (leftType.isUnsizedArray())
+ {
+ // For runtime-sized arrays in ESSL 3.10 we need to call the length method
+ // to get the length to clamp against. See ESSL 3.10 section 4.1.9. Note
+ // that a runtime-sized array expression is guaranteed not to have side
+ // effects, so it's fine to add the expression to the output twice.
+ ASSERT(mShaderVersion >= 310);
+ ASSERT(!left->hasSideEffects());
+ left->traverse(this);
+ out << ".length() - 1";
+ }
+ else
+ {
+ int maxSize;
+ if (leftType.isArray())
+ {
+ maxSize = static_cast<int>(leftType.getOutermostArraySize()) - 1;
+ }
+ else
+ {
+ maxSize = leftType.getNominalSize() - 1;
+ }
+ out << maxSize;
+ }
+ if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
+ out << ")))]";
+ else
+ out << ")]";
}
}
else
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 7417ab7..67207ca 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -3785,6 +3785,15 @@
fieldType->setLayoutQualifier(fieldLayoutQualifier);
+ if (mShaderVersion < 310 || memberIndex != fieldList->size() - 1u ||
+ typeQualifier.qualifier != EvqBuffer)
+ {
+ // ESSL 3.10 spec section 4.1.9 allows for runtime-sized arrays.
+ checkIsNotUnsizedArray(field->line(),
+ "array members of interface blocks must specify a size",
+ field->name().c_str(), field->type());
+ }
+
if (typeQualifier.qualifier == EvqBuffer)
{
// set memory qualifiers
@@ -4007,85 +4016,88 @@
int safeIndex = -1;
- if (baseExpression->isArray())
+ if (index < 0)
{
- if (baseExpression->getQualifier() == EvqFragData && index > 0)
+ outOfRangeError(outOfRangeIndexIsError, location, "index expression is negative", "[]");
+ safeIndex = 0;
+ }
+
+ if (!baseExpression->getType().isUnsizedArray())
+ {
+ if (baseExpression->isArray())
{
- if (!isExtensionEnabled(TExtension::EXT_draw_buffers))
+ if (baseExpression->getQualifier() == EvqFragData && index > 0)
{
- outOfRangeError(outOfRangeIndexIsError, location,
- "array index for gl_FragData must be zero when "
- "GL_EXT_draw_buffers is disabled",
- "[");
- safeIndex = 0;
+ if (!isExtensionEnabled(TExtension::EXT_draw_buffers))
+ {
+ outOfRangeError(outOfRangeIndexIsError, location,
+ "array index for gl_FragData must be zero when "
+ "GL_EXT_draw_buffers is disabled",
+ "[]");
+ safeIndex = 0;
+ }
+ }
+ // Only do generic out-of-range check if similar error hasn't already been reported.
+ if (safeIndex < 0)
+ {
+ safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index,
+ baseExpression->getOutermostArraySize(),
+ "array index out of range");
}
}
- // Only do generic out-of-range check if similar error hasn't already been reported.
- if (safeIndex < 0)
+ else if (baseExpression->isMatrix())
{
- safeIndex = checkIndexOutOfRange(outOfRangeIndexIsError, location, index,
- baseExpression->getOutermostArraySize(),
- "array index out of range");
+ safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index,
+ baseExpression->getType().getCols(),
+ "matrix field selection out of range");
}
- }
- else if (baseExpression->isMatrix())
- {
- safeIndex = checkIndexOutOfRange(outOfRangeIndexIsError, location, index,
- baseExpression->getType().getCols(),
- "matrix field selection out of range");
- }
- else if (baseExpression->isVector())
- {
- safeIndex = checkIndexOutOfRange(outOfRangeIndexIsError, location, index,
- baseExpression->getType().getNominalSize(),
- "vector field selection out of range");
- }
+ else if (baseExpression->isVector())
+ {
+ safeIndex = checkIndexLessThan(outOfRangeIndexIsError, location, index,
+ baseExpression->getType().getNominalSize(),
+ "vector field selection out of range");
+ }
- ASSERT(safeIndex >= 0);
- // Data of constant unions can't be changed, because it may be shared with other
- // constant unions or even builtins, like gl_MaxDrawBuffers. Instead use a new
- // sanitized object.
- if (safeIndex != index || indexConstantUnion->getBasicType() != EbtInt)
- {
- TConstantUnion *safeConstantUnion = new TConstantUnion();
- safeConstantUnion->setIConst(safeIndex);
- indexConstantUnion->replaceConstantUnion(safeConstantUnion);
- indexConstantUnion->getTypePointer()->setBasicType(EbtInt);
- }
+ ASSERT(safeIndex >= 0);
+ // Data of constant unions can't be changed, because it may be shared with other
+ // constant unions or even builtins, like gl_MaxDrawBuffers. Instead use a new
+ // sanitized object.
+ if (safeIndex != index || indexConstantUnion->getBasicType() != EbtInt)
+ {
+ TConstantUnion *safeConstantUnion = new TConstantUnion();
+ safeConstantUnion->setIConst(safeIndex);
+ indexConstantUnion->replaceConstantUnion(safeConstantUnion);
+ indexConstantUnion->getTypePointer()->setBasicType(EbtInt);
+ }
- TIntermBinary *node = new TIntermBinary(EOpIndexDirect, baseExpression, indexExpression);
- node->setLine(location);
- return node->fold(mDiagnostics);
+ TIntermBinary *node =
+ new TIntermBinary(EOpIndexDirect, baseExpression, indexExpression);
+ node->setLine(location);
+ return node->fold(mDiagnostics);
+ }
}
- else
- {
- TIntermBinary *node = new TIntermBinary(EOpIndexIndirect, baseExpression, indexExpression);
- node->setLine(location);
- // Indirect indexing can never be constant folded.
- return node;
- }
+
+ TIntermBinary *node = new TIntermBinary(EOpIndexIndirect, baseExpression, indexExpression);
+ node->setLine(location);
+ // Indirect indexing can never be constant folded.
+ return node;
}
-int TParseContext::checkIndexOutOfRange(bool outOfRangeIndexIsError,
- const TSourceLoc &location,
- int index,
- int arraySize,
- const char *reason)
+int TParseContext::checkIndexLessThan(bool outOfRangeIndexIsError,
+ const TSourceLoc &location,
+ int index,
+ int arraySize,
+ const char *reason)
{
- if (index >= arraySize || index < 0)
+ // Should not reach here with an unsized / runtime-sized array.
+ ASSERT(arraySize > 0);
+ if (index >= arraySize)
{
std::stringstream reasonStream;
reasonStream << reason << " '" << index << "'";
std::string token = reasonStream.str();
outOfRangeError(outOfRangeIndexIsError, location, reason, "[]");
- if (index < 0)
- {
- return 0;
- }
- else
- {
- return arraySize - 1;
- }
+ return arraySize - 1;
}
return index;
}
@@ -4759,9 +4771,6 @@
{
type->makeArray(arraySize);
}
- checkIsNotUnsizedArray(typeSpecifier.getLine(),
- "array members of structs must specify a size",
- declarator->name().c_str(), type);
checkIsBelowStructNestingLimit(typeSpecifier.getLine(), *declarator);
}
@@ -4792,7 +4801,7 @@
// ensure we do not specify any storage qualifiers on the struct members
for (unsigned int typeListIndex = 0; typeListIndex < fieldList->size(); typeListIndex++)
{
- const TField &field = *(*fieldList)[typeListIndex];
+ TField &field = *(*fieldList)[typeListIndex];
const TQualifier qualifier = field.type()->getQualifier();
switch (qualifier)
{
@@ -4814,6 +4823,9 @@
error(field.line(), "disallowed type in struct", field.type()->getBasicString());
}
+ checkIsNotUnsizedArray(field.line(), "array members of structs must specify a size",
+ field.name().c_str(), field.type());
+
checkMemoryQualifierIsNotSpecified(field.type()->getMemoryQualifier(), field.line());
checkBindingIsNotSpecified(field.line(), field.type()->getLayoutQualifier().binding);
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index 6cf5fad..8bfdbd5 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -462,11 +462,11 @@
constexpr static size_t kAtomicCounterArrayStride = 4;
// Returns a clamped index. If it prints out an error message, the token is "[]".
- int checkIndexOutOfRange(bool outOfRangeIndexIsError,
- const TSourceLoc &location,
- int index,
- int arraySize,
- const char *reason);
+ int checkIndexLessThan(bool outOfRangeIndexIsError,
+ const TSourceLoc &location,
+ int index,
+ int arraySize,
+ const char *reason);
bool declareVariable(const TSourceLoc &line,
const TString &identifier,
diff --git a/src/compiler/translator/RemoveArrayLengthMethod.cpp b/src/compiler/translator/RemoveArrayLengthMethod.cpp
index f6f029c..e9e6a17 100644
--- a/src/compiler/translator/RemoveArrayLengthMethod.cpp
+++ b/src/compiler/translator/RemoveArrayLengthMethod.cpp
@@ -16,6 +16,8 @@
//
// Must be run after SplitSequenceOperator, SimplifyLoopConditions and SeparateDeclarations steps
// have been done to expressions containing calls of the array length method.
+//
+// Does nothing to length method calls done on runtime-sized arrays.
#include "compiler/translator/RemoveArrayLengthMethod.h"
@@ -45,7 +47,8 @@
bool RemoveArrayLengthTraverser::visitUnary(Visit visit, TIntermUnary *node)
{
- if (node->getOp() == EOpArrayLength)
+ // The only case where we leave array length() in place is for runtime-sized arrays.
+ if (node->getOp() == EOpArrayLength && !node->getOperand()->getType().isUnsizedArray())
{
mFoundArrayLength = true;
if (!node->getOperand()->hasSideEffects())
diff --git a/src/compiler/translator/RemoveArrayLengthMethod.h b/src/compiler/translator/RemoveArrayLengthMethod.h
index f4cbb14..3b2c6df 100644
--- a/src/compiler/translator/RemoveArrayLengthMethod.h
+++ b/src/compiler/translator/RemoveArrayLengthMethod.h
@@ -16,6 +16,8 @@
//
// Must be run after SplitSequenceOperator, SimplifyLoopConditions and SeparateDeclarations steps
// have been done to expressions containing calls of the array length method.
+//
+// Does nothing to length method calls done on runtime-sized arrays.
namespace sh
{
diff --git a/src/compiler/translator/ShaderLang.cpp b/src/compiler/translator/ShaderLang.cpp
index fabce78..eeb13f2 100644
--- a/src/compiler/translator/ShaderLang.cpp
+++ b/src/compiler/translator/ShaderLang.cpp
@@ -272,6 +272,7 @@
resources->MaxAtomicCounterBufferSize = 32;
resources->MaxUniformBufferBindings = 32;
+ resources->MaxShaderStorageBufferBindings = 4;
resources->MaxGeometryUniformComponents = 1024;
resources->MaxGeometryUniformBlocks = 12;