translator: separate declarations after rewriting loops
Otherwise when trying to add the declarations back, things might fail
because the loop initialization is a sequence and not a block.
BUG=668028
Change-Id: I8d84a25c25765e9655c16ce56604ae08f0f8176c
Reviewed-on: https://chromium-review.googlesource.com/414305
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/IntermNodePatternMatcher.cpp b/src/compiler/translator/IntermNodePatternMatcher.cpp
index dd2054f..6996c45 100644
--- a/src/compiler/translator/IntermNodePatternMatcher.cpp
+++ b/src/compiler/translator/IntermNodePatternMatcher.cpp
@@ -106,4 +106,13 @@
return false;
}
+bool IntermNodePatternMatcher::match(TIntermDeclaration *node)
+{
+ if ((mMask & kMultiDeclaration) != 0)
+ {
+ return node->getSequence()->size() > 1;
+ }
+ return false;
+}
+
} // namespace sh
diff --git a/src/compiler/translator/IntermNodePatternMatcher.h b/src/compiler/translator/IntermNodePatternMatcher.h
index 10f9657..c156100 100644
--- a/src/compiler/translator/IntermNodePatternMatcher.h
+++ b/src/compiler/translator/IntermNodePatternMatcher.h
@@ -18,6 +18,7 @@
class TIntermBinary;
class TIntermNode;
class TIntermTernary;
+class TIntermDeclaration;
class IntermNodePatternMatcher
{
@@ -34,7 +35,10 @@
kExpressionReturningArray = 0x0002,
// Matches dynamic indexing of vectors or matrices in l-values.
- kDynamicIndexingOfVectorOrMatrixInLValue = 0x0004
+ kDynamicIndexingOfVectorOrMatrixInLValue = 0x0004,
+
+ // Matches declarations with more than one declared variables
+ kMultiDeclaration = 0x0008,
};
IntermNodePatternMatcher(const unsigned int mask);
@@ -46,6 +50,7 @@
bool match(TIntermAggregate *node, TIntermNode *parentNode);
bool match(TIntermTernary *node);
+ bool match(TIntermDeclaration *node);
private:
const unsigned int mMask;
diff --git a/src/compiler/translator/SimplifyLoopConditions.cpp b/src/compiler/translator/SimplifyLoopConditions.cpp
index 85c166e..1f8a384 100644
--- a/src/compiler/translator/SimplifyLoopConditions.cpp
+++ b/src/compiler/translator/SimplifyLoopConditions.cpp
@@ -40,6 +40,7 @@
bool visitBinary(Visit visit, TIntermBinary *node) override;
bool visitAggregate(Visit visit, TIntermAggregate *node) override;
bool visitTernary(Visit visit, TIntermTernary *node) override;
+ bool visitDeclaration(Visit visit, TIntermDeclaration *node) override;
void nextIteration();
bool foundLoopToChange() const { return mFoundLoopToChange; }
@@ -48,7 +49,7 @@
// Marked to true once an operation that needs to be hoisted out of the expression has been
// found. After that, no more AST updates are performed on that traversal.
bool mFoundLoopToChange;
- bool mInsideLoopConditionOrExpression;
+ bool mInsideLoopInitConditionOrExpression;
IntermNodePatternMatcher mConditionsToSimplify;
};
@@ -58,7 +59,7 @@
int shaderVersion)
: TLValueTrackingTraverser(true, false, false, symbolTable, shaderVersion),
mFoundLoopToChange(false),
- mInsideLoopConditionOrExpression(false),
+ mInsideLoopInitConditionOrExpression(false),
mConditionsToSimplify(conditionsToSimplifyMask)
{
}
@@ -66,24 +67,25 @@
void SimplifyLoopConditionsTraverser::nextIteration()
{
mFoundLoopToChange = false;
- mInsideLoopConditionOrExpression = false;
+ mInsideLoopInitConditionOrExpression = false;
nextTemporaryIndex();
}
+// The visit functions operate in three modes:
+// 1. If a matching expression has already been found, we return early since only one loop can
+// be transformed on one traversal.
+// 2. We try to find loops. In case a node is not inside a loop and can not contain loops, we
+// stop traversing the subtree.
+// 3. If we're inside a loop initialization, condition or expression, we check for expressions
+// that should be moved out of the loop condition or expression. If one is found, the loop
+// is processed.
bool SimplifyLoopConditionsTraverser::visitBinary(Visit visit, TIntermBinary *node)
{
- // The visit functions operate in three modes:
- // 1. If a matching expression has already been found, we return early since only one loop can
- // be transformed on one traversal.
- // 2. We try to find loops. In case a node is not inside a loop and can not contain loops, we
- // stop traversing the subtree.
- // 3. If we're inside a loop condition or expression, we check for expressions that should be
- // moved out of the loop condition or expression. If one is found, the loop is processed.
if (mFoundLoopToChange)
return false;
- if (!mInsideLoopConditionOrExpression)
+ if (!mInsideLoopInitConditionOrExpression)
return false;
mFoundLoopToChange = mConditionsToSimplify.match(node, getParentNode(), isLValueRequiredHere());
@@ -95,8 +97,7 @@
if (mFoundLoopToChange)
return false;
- // If we're outside a loop condition, we only need to traverse nodes that may contain loops.
- if (!mInsideLoopConditionOrExpression)
+ if (!mInsideLoopInitConditionOrExpression)
return false;
mFoundLoopToChange = mConditionsToSimplify.match(node, getParentNode());
@@ -108,8 +109,19 @@
if (mFoundLoopToChange)
return false;
- // Don't traverse ternary operators outside loop conditions.
- if (!mInsideLoopConditionOrExpression)
+ if (!mInsideLoopInitConditionOrExpression)
+ return false;
+
+ mFoundLoopToChange = mConditionsToSimplify.match(node);
+ return !mFoundLoopToChange;
+}
+
+bool SimplifyLoopConditionsTraverser::visitDeclaration(Visit visit, TIntermDeclaration *node)
+{
+ if (mFoundLoopToChange)
+ return false;
+
+ if (!mInsideLoopInitConditionOrExpression)
return false;
mFoundLoopToChange = mConditionsToSimplify.match(node);
@@ -127,136 +139,122 @@
// Note: No need to traverse the loop init node.
- mInsideLoopConditionOrExpression = true;
- TLoopType loopType = node->getType();
+ mInsideLoopInitConditionOrExpression = true;
+ TLoopType loopType = node->getType();
- if (node->getCondition())
+ if (!mFoundLoopToChange && node->getInit())
+ {
+ node->getInit()->traverse(this);
+ }
+
+ if (!mFoundLoopToChange && node->getCondition())
{
node->getCondition()->traverse(this);
-
- if (mFoundLoopToChange)
- {
- // Replace the loop condition with a boolean variable that's updated on each iteration.
- if (loopType == ELoopWhile)
- {
- // Transform:
- // while (expr) { body; }
- // into
- // bool s0 = expr;
- // while (s0) { { body; } s0 = expr; }
- TIntermSequence tempInitSeq;
- tempInitSeq.push_back(createTempInitDeclaration(node->getCondition()->deepCopy()));
- insertStatementsInParentBlock(tempInitSeq);
-
- TIntermBlock *newBody = new TIntermBlock();
- if (node->getBody())
- {
- newBody->getSequence()->push_back(node->getBody());
- }
- newBody->getSequence()->push_back(
- createTempAssignment(node->getCondition()->deepCopy()));
-
- // Can't use queueReplacement to replace old body, since it may have been nullptr.
- // It's safe to do the replacements in place here - this node won't be traversed
- // further.
- node->setBody(newBody);
- node->setCondition(createTempSymbol(node->getCondition()->getType()));
- }
- else if (loopType == ELoopDoWhile)
- {
- // Transform:
- // do {
- // body;
- // } while (expr);
- // into
- // bool s0 = true;
- // do {
- // { body; }
- // s0 = expr;
- // while (s0);
- TIntermSequence tempInitSeq;
- tempInitSeq.push_back(createTempInitDeclaration(CreateBoolConstantNode(true)));
- insertStatementsInParentBlock(tempInitSeq);
-
- TIntermBlock *newBody = new TIntermBlock();
- if (node->getBody())
- {
- newBody->getSequence()->push_back(node->getBody());
- }
- newBody->getSequence()->push_back(
- createTempAssignment(node->getCondition()->deepCopy()));
-
- // Can't use queueReplacement to replace old body, since it may have been nullptr.
- // It's safe to do the replacements in place here - this node won't be traversed
- // further.
- node->setBody(newBody);
- node->setCondition(createTempSymbol(node->getCondition()->getType()));
- }
- else if (loopType == ELoopFor)
- {
- // Move the loop condition inside the loop.
- // Transform:
- // for (init; expr; exprB) { body; }
- // into
- // {
- // init;
- // bool s0 = expr;
- // while (s0) { { body; } exprB; s0 = expr; }
- // }
- TIntermBlock *loopScope = new TIntermBlock();
- if (node->getInit())
- {
- loopScope->getSequence()->push_back(node->getInit());
- }
- loopScope->getSequence()->push_back(
- createTempInitDeclaration(node->getCondition()->deepCopy()));
-
- TIntermBlock *whileLoopBody = new TIntermBlock();
- if (node->getBody())
- {
- whileLoopBody->getSequence()->push_back(node->getBody());
- }
- if (node->getExpression())
- {
- whileLoopBody->getSequence()->push_back(node->getExpression());
- }
- whileLoopBody->getSequence()->push_back(
- createTempAssignment(node->getCondition()->deepCopy()));
- TIntermLoop *whileLoop = new TIntermLoop(
- ELoopWhile, nullptr, createTempSymbol(node->getCondition()->getType()), nullptr,
- whileLoopBody);
- loopScope->getSequence()->push_back(whileLoop);
- queueReplacementWithParent(getAncestorNode(1), node, loopScope,
- OriginalNode::IS_DROPPED);
- }
- }
}
if (!mFoundLoopToChange && node->getExpression())
{
node->getExpression()->traverse(this);
+ }
- if (mFoundLoopToChange)
+ if (mFoundLoopToChange)
+ {
+ // Replace the loop condition with a boolean variable that's updated on each iteration.
+ if (loopType == ELoopWhile)
{
- ASSERT(loopType == ELoopFor);
- // Move the loop expression to inside the loop.
+ // Transform:
+ // while (expr) { body; }
+ // into
+ // bool s0 = expr;
+ // while (s0) { { body; } s0 = expr; }
+ TIntermSequence tempInitSeq;
+ tempInitSeq.push_back(createTempInitDeclaration(node->getCondition()->deepCopy()));
+ insertStatementsInParentBlock(tempInitSeq);
+
+ TIntermBlock *newBody = new TIntermBlock();
+ if (node->getBody())
+ {
+ newBody->getSequence()->push_back(node->getBody());
+ }
+ newBody->getSequence()->push_back(
+ createTempAssignment(node->getCondition()->deepCopy()));
+
+ // Can't use queueReplacement to replace old body, since it may have been nullptr.
+ // It's safe to do the replacements in place here - this node won't be traversed
+ // further.
+ node->setBody(newBody);
+ node->setCondition(createTempSymbol(node->getCondition()->getType()));
+ }
+ else if (loopType == ELoopDoWhile)
+ {
+ // Transform:
+ // do {
+ // body;
+ // } while (expr);
+ // into
+ // bool s0 = true;
+ // do {
+ // { body; }
+ // s0 = expr;
+ // } while (s0);
+ TIntermSequence tempInitSeq;
+ tempInitSeq.push_back(createTempInitDeclaration(CreateBoolConstantNode(true)));
+ insertStatementsInParentBlock(tempInitSeq);
+
+ TIntermBlock *newBody = new TIntermBlock();
+ if (node->getBody())
+ {
+ newBody->getSequence()->push_back(node->getBody());
+ }
+ newBody->getSequence()->push_back(
+ createTempAssignment(node->getCondition()->deepCopy()));
+
+ // Can't use queueReplacement to replace old body, since it may have been nullptr.
+ // It's safe to do the replacements in place here - this node won't be traversed
+ // further.
+ node->setBody(newBody);
+ node->setCondition(createTempSymbol(node->getCondition()->getType()));
+ }
+ else if (loopType == ELoopFor)
+ {
+ // Move the loop condition inside the loop.
// Transform:
// for (init; expr; exprB) { body; }
// into
- // for (init; expr; ) { { body; } exprB; }
- TIntermTyped *loopExpression = node->getExpression();
- node->setExpression(nullptr);
- TIntermBlock *oldBody = node->getBody();
- node->setBody(new TIntermBlock());
- if (oldBody != nullptr)
+ // {
+ // init;
+ // bool s0 = expr;
+ // while (s0) { { body; } exprB; s0 = expr; }
+ // }
+ TIntermBlock *loopScope = new TIntermBlock();
+ if (node->getInit())
{
- node->getBody()->getSequence()->push_back(oldBody);
+ loopScope->getSequence()->push_back(node->getInit());
}
- node->getBody()->getSequence()->push_back(loopExpression);
+ loopScope->getSequence()->push_back(
+ createTempInitDeclaration(node->getCondition()->deepCopy()));
+
+ TIntermBlock *whileLoopBody = new TIntermBlock();
+ if (node->getBody())
+ {
+ whileLoopBody->getSequence()->push_back(node->getBody());
+ }
+ if (node->getExpression())
+ {
+ whileLoopBody->getSequence()->push_back(node->getExpression());
+ }
+ whileLoopBody->getSequence()->push_back(
+ createTempAssignment(node->getCondition()->deepCopy()));
+ TIntermLoop *whileLoop = new TIntermLoop(
+ ELoopWhile, nullptr, createTempSymbol(node->getCondition()->getType()), nullptr,
+ whileLoopBody);
+ loopScope->getSequence()->push_back(whileLoop);
+ queueReplacementWithParent(getAncestorNode(1), node, loopScope,
+ OriginalNode::IS_DROPPED);
}
}
- mInsideLoopConditionOrExpression = false;
+ mInsideLoopInitConditionOrExpression = false;
if (!mFoundLoopToChange && node->getBody())
node->getBody()->traverse(this);
diff --git a/src/compiler/translator/TranslatorHLSL.cpp b/src/compiler/translator/TranslatorHLSL.cpp
index 7ef1d4e..59417a7 100644
--- a/src/compiler/translator/TranslatorHLSL.cpp
+++ b/src/compiler/translator/TranslatorHLSL.cpp
@@ -39,16 +39,19 @@
sh::AddDefaultReturnStatements(root);
- SeparateDeclarations(root);
-
// 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::kExpressionReturningArray |
IntermNodePatternMatcher::kUnfoldedShortCircuitExpression |
- IntermNodePatternMatcher::kDynamicIndexingOfVectorOrMatrixInLValue,
+ IntermNodePatternMatcher::kDynamicIndexingOfVectorOrMatrixInLValue |
+ IntermNodePatternMatcher::kMultiDeclaration,
getTemporaryIndex(), 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 |