Track where l-values are required in AST traversal
This functionality is refactored out of EmulatePrecision to be a common
feature of TIntermTraverser. This is done since tracking where l-values
are required will be useful for other traversers. For example, it will
be needed for converting dynamic indexing of matrices and vectors to
function calls. This change adds some overhead to all tree traversers,
but the overhead is expected to be small for typical shaders which don't
contain too many user-defined functions.
BUG=angleproject:1116
TEST=angle_unittests
Change-Id: I54d34c2b5093ef028f2b24d854c11c0195dc1dbb
Reviewed-on: https://chromium-review.googlesource.com/290514
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
diff --git a/src/compiler/translator/EmulatePrecision.cpp b/src/compiler/translator/EmulatePrecision.cpp
index c4659d5..befb319 100644
--- a/src/compiler/translator/EmulatePrecision.cpp
+++ b/src/compiler/translator/EmulatePrecision.cpp
@@ -292,16 +292,12 @@
} // namespace anonymous
EmulatePrecision::EmulatePrecision()
- : TIntermTraverser(true, true, true),
- mDeclaringVariables(false),
- mInLValue(false),
- mInFunctionCallOutParameter(false)
+ : TIntermTraverser(true, true, true), mDeclaringVariables(false)
{}
void EmulatePrecision::visitSymbol(TIntermSymbol *node)
{
- if (canRoundFloat(node->getType()) &&
- !mDeclaringVariables && !mInLValue && !mInFunctionCallOutParameter)
+ if (canRoundFloat(node->getType()) && !mDeclaringVariables && !isLValueRequiredHere())
{
TIntermNode *parent = getParentNode();
TIntermNode *replacement = createRoundingFunctionCallNode(node);
@@ -314,14 +310,6 @@
{
bool visitChildren = true;
- if (node->isAssignment())
- {
- if (visit == PreVisit)
- mInLValue = true;
- else if (visit == InVisit)
- mInLValue = false;
- }
-
TOperator op = node->getOp();
// RHS of initialize is not being declared.
@@ -415,22 +403,9 @@
{
case EOpSequence:
case EOpConstructStruct:
- // No special handling
- break;
case EOpFunction:
- if (visit == PreVisit)
- {
- const TIntermSequence &sequence = *(node->getSequence());
- TIntermSequence::const_iterator seqIter = sequence.begin();
- TIntermAggregate *params = (*seqIter)->getAsAggregate();
- ASSERT(params != NULL);
- ASSERT(params->getOp() == EOpParameters);
- mFunctionMap[node->getName()] = params->getSequence();
- }
break;
case EOpPrototype:
- if (visit == PreVisit)
- mFunctionMap[node->getName()] = node->getSequence();
visitChildren = false;
break;
case EOpParameters:
@@ -457,50 +432,17 @@
case EOpFunctionCall:
{
// Function call.
- bool inFunctionMap = (mFunctionMap.find(node->getName()) != mFunctionMap.end());
if (visit == PreVisit)
{
// User-defined function return values are not rounded, this relies on that
// calculations producing the value were rounded.
TIntermNode *parent = getParentNode();
- if (canRoundFloat(node->getType()) && !inFunctionMap && parentUsesResult(parent, node))
+ if (canRoundFloat(node->getType()) && !isInFunctionMap(node) &&
+ parentUsesResult(parent, node))
{
TIntermNode *replacement = createRoundingFunctionCallNode(node);
mReplacements.push_back(NodeUpdateEntry(parent, node, replacement, true));
}
-
- if (inFunctionMap)
- {
- mSeqIterStack.push_back(mFunctionMap[node->getName()]->begin());
- if (mSeqIterStack.back() != mFunctionMap[node->getName()]->end())
- {
- TQualifier qualifier = (*mSeqIterStack.back())->getAsTyped()->getQualifier();
- mInFunctionCallOutParameter = (qualifier == EvqOut || qualifier == EvqInOut);
- }
- }
- else
- {
- // The function is not user-defined - it is likely built-in texture function.
- // Assume that those do not have out parameters.
- mInFunctionCallOutParameter = false;
- }
- }
- else if (visit == InVisit)
- {
- if (inFunctionMap)
- {
- ++mSeqIterStack.back();
- TQualifier qualifier = (*mSeqIterStack.back())->getAsTyped()->getQualifier();
- mInFunctionCallOutParameter = (qualifier == EvqOut || qualifier == EvqInOut);
- }
- }
- else
- {
- if (inFunctionMap)
- {
- mSeqIterStack.pop_back();
- mInFunctionCallOutParameter = false;
- }
}
break;
}
@@ -523,15 +465,10 @@
case EOpNegative:
case EOpVectorLogicalNot:
case EOpLogicalNot:
- break;
case EOpPostIncrement:
case EOpPostDecrement:
case EOpPreIncrement:
case EOpPreDecrement:
- if (visit == PreVisit)
- mInLValue = true;
- else if (visit == PostVisit)
- mInLValue = false;
break;
default:
if (canRoundFloat(node->getType()) && visit == PreVisit)
diff --git a/src/compiler/translator/EmulatePrecision.h b/src/compiler/translator/EmulatePrecision.h
index 62cea67..2a7b7fa 100644
--- a/src/compiler/translator/EmulatePrecision.h
+++ b/src/compiler/translator/EmulatePrecision.h
@@ -56,20 +56,7 @@
EmulationSet mEmulateCompoundMul;
EmulationSet mEmulateCompoundDiv;
- // Stack of function call parameter iterators
- std::vector<TIntermSequence::const_iterator> mSeqIterStack;
-
bool mDeclaringVariables;
- bool mInLValue;
- bool mInFunctionCallOutParameter;
-
- struct TStringComparator
- {
- bool operator() (const TString& a, const TString& b) const { return a.compare(b) < 0; }
- };
-
- // Map from function names to their parameter sequences
- std::map<TString, TIntermSequence*, TStringComparator> mFunctionMap;
};
#endif // COMPILER_TRANSLATOR_EMULATE_PRECISION_H_
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 5c996f7..f0a232c 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -609,7 +609,9 @@
postVisit(postVisit),
mDepth(0),
mMaxDepth(0),
- mTemporaryIndex(nullptr)
+ mTemporaryIndex(nullptr),
+ mOperatorRequiresLValue(false),
+ mInFunctionCallOutParameter(false)
{
}
virtual ~TIntermTraverser() {}
@@ -671,6 +673,35 @@
// Start creating temporary symbols from the given temporary symbol index + 1.
void useTemporaryIndex(unsigned int *temporaryIndex);
+ // Track whether an l-value is required in the node that is currently being traversed.
+ // These functions are intended to be called only from traversal functions, not from subclasses
+ // of TIntermTraverser.
+ // Use isLValueRequiredHere instead to check all conditions which require an l-value.
+ void setOperatorRequiresLValue(bool lValueRequired)
+ {
+ mOperatorRequiresLValue = lValueRequired;
+ }
+ bool operatorRequiresLValue() const { return mOperatorRequiresLValue; }
+
+ // Add a function encountered during traversal to the function map. Intended to be called only
+ // from traversal functions, not from subclasses of TIntermTraverser.
+ void addToFunctionMap(const TString &name, TIntermSequence *paramSequence);
+
+ // Return true if the prototype or definition of the function being called has been encountered
+ // during traversal.
+ bool isInFunctionMap(const TIntermAggregate *callNode) const;
+ // Return the parameters sequence from the function definition or prototype.
+ TIntermSequence *getFunctionParameters(const TIntermAggregate *callNode);
+
+ // Track whether an l-value is required inside a function call.
+ // This function is intended to be called only from traversal functions, not from traverers.
+ void setInFunctionCallOutParameter(bool inOutParameter);
+
+ bool isLValueRequiredHere() const
+ {
+ return mOperatorRequiresLValue || mInFunctionCallOutParameter;
+ }
+
protected:
int mDepth;
int mMaxDepth;
@@ -771,6 +802,17 @@
std::vector<ParentBlock> mParentBlockStack;
unsigned int *mTemporaryIndex;
+
+ bool mOperatorRequiresLValue;
+ bool mInFunctionCallOutParameter;
+
+ struct TStringComparator
+ {
+ bool operator()(const TString &a, const TString &b) const { return a.compare(b) < 0; }
+ };
+
+ // Map from mangled function names to their parameter sequences
+ TMap<TString, TIntermSequence *, TStringComparator> mFunctionMap;
};
//
diff --git a/src/compiler/translator/IntermTraverse.cpp b/src/compiler/translator/IntermTraverse.cpp
index aeaf7de..a19b546 100644
--- a/src/compiler/translator/IntermTraverse.cpp
+++ b/src/compiler/translator/IntermTraverse.cpp
@@ -96,6 +96,28 @@
++(*mTemporaryIndex);
}
+void TIntermTraverser::addToFunctionMap(const TString &name, TIntermSequence *paramSequence)
+{
+ mFunctionMap[name] = paramSequence;
+}
+
+bool TIntermTraverser::isInFunctionMap(const TIntermAggregate *callNode) const
+{
+ ASSERT(callNode->getOp() == EOpFunctionCall || callNode->getOp() == EOpInternalFunctionCall);
+ return (mFunctionMap.find(callNode->getName()) != mFunctionMap.end());
+}
+
+TIntermSequence *TIntermTraverser::getFunctionParameters(const TIntermAggregate *callNode)
+{
+ ASSERT(isInFunctionMap(callNode));
+ return mFunctionMap[callNode->getName()];
+}
+
+void TIntermTraverser::setInFunctionCallOutParameter(bool inOutParameter)
+{
+ mInFunctionCallOutParameter = inOutParameter;
+}
+
//
// Traverse the intermediate representation tree, and
// call a node type specific function for each node.
@@ -140,12 +162,23 @@
{
it->incrementDepth(this);
+ if (isAssignment())
+ {
+ // Some binary operations like indexing can be inside an l-value.
+ // TODO(oetuaho@nvidia.com): Now the code doesn't unset operatorRequiresLValue for the
+ // index, fix this.
+ it->setOperatorRequiresLValue(true);
+ }
+
if (mLeft)
mLeft->traverse(it);
if (it->inVisit)
visit = it->visitBinary(InVisit, this);
+ if (isAssignment())
+ it->setOperatorRequiresLValue(false);
+
if (visit && mRight)
mRight->traverse(it);
@@ -170,9 +203,26 @@
if (it->preVisit)
visit = it->visitUnary(PreVisit, this);
- if (visit) {
+ if (visit)
+ {
it->incrementDepth(this);
+
+ switch (getOp())
+ {
+ case EOpPostIncrement:
+ case EOpPostDecrement:
+ case EOpPreIncrement:
+ case EOpPreDecrement:
+ it->setOperatorRequiresLValue(true);
+ break;
+ default:
+ break;
+ }
+
mOperand->traverse(it);
+
+ it->setOperatorRequiresLValue(false);
+
it->decrementDepth();
}
@@ -187,36 +237,87 @@
{
bool visit = true;
+ switch (mOp)
+ {
+ case EOpFunction:
+ {
+ TIntermAggregate *params = mSequence.front()->getAsAggregate();
+ ASSERT(params != nullptr);
+ ASSERT(params->getOp() == EOpParameters);
+ it->addToFunctionMap(mName, params->getSequence());
+ break;
+ }
+ case EOpPrototype:
+ it->addToFunctionMap(mName, &mSequence);
+ break;
+ default:
+ break;
+ }
+
if (it->preVisit)
visit = it->visitAggregate(PreVisit, this);
if (visit)
{
- if (mOp == EOpSequence)
- it->pushParentBlock(this);
-
- it->incrementDepth(this);
-
- for (TIntermSequence::iterator sit = mSequence.begin();
- sit != mSequence.end(); sit++)
+ bool inFunctionMap = false;
+ if (mOp == EOpFunctionCall)
{
- (*sit)->traverse(it);
-
- if (visit && it->inVisit)
+ inFunctionMap = it->isInFunctionMap(this);
+ if (!inFunctionMap)
{
- if (*sit != mSequence.back())
- visit = it->visitAggregate(InVisit, this);
- }
- if (mOp == EOpSequence)
- {
- it->incrementParentBlockPos();
+ // The function is not user-defined - it is likely built-in texture function.
+ // Assume that those do not have out parameters.
+ it->setInFunctionCallOutParameter(false);
}
}
- it->decrementDepth();
+ it->incrementDepth(this);
- if (mOp == EOpSequence)
- it->popParentBlock();
+ if (inFunctionMap)
+ {
+ TIntermSequence *params = it->getFunctionParameters(this);
+ TIntermSequence::iterator paramIter = params->begin();
+ for (auto *child : mSequence)
+ {
+ ASSERT(paramIter != params->end());
+ TQualifier qualifier = (*paramIter)->getAsTyped()->getQualifier();
+ it->setInFunctionCallOutParameter(qualifier == EvqOut || qualifier == EvqInOut);
+
+ child->traverse(it);
+ if (visit && it->inVisit)
+ {
+ if (child != mSequence.back())
+ visit = it->visitAggregate(InVisit, this);
+ }
+
+ ++paramIter;
+ }
+
+ it->setInFunctionCallOutParameter(false);
+ }
+ else
+ {
+ if (mOp == EOpSequence)
+ it->pushParentBlock(this);
+
+ for (auto *child : mSequence)
+ {
+ child->traverse(it);
+ if (visit && it->inVisit)
+ {
+ if (child != mSequence.back())
+ visit = it->visitAggregate(InVisit, this);
+ }
+
+ if (mOp == EOpSequence)
+ it->incrementParentBlockPos();
+ }
+
+ if (mOp == EOpSequence)
+ it->popParentBlock();
+ }
+
+ it->decrementDepth();
}
if (visit && it->postVisit)