Allow length() on arbitrary array expressions
This is required to pass some dEQP GLES 3.1 tests for arrays of
arrays, and WebGL conformance tests were also recently fixed to
require this behavior. The intent of the GLSL ES spec was not to
restrict usage of length().
In practice GL drivers don't implement array length() on expressions
with side effects correctly in all cases. HLSL doesn't have an array
length operator either. Because of this we always remove array length
ops from the AST before output.
BUG=angleproject:2142
TEST=angle_unittests, angle_end2end_tests, WebGL conformance tests
Change-Id: I863a92e83ac5315b013af9a5626348482bad72b3
Reviewed-on: https://chromium-review.googlesource.com/643190
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 44578c1..ed2bcff 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -27,12 +27,14 @@
#include "compiler/translator/ParseContext.h"
#include "compiler/translator/PruneEmptyDeclarations.h"
#include "compiler/translator/RegenerateStructNames.h"
+#include "compiler/translator/RemoveArrayLengthMethod.h"
#include "compiler/translator/RemoveInvariantDeclaration.h"
#include "compiler/translator/RemovePow.h"
#include "compiler/translator/RewriteDoWhile.h"
#include "compiler/translator/ScalarizeVecAndMatConstructorArgs.h"
#include "compiler/translator/SeparateDeclarations.h"
#include "compiler/translator/SimplifyLoopConditions.h"
+#include "compiler/translator/SplitSequenceOperator.h"
#include "compiler/translator/UnfoldShortCircuitAST.h"
#include "compiler/translator/UseInterfaceBlockFields.h"
#include "compiler/translator/ValidateLimitations.h"
@@ -526,26 +528,43 @@
DeferGlobalInitializers(root, needToInitializeGlobalsInAST(), &symbolTable);
}
+ // Split multi declarations and remove calls to array length().
+ if (success)
+ {
+ // Note that SimplifyLoopConditions needs to be run before any other AST transformations
+ // that may need to generate new statements from loop conditions or loop expressions.
+ SimplifyLoopConditions(root,
+ IntermNodePatternMatcher::kMultiDeclaration |
+ IntermNodePatternMatcher::kArrayLengthMethod,
+ &getSymbolTable(), getShaderVersion());
+
+ // Note that separate declarations need to be run before other AST transformations that
+ // generate new statements from expressions.
+ SeparateDeclarations(root);
+
+ SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod,
+ &getSymbolTable(), getShaderVersion());
+
+ RemoveArrayLengthMethod(root);
+ }
+
if (success && (compileOptions & SH_INITIALIZE_UNINITIALIZED_LOCALS) && getOutputType())
{
// Initialize uninitialized local variables.
// In some cases initializing can generate extra statements in the parent block, such as
// when initializing nameless structs or initializing arrays in ESSL 1.00. In that case
- // we need to first simplify loop conditions and separate declarations. If we don't
- // follow the Appendix A limitations, loop init statements can declare arrays or
- // nameless structs and have multiple declarations.
+ // we need to first simplify loop conditions. We've already separated declarations
+ // earlier, which is also required. If we don't follow the Appendix A limitations, loop
+ // init statements can declare arrays or nameless structs and have multiple
+ // declarations.
if (!shouldRunLoopAndIndexingValidation(compileOptions))
{
SimplifyLoopConditions(root,
- IntermNodePatternMatcher::kMultiDeclaration |
IntermNodePatternMatcher::kArrayDeclaration |
IntermNodePatternMatcher::kNamelessStructDeclaration,
&getSymbolTable(), getShaderVersion());
}
- // We only really need to separate array declarations and nameless struct declarations,
- // but it's simpler to just use the regular SeparateDeclarations.
- SeparateDeclarations(root);
InitializeUninitializedLocals(root, getShaderVersion());
}
diff --git a/src/compiler/translator/IntermNodePatternMatcher.cpp b/src/compiler/translator/IntermNodePatternMatcher.cpp
index ed2b9f7..567e8f7 100644
--- a/src/compiler/translator/IntermNodePatternMatcher.cpp
+++ b/src/compiler/translator/IntermNodePatternMatcher.cpp
@@ -48,6 +48,18 @@
return false;
}
+bool IntermNodePatternMatcher::match(TIntermUnary *node)
+{
+ if ((mMask & kArrayLengthMethod) != 0)
+ {
+ if (node->getOp() == EOpArrayLength)
+ {
+ return true;
+ }
+ }
+ return false;
+}
+
bool IntermNodePatternMatcher::match(TIntermBinary *node, TIntermNode *parentNode)
{
// L-value tracking information is needed to check for dynamic indexing in L-value.
diff --git a/src/compiler/translator/IntermNodePatternMatcher.h b/src/compiler/translator/IntermNodePatternMatcher.h
index e908723..997fc2e 100644
--- a/src/compiler/translator/IntermNodePatternMatcher.h
+++ b/src/compiler/translator/IntermNodePatternMatcher.h
@@ -16,9 +16,10 @@
class TIntermAggregate;
class TIntermBinary;
+class TIntermDeclaration;
class TIntermNode;
class TIntermTernary;
-class TIntermDeclaration;
+class TIntermUnary;
class IntermNodePatternMatcher
{
@@ -44,10 +45,15 @@
kArrayDeclaration = 0x0001 << 4,
// Matches declarations of structs where the struct type does not have a name.
- kNamelessStructDeclaration = 0x0001 << 5
+ kNamelessStructDeclaration = 0x0001 << 5,
+
+ // Matches array length() method.
+ kArrayLengthMethod = 0x0001 << 6
};
IntermNodePatternMatcher(const unsigned int mask);
+ bool match(TIntermUnary *node);
+
bool match(TIntermBinary *node, TIntermNode *parentNode);
// Use this version for checking binary node matches in case you're using flag
diff --git a/src/compiler/translator/OutputGLSLBase.cpp b/src/compiler/translator/OutputGLSLBase.cpp
index aaab3f3..4d3f098 100644
--- a/src/compiler/translator/OutputGLSLBase.cpp
+++ b/src/compiler/translator/OutputGLSLBase.cpp
@@ -734,6 +734,10 @@
case EOpPreDecrement:
preString = "(--";
break;
+ case EOpArrayLength:
+ preString = "((";
+ postString = ").length())";
+ break;
case EOpRadians:
case EOpDegrees:
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 51f0e70..da64488 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -5461,11 +5461,6 @@
{
error(loc, "missing input primitive declaration before calling length on gl_in", "length");
}
- else if (typedThis->hasSideEffects())
- {
- error(loc, "length method not supported on expressions with possible side effects",
- "length");
- }
else
{
TIntermUnary *node = new TIntermUnary(EOpArrayLength, typedThis);
diff --git a/src/compiler/translator/RemoveArrayLengthMethod.cpp b/src/compiler/translator/RemoveArrayLengthMethod.cpp
new file mode 100644
index 0000000..f6f029c
--- /dev/null
+++ b/src/compiler/translator/RemoveArrayLengthMethod.cpp
@@ -0,0 +1,80 @@
+//
+// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// RemoveArrayLengthMethod.cpp:
+// Fold array length expressions, including cases where the "this" node has side effects.
+// Example:
+// int i = (a = b).length();
+// int j = (func()).length();
+// becomes:
+// (a = b);
+// int i = <constant array length>;
+// func();
+// int j = <constant array length>;
+//
+// Must be run after SplitSequenceOperator, SimplifyLoopConditions and SeparateDeclarations steps
+// have been done to expressions containing calls of the array length method.
+
+#include "compiler/translator/RemoveArrayLengthMethod.h"
+
+#include "compiler/translator/IntermNode.h"
+#include "compiler/translator/IntermTraverse.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class RemoveArrayLengthTraverser : public TIntermTraverser
+{
+ public:
+ RemoveArrayLengthTraverser() : TIntermTraverser(true, false, false), mFoundArrayLength(false) {}
+
+ bool visitUnary(Visit visit, TIntermUnary *node) override;
+
+ void nextIteration() { mFoundArrayLength = false; }
+
+ bool foundArrayLength() const { return mFoundArrayLength; }
+
+ private:
+ bool mFoundArrayLength;
+};
+
+bool RemoveArrayLengthTraverser::visitUnary(Visit visit, TIntermUnary *node)
+{
+ if (node->getOp() == EOpArrayLength)
+ {
+ mFoundArrayLength = true;
+ if (!node->getOperand()->hasSideEffects())
+ {
+ queueReplacement(node->fold(nullptr), OriginalNode::IS_DROPPED);
+ return false;
+ }
+ insertStatementInParentBlock(node->getOperand()->deepCopy());
+ TConstantUnion *constArray = new TConstantUnion[1];
+ constArray->setIConst(node->getOperand()->getOutermostArraySize());
+ queueReplacement(new TIntermConstantUnion(constArray, node->getType()),
+ OriginalNode::IS_DROPPED);
+ return false;
+ }
+ return true;
+}
+
+} // anonymous namespace
+
+void RemoveArrayLengthMethod(TIntermBlock *root)
+{
+ RemoveArrayLengthTraverser traverser;
+ do
+ {
+ traverser.nextIteration();
+ root->traverse(&traverser);
+ if (traverser.foundArrayLength())
+ traverser.updateTree();
+ } while (traverser.foundArrayLength());
+}
+
+} // namespace sh
diff --git a/src/compiler/translator/RemoveArrayLengthMethod.h b/src/compiler/translator/RemoveArrayLengthMethod.h
new file mode 100644
index 0000000..f4cbb14
--- /dev/null
+++ b/src/compiler/translator/RemoveArrayLengthMethod.h
@@ -0,0 +1,27 @@
+//
+// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// RemoveArrayLengthMethod.h:
+// Fold array length expressions, including cases where the "this" node has side effects.
+// Example:
+// int i = (a = b).length();
+// int j = (func()).length();
+// becomes:
+// (a = b);
+// int i = <constant array length>;
+// func();
+// int j = <constant array length>;
+//
+// Must be run after SplitSequenceOperator, SimplifyLoopConditions and SeparateDeclarations steps
+// have been done to expressions containing calls of the array length method.
+
+namespace sh
+{
+
+class TIntermBlock;
+
+void RemoveArrayLengthMethod(TIntermBlock *root);
+
+} // namespace sh
diff --git a/src/compiler/translator/SimplifyLoopConditions.cpp b/src/compiler/translator/SimplifyLoopConditions.cpp
index 169ad9b..9704046 100644
--- a/src/compiler/translator/SimplifyLoopConditions.cpp
+++ b/src/compiler/translator/SimplifyLoopConditions.cpp
@@ -29,6 +29,7 @@
void traverseLoop(TIntermLoop *node) override;
+ bool visitUnary(Visit visit, TIntermUnary *node) override;
bool visitBinary(Visit visit, TIntermBinary *node) override;
bool visitAggregate(Visit visit, TIntermAggregate *node) override;
bool visitTernary(Visit visit, TIntermTernary *node) override;
@@ -61,6 +62,18 @@
// If we're not inside loop initialization, condition, or expression, we only need to traverse nodes
// that may contain loops.
+bool SimplifyLoopConditionsTraverser::visitUnary(Visit visit, TIntermUnary *node)
+{
+ if (!mInsideLoopInitConditionOrExpression)
+ return false;
+
+ if (mFoundLoopToChange)
+ return false; // Already decided to change this loop.
+
+ mFoundLoopToChange = mConditionsToSimplify.match(node);
+ return !mFoundLoopToChange;
+}
+
bool SimplifyLoopConditionsTraverser::visitBinary(Visit visit, TIntermBinary *node)
{
if (!mInsideLoopInitConditionOrExpression)
diff --git a/src/compiler/translator/SplitSequenceOperator.cpp b/src/compiler/translator/SplitSequenceOperator.cpp
index 5354709..5df3154 100644
--- a/src/compiler/translator/SplitSequenceOperator.cpp
+++ b/src/compiler/translator/SplitSequenceOperator.cpp
@@ -27,6 +27,7 @@
TSymbolTable *symbolTable,
int shaderVersion);
+ bool visitUnary(Visit visit, TIntermUnary *node) override;
bool visitBinary(Visit visit, TIntermBinary *node) override;
bool visitAggregate(Visit visit, TIntermAggregate *node) override;
bool visitTernary(Visit visit, TIntermTernary *node) override;
@@ -75,6 +76,21 @@
return true;
}
+bool SplitSequenceOperatorTraverser::visitUnary(Visit visit, TIntermUnary *node)
+{
+ if (mFoundExpressionToSplit)
+ return false;
+
+ if (mInsideSequenceOperator > 0 && visit == PreVisit)
+ {
+ // Detect expressions that need to be simplified
+ mFoundExpressionToSplit = mPatternToSplitMatcher.match(node);
+ return !mFoundExpressionToSplit;
+ }
+
+ return true;
+}
+
bool SplitSequenceOperatorTraverser::visitBinary(Visit visit, TIntermBinary *node)
{
if (node->getOp() == EOpComma)
diff --git a/src/compiler/translator/TranslatorHLSL.cpp b/src/compiler/translator/TranslatorHLSL.cpp
index 005edf5..2ef1b83 100644
--- a/src/compiler/translator/TranslatorHLSL.cpp
+++ b/src/compiler/translator/TranslatorHLSL.cpp
@@ -41,17 +41,13 @@
// Note that SimplifyLoopConditions needs to be run before any other AST transformations that
// may need to generate new statements from loop conditions or loop expressions.
+ // Note that SeparateDeclarations has already been run in TCompiler::compileTreeImpl().
SimplifyLoopConditions(root,
IntermNodePatternMatcher::kExpressionReturningArray |
IntermNodePatternMatcher::kUnfoldedShortCircuitExpression |
- IntermNodePatternMatcher::kDynamicIndexingOfVectorOrMatrixInLValue |
- IntermNodePatternMatcher::kMultiDeclaration,
+ IntermNodePatternMatcher::kDynamicIndexingOfVectorOrMatrixInLValue,
&getSymbolTable(), getShaderVersion());
- // Note that separate declarations need to be run before other AST transformations that
- // generate new statements from expressions.
- SeparateDeclarations(root);
-
SplitSequenceOperator(root,
IntermNodePatternMatcher::kExpressionReturningArray |
IntermNodePatternMatcher::kUnfoldedShortCircuitExpression |