Commit 8afe1e1b by Olli Etuaho

Track that indices of l-values are not required to be l-values

In an expression like a[ind]++, a[ind] is required to be an l-value but ind is not. Reset the l-value required flags before traversing the index of an indexing operation, so that this is accurately tracked. After the index has been traversed, the previous state of the l-value required flags is restored. New tests are added to angle_unittests cover this functionality. TEST=angle_unittests BUG=angleproject:1116 Change-Id: I8929ec01e85e672c83ef7d385e455b7df8682f4b Reviewed-on: https://chromium-review.googlesource.com/290561Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarZhenyao Mo <zmo@chromium.org> Tested-by: 's avatarOlli Etuaho <oetuaho@nvidia.com>
parent a26ad58d
...@@ -694,8 +694,9 @@ class TIntermTraverser : angle::NonCopyable ...@@ -694,8 +694,9 @@ class TIntermTraverser : angle::NonCopyable
TIntermSequence *getFunctionParameters(const TIntermAggregate *callNode); TIntermSequence *getFunctionParameters(const TIntermAggregate *callNode);
// Track whether an l-value is required inside a function call. // 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. // These functions are intended to be called only from traversal functions, not from traverers.
void setInFunctionCallOutParameter(bool inOutParameter); void setInFunctionCallOutParameter(bool inOutParameter);
bool isInFunctionCallOutParameter() const;
bool isLValueRequiredHere() const bool isLValueRequiredHere() const
{ {
......
...@@ -118,6 +118,11 @@ void TIntermTraverser::setInFunctionCallOutParameter(bool inOutParameter) ...@@ -118,6 +118,11 @@ void TIntermTraverser::setInFunctionCallOutParameter(bool inOutParameter)
mInFunctionCallOutParameter = inOutParameter; mInFunctionCallOutParameter = inOutParameter;
} }
bool TIntermTraverser::isInFunctionCallOutParameter() const
{
return mInFunctionCallOutParameter;
}
// //
// Traverse the intermediate representation tree, and // Traverse the intermediate representation tree, and
// call a node type specific function for each node. // call a node type specific function for each node.
...@@ -162,11 +167,13 @@ void TIntermBinary::traverse(TIntermTraverser *it) ...@@ -162,11 +167,13 @@ void TIntermBinary::traverse(TIntermTraverser *it)
{ {
it->incrementDepth(this); it->incrementDepth(this);
// Some binary operations like indexing can be inside an expression which must be an
// l-value.
bool parentOperatorRequiresLValue = it->operatorRequiresLValue();
bool parentInFunctionCallOutParameter = it->isInFunctionCallOutParameter();
if (isAssignment()) if (isAssignment())
{ {
// Some binary operations like indexing can be inside an l-value. ASSERT(!it->isLValueRequiredHere());
// TODO(oetuaho@nvidia.com): Now the code doesn't unset operatorRequiresLValue for the
// index, fix this.
it->setOperatorRequiresLValue(true); it->setOperatorRequiresLValue(true);
} }
...@@ -179,9 +186,21 @@ void TIntermBinary::traverse(TIntermTraverser *it) ...@@ -179,9 +186,21 @@ void TIntermBinary::traverse(TIntermTraverser *it)
if (isAssignment()) if (isAssignment())
it->setOperatorRequiresLValue(false); it->setOperatorRequiresLValue(false);
// Index is not required to be an l-value even when the surrounding expression is required
// to be an l-value.
if (mOp == EOpIndexDirect || mOp == EOpIndexDirectInterfaceBlock ||
mOp == EOpIndexDirectStruct || mOp == EOpIndexIndirect)
{
it->setOperatorRequiresLValue(false);
it->setInFunctionCallOutParameter(false);
}
if (visit && mRight) if (visit && mRight)
mRight->traverse(it); mRight->traverse(it);
it->setOperatorRequiresLValue(parentOperatorRequiresLValue);
it->setInFunctionCallOutParameter(parentInFunctionCallOutParameter);
it->decrementDepth(); it->decrementDepth();
} }
...@@ -207,6 +226,7 @@ void TIntermUnary::traverse(TIntermTraverser *it) ...@@ -207,6 +226,7 @@ void TIntermUnary::traverse(TIntermTraverser *it)
{ {
it->incrementDepth(this); it->incrementDepth(this);
ASSERT(!it->operatorRequiresLValue());
switch (getOp()) switch (getOp())
{ {
case EOpPostIncrement: case EOpPostIncrement:
......
...@@ -771,3 +771,35 @@ TEST_F(DebugShaderPrecisionTest, NestedFunctionCalls) ...@@ -771,3 +771,35 @@ TEST_F(DebugShaderPrecisionTest, NestedFunctionCalls)
// Test nested calls // Test nested calls
ASSERT_TRUE(foundInCode("v2 = add(compound_add(v, angle_frm(u2)), angle_frm(fract(angle_frm(u3))))")); ASSERT_TRUE(foundInCode("v2 = add(compound_add(v, angle_frm(u2)), angle_frm(fract(angle_frm(u3))))"));
} }
// Test that code inside an index of a function out parameter gets processed.
TEST_F(DebugShaderPrecisionTest, OpInIndexOfFunctionOutParameter)
{
const std::string &shaderString =
"precision mediump float;\n"
"void foo(out vec4 f) { f.x = 0.0; }\n"
"uniform float u2;\n"
"void main() {\n"
" vec4 v[2];\n"
" foo(v[int(exp2(u2))]);\n"
" gl_FragColor = v[0];\n"
"}\n";
compile(shaderString);
ASSERT_TRUE(foundInCode("angle_frm(exp2(angle_frm(u2)))"));
}
// Test that code inside an index of an l-value gets processed.
TEST_F(DebugShaderPrecisionTest, OpInIndexOfLValue)
{
const std::string &shaderString =
"precision mediump float;\n"
"uniform vec4 u1;\n"
"uniform float u2;\n"
"void main() {\n"
" vec4 v[2];\n"
" v[int(exp2(u2))] = u1;\n"
" gl_FragColor = v[0];\n"
"}\n";
compile(shaderString);
ASSERT_TRUE(foundInCode("angle_frm(exp2(angle_frm(u2)))"));
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment