Cover vector dynamic indexing case in SplitSequenceOperator
Vectors or matrices that are dynamically indexed as a part of an
l-value generate new statements in the RemoveDynamicIndexing AST
transformation step. SplitSequenceOperator needs to detect this case
and split the sequence operator before statements are generated from
its operands to ensure the correct order of execution.
BUG=angleproject:1341
TEST=angle_end2end_tests
Change-Id: I84e41a59c88fb5d0111669cab60312b930531a22
Reviewed-on: https://chromium-review.googlesource.com/361695
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/IntermNodePatternMatcher.cpp b/src/compiler/translator/IntermNodePatternMatcher.cpp
index a0cddac..b614d83 100644
--- a/src/compiler/translator/IntermNodePatternMatcher.cpp
+++ b/src/compiler/translator/IntermNodePatternMatcher.cpp
@@ -27,7 +27,14 @@
{
}
-bool IntermNodePatternMatcher::match(TIntermBinary *node, TIntermNode *parentNode)
+// static
+bool IntermNodePatternMatcher::IsDynamicIndexingOfVectorOrMatrix(TIntermBinary *node)
+{
+ return node->getOp() == EOpIndexIndirect && !node->getLeft()->isArray() &&
+ node->getLeft()->getBasicType() != EbtStruct;
+}
+
+bool IntermNodePatternMatcher::matchInternal(TIntermBinary *node, TIntermNode *parentNode)
{
if ((mMask & kExpressionReturningArray) != 0)
{
@@ -49,6 +56,33 @@
return false;
}
+bool IntermNodePatternMatcher::match(TIntermBinary *node, TIntermNode *parentNode)
+{
+ // L-value tracking information is needed to check for dynamic indexing in L-value.
+ // Traversers that don't track l-values can still use this class and match binary nodes with
+ // this variation of this method if they don't need to check for dynamic indexing in l-values.
+ ASSERT((mMask & kDynamicIndexingOfVectorOrMatrixInLValue) == 0);
+ return matchInternal(node, parentNode);
+}
+
+bool IntermNodePatternMatcher::match(TIntermBinary *node,
+ TIntermNode *parentNode,
+ bool isLValueRequiredHere)
+{
+ if (matchInternal(node, parentNode))
+ {
+ return true;
+ }
+ if ((mMask & kDynamicIndexingOfVectorOrMatrixInLValue) != 0)
+ {
+ if (isLValueRequiredHere && IsDynamicIndexingOfVectorOrMatrix(node))
+ {
+ return true;
+ }
+ }
+ return false;
+}
+
bool IntermNodePatternMatcher::match(TIntermAggregate *node, TIntermNode *parentNode)
{
if ((mMask & kExpressionReturningArray) != 0)
diff --git a/src/compiler/translator/IntermNodePatternMatcher.h b/src/compiler/translator/IntermNodePatternMatcher.h
index b088633..be6fc61 100644
--- a/src/compiler/translator/IntermNodePatternMatcher.h
+++ b/src/compiler/translator/IntermNodePatternMatcher.h
@@ -19,22 +19,35 @@
class IntermNodePatternMatcher
{
public:
+ static bool IsDynamicIndexingOfVectorOrMatrix(TIntermBinary *node);
+
enum PatternType
{
+ // Matches expressions that are unfolded to if statements by UnfoldShortCircuitToIf
kUnfoldedShortCircuitExpression = 0x0001,
// Matches expressions that return arrays with the exception of simple statements where a
// constructor or function call result is assigned.
- kExpressionReturningArray = 0x0002
+ kExpressionReturningArray = 0x0002,
+
+ // Matches dynamic indexing of vectors or matrices in l-values.
+ kDynamicIndexingOfVectorOrMatrixInLValue = 0x0004
};
IntermNodePatternMatcher(const unsigned int mask);
bool match(TIntermBinary *node, TIntermNode *parentNode);
+
+ // Use this version for checking binary node matches in case you're using flag
+ // kDynamicIndexingOfVectorOrMatrixInLValue.
+ bool match(TIntermBinary *node, TIntermNode *parentNode, bool isLValueRequiredHere);
+
bool match(TIntermAggregate *node, TIntermNode *parentNode);
bool match(TIntermSelection *node);
private:
const unsigned int mMask;
+
+ bool matchInternal(TIntermBinary *node, TIntermNode *parentNode);
};
#endif
diff --git a/src/compiler/translator/RemoveDynamicIndexing.cpp b/src/compiler/translator/RemoveDynamicIndexing.cpp
index 74814f2..35f6c10 100644
--- a/src/compiler/translator/RemoveDynamicIndexing.cpp
+++ b/src/compiler/translator/RemoveDynamicIndexing.cpp
@@ -11,6 +11,7 @@
#include "compiler/translator/InfoSink.h"
#include "compiler/translator/IntermNode.h"
+#include "compiler/translator/IntermNodePatternMatcher.h"
#include "compiler/translator/SymbolTable.h"
namespace
@@ -408,10 +409,18 @@
NodeUpdateEntry replaceIndex(node, node->getRight(), tempIndex, false);
mReplacements.push_back(replaceIndex);
}
- else if (!node->getLeft()->isArray() && node->getLeft()->getBasicType() != EbtStruct)
+ else if (IntermNodePatternMatcher::IsDynamicIndexingOfVectorOrMatrix(node))
{
bool write = isLValueRequiredHere();
+#if defined(ANGLE_ENABLE_ASSERTS)
+ // Make sure that IntermNodePatternMatcher is consistent with the slightly differently
+ // implemented checks in this traverser.
+ IntermNodePatternMatcher matcher(
+ IntermNodePatternMatcher::kDynamicIndexingOfVectorOrMatrixInLValue);
+ ASSERT(matcher.match(node, getParentNode(), isLValueRequiredHere()) == write);
+#endif
+
TType type = node->getLeft()->getType();
mIndexedVecAndMatrixTypes.insert(type);
diff --git a/src/compiler/translator/SplitSequenceOperator.cpp b/src/compiler/translator/SplitSequenceOperator.cpp
index 3423355..22f9ec6 100644
--- a/src/compiler/translator/SplitSequenceOperator.cpp
+++ b/src/compiler/translator/SplitSequenceOperator.cpp
@@ -17,10 +17,12 @@
namespace
{
-class SplitSequenceOperatorTraverser : public TIntermTraverser
+class SplitSequenceOperatorTraverser : public TLValueTrackingTraverser
{
public:
- SplitSequenceOperatorTraverser(unsigned int patternsToSplitMask);
+ SplitSequenceOperatorTraverser(unsigned int patternsToSplitMask,
+ const TSymbolTable &symbolTable,
+ int shaderVersion);
bool visitBinary(Visit visit, TIntermBinary *node) override;
bool visitAggregate(Visit visit, TIntermAggregate *node) override;
@@ -38,8 +40,10 @@
IntermNodePatternMatcher mPatternToSplitMatcher;
};
-SplitSequenceOperatorTraverser::SplitSequenceOperatorTraverser(unsigned int patternsToSplitMask)
- : TIntermTraverser(true, false, true),
+SplitSequenceOperatorTraverser::SplitSequenceOperatorTraverser(unsigned int patternsToSplitMask,
+ const TSymbolTable &symbolTable,
+ int shaderVersion)
+ : TLValueTrackingTraverser(true, false, true, symbolTable, shaderVersion),
mFoundExpressionToSplit(false),
mInsideSequenceOperator(0),
mPatternToSplitMatcher(patternsToSplitMask)
@@ -61,7 +65,8 @@
if (mInsideSequenceOperator > 0 && visit == PreVisit)
{
// Detect expressions that need to be simplified
- mFoundExpressionToSplit = mPatternToSplitMatcher.match(node, getParentNode());
+ mFoundExpressionToSplit =
+ mPatternToSplitMatcher.match(node, getParentNode(), isLValueRequiredHere());
return !mFoundExpressionToSplit;
}
@@ -135,9 +140,13 @@
} // namespace
-void SplitSequenceOperator(TIntermNode *root, int patternsToSplitMask, unsigned int *temporaryIndex)
+void SplitSequenceOperator(TIntermNode *root,
+ int patternsToSplitMask,
+ unsigned int *temporaryIndex,
+ const TSymbolTable &symbolTable,
+ int shaderVersion)
{
- SplitSequenceOperatorTraverser traverser(patternsToSplitMask);
+ SplitSequenceOperatorTraverser traverser(patternsToSplitMask, symbolTable, shaderVersion);
ASSERT(temporaryIndex != nullptr);
traverser.useTemporaryIndex(temporaryIndex);
// Separate one expression at a time, and reset the traverser between iterations.
diff --git a/src/compiler/translator/SplitSequenceOperator.h b/src/compiler/translator/SplitSequenceOperator.h
index 30e13e2..4a46fe3 100644
--- a/src/compiler/translator/SplitSequenceOperator.h
+++ b/src/compiler/translator/SplitSequenceOperator.h
@@ -13,9 +13,12 @@
#define COMPILER_TRANSLATOR_SPLITSEQUENCEOPERATOR_H_
class TIntermNode;
+class TSymbolTable;
void SplitSequenceOperator(TIntermNode *root,
int patternsToSplitMask,
- unsigned int *temporaryIndex);
+ unsigned int *temporaryIndex,
+ const TSymbolTable &symbolTable,
+ int shaderVersion);
#endif // COMPILER_TRANSLATOR_SPLITSEQUENCEOPERATOR_H_
diff --git a/src/compiler/translator/TranslatorHLSL.cpp b/src/compiler/translator/TranslatorHLSL.cpp
index 4e2ebbb..d120ac5 100644
--- a/src/compiler/translator/TranslatorHLSL.cpp
+++ b/src/compiler/translator/TranslatorHLSL.cpp
@@ -33,9 +33,11 @@
// TODO (oetuaho): Sequence operators should also be split in case there is dynamic indexing of
// a vector or matrix as an l-value inside (RemoveDynamicIndexing transformation step generates
// statements in this case).
- SplitSequenceOperator(root, IntermNodePatternMatcher::kExpressionReturningArray |
- IntermNodePatternMatcher::kUnfoldedShortCircuitExpression,
- getTemporaryIndex());
+ SplitSequenceOperator(root,
+ IntermNodePatternMatcher::kExpressionReturningArray |
+ IntermNodePatternMatcher::kUnfoldedShortCircuitExpression |
+ IntermNodePatternMatcher::kDynamicIndexingOfVectorOrMatrixInLValue,
+ getTemporaryIndex(), getSymbolTable(), getShaderVersion());
// Note that SeparateDeclarations needs to be run before UnfoldShortCircuitToIf.
UnfoldShortCircuitToIf(root, getTemporaryIndex());