Use TLValueTrackingTraverser in ValidateLimitations
Use TLValueTrackingTraverser to determine whether a loop index is used
as an l-value. This replaces custom logic in ValidateLimitations,
greatly simplifying the code. Also pass the symbol table to
ValidateLimitations as a parameter, which removes the need to store a
global pointer to the current ParseContext.
BUG=angleproject:1960
TEST=angle_unittests, WebGL conformance tests
Change-Id: I122c85c78bbea05833d7c787cd184de568c5c45f
Reviewed-on: https://chromium-review.googlesource.com/465606
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 9cedfab..a09c505 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -17,7 +17,6 @@
#include "compiler/translator/EmulateGLFragColorBroadcast.h"
#include "compiler/translator/EmulatePrecision.h"
#include "compiler/translator/Initialize.h"
-#include "compiler/translator/InitializeParseContext.h"
#include "compiler/translator/InitializeVariables.h"
#include "compiler/translator/ParseContext.h"
#include "compiler/translator/PruneEmptyDeclarations.h"
@@ -293,7 +292,6 @@
compileOptions, true, &mDiagnostics, getResources());
parseContext.setFragmentPrecisionHighOnESSL1(fragmentPrecisionHigh);
- SetGlobalParseContext(&parseContext);
// We preserve symbols at the built-in level from compile-to-compile.
// Start pushing the user-defined symbols at global level.
@@ -368,7 +366,8 @@
success = validateOutputs(root);
if (success && shouldRunLoopAndIndexingValidation(compileOptions))
- success = validateLimitations(root);
+ success =
+ ValidateLimitations(root, shaderType, symbolTable, shaderVersion, &mDiagnostics);
bool multiview2 = IsExtensionEnabled(extensionBehavior, "GL_OVR_multiview2");
if (success && compileResources.OVR_multiview && IsWebGLBasedSpec(shaderSpec) &&
@@ -477,7 +476,6 @@
}
}
- SetGlobalParseContext(NULL);
if (success)
return root;
@@ -854,13 +852,6 @@
return (mDiagnostics.numErrors() == 0);
}
-bool TCompiler::validateLimitations(TIntermNode *root)
-{
- ValidateLimitations validate(shaderType, &mDiagnostics);
- root->traverse(&validate);
- return mDiagnostics.numErrors() == 0;
-}
-
bool TCompiler::limitExpressionComplexity(TIntermNode *root)
{
TMaxDepthTraverser traverser(maxExpressionComplexity + 1);
diff --git a/src/compiler/translator/Compiler.h b/src/compiler/translator/Compiler.h
index cb484df..ffc063f 100644
--- a/src/compiler/translator/Compiler.h
+++ b/src/compiler/translator/Compiler.h
@@ -135,9 +135,6 @@
bool checkCallDepth();
// Returns true if a program has no conflicting or missing fragment outputs
bool validateOutputs(TIntermNode *root);
- // Returns true if the given shader does not exceed the minimum
- // functionality mandated in GLSL 1.0 spec Appendix A.
- bool validateLimitations(TIntermNode *root);
// Add emulated functions to the built-in function emulator.
virtual void initBuiltInFunctionEmulator(BuiltInFunctionEmulator *emu,
ShCompileOptions compileOptions){};
diff --git a/src/compiler/translator/InitializeDll.cpp b/src/compiler/translator/InitializeDll.cpp
index c6e16f5..43c215f 100644
--- a/src/compiler/translator/InitializeDll.cpp
+++ b/src/compiler/translator/InitializeDll.cpp
@@ -7,7 +7,6 @@
#include "compiler/translator/Cache.h"
#include "compiler/translator/InitializeDll.h"
#include "compiler/translator/InitializeGlobals.h"
-#include "compiler/translator/InitializeParseContext.h"
#include "common/platform.h"
@@ -24,12 +23,6 @@
return false;
}
- if (!InitializeParseContextIndex())
- {
- assert(0 && "InitProcess(): Failed to initalize parse context");
- return false;
- }
-
TCache::initialize();
return true;
@@ -37,7 +30,6 @@
void DetachProcess()
{
- FreeParseContextIndex();
FreePoolIndex();
TCache::destroy();
}
diff --git a/src/compiler/translator/InitializeParseContext.cpp b/src/compiler/translator/InitializeParseContext.cpp
deleted file mode 100644
index 751ee36..0000000
--- a/src/compiler/translator/InitializeParseContext.cpp
+++ /dev/null
@@ -1,46 +0,0 @@
-//
-// Copyright (c) 2012 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.
-//
-
-#include "compiler/translator/InitializeParseContext.h"
-
-#include "common/tls.h"
-
-#include <assert.h>
-
-namespace sh
-{
-
-TLSIndex GlobalParseContextIndex = TLS_INVALID_INDEX;
-
-bool InitializeParseContextIndex()
-{
- assert(GlobalParseContextIndex == TLS_INVALID_INDEX);
-
- GlobalParseContextIndex = CreateTLSIndex();
- return GlobalParseContextIndex != TLS_INVALID_INDEX;
-}
-
-void FreeParseContextIndex()
-{
- assert(GlobalParseContextIndex != TLS_INVALID_INDEX);
-
- DestroyTLSIndex(GlobalParseContextIndex);
- GlobalParseContextIndex = TLS_INVALID_INDEX;
-}
-
-void SetGlobalParseContext(TParseContext *context)
-{
- assert(GlobalParseContextIndex != TLS_INVALID_INDEX);
- SetTLSValue(GlobalParseContextIndex, context);
-}
-
-TParseContext *GetGlobalParseContext()
-{
- assert(GlobalParseContextIndex != TLS_INVALID_INDEX);
- return static_cast<TParseContext *>(GetTLSValue(GlobalParseContextIndex));
-}
-
-} // namespace sh
diff --git a/src/compiler/translator/InitializeParseContext.h b/src/compiler/translator/InitializeParseContext.h
deleted file mode 100644
index ad93c03..0000000
--- a/src/compiler/translator/InitializeParseContext.h
+++ /dev/null
@@ -1,21 +0,0 @@
-//
-// Copyright (c) 2002-2010 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.
-//
-
-#ifndef COMPILER_TRANSLATOR_INITIALIZEPARSECONTEXT_H_
-#define COMPILER_TRANSLATOR_INITIALIZEPARSECONTEXT_H_
-
-namespace sh
-{
-
-bool InitializeParseContextIndex();
-void FreeParseContextIndex();
-
-class TParseContext;
-extern void SetGlobalParseContext(TParseContext *context);
-extern TParseContext *GetGlobalParseContext();
-} // namespace sh
-
-#endif // COMPILER_TRANSLATOR_INITIALIZEPARSECONTEXT_H_
diff --git a/src/compiler/translator/ValidateLimitations.cpp b/src/compiler/translator/ValidateLimitations.cpp
index e647e4c..3a17614 100644
--- a/src/compiler/translator/ValidateLimitations.cpp
+++ b/src/compiler/translator/ValidateLimitations.cpp
@@ -6,10 +6,9 @@
#include "compiler/translator/ValidateLimitations.h"
-#include "compiler/translator/Diagnostics.h"
-#include "compiler/translator/InitializeParseContext.h"
-#include "compiler/translator/ParseContext.h"
#include "angle_gl.h"
+#include "compiler/translator/Diagnostics.h"
+#include "compiler/translator/ParseContext.h"
namespace sh
{
@@ -65,30 +64,73 @@
const std::vector<int> mLoopSymbolIds;
};
-} // namespace anonymous
+// Traverses intermediate tree to ensure that the shader does not exceed the
+// minimum functionality mandated in GLSL 1.0 spec, Appendix A.
+class ValidateLimitationsTraverser : public TLValueTrackingTraverser
+{
+ public:
+ ValidateLimitationsTraverser(sh::GLenum shaderType,
+ const TSymbolTable &symbolTable,
+ int shaderVersion,
+ TDiagnostics *diagnostics);
-ValidateLimitations::ValidateLimitations(sh::GLenum shaderType, TDiagnostics *diagnostics)
- : TIntermTraverser(true, false, false),
+ void visitSymbol(TIntermSymbol *node) override;
+ bool visitBinary(Visit, TIntermBinary *) override;
+ bool visitLoop(Visit, TIntermLoop *) override;
+
+ private:
+ void error(TSourceLoc loc, const char *reason, const char *token);
+
+ bool withinLoopBody() const;
+ bool isLoopIndex(TIntermSymbol *symbol);
+ bool validateLoopType(TIntermLoop *node);
+
+ bool validateForLoopHeader(TIntermLoop *node);
+ // If valid, return the index symbol id; Otherwise, return -1.
+ int validateForLoopInit(TIntermLoop *node);
+ bool validateForLoopCond(TIntermLoop *node, int indexSymbolId);
+ bool validateForLoopExpr(TIntermLoop *node, int indexSymbolId);
+
+ // Returns true if indexing does not exceed the minimum functionality
+ // mandated in GLSL 1.0 spec, Appendix A, Section 5.
+ bool isConstExpr(TIntermNode *node);
+ bool isConstIndexExpr(TIntermNode *node);
+ bool validateIndexing(TIntermBinary *node);
+
+ sh::GLenum mShaderType;
+ TDiagnostics *mDiagnostics;
+ std::vector<int> mLoopSymbolIds;
+};
+
+ValidateLimitationsTraverser::ValidateLimitationsTraverser(sh::GLenum shaderType,
+ const TSymbolTable &symbolTable,
+ int shaderVersion,
+ TDiagnostics *diagnostics)
+ : TLValueTrackingTraverser(true, false, false, symbolTable, shaderVersion),
mShaderType(shaderType),
- mDiagnostics(diagnostics),
- mValidateIndexing(true),
- mValidateInnerLoops(true)
+ mDiagnostics(diagnostics)
{
ASSERT(diagnostics);
}
-bool ValidateLimitations::visitBinary(Visit, TIntermBinary *node)
+void ValidateLimitationsTraverser::visitSymbol(TIntermSymbol *node)
{
- // Check if loop index is modified in the loop body.
- validateOperation(node, node->getLeft());
+ if (isLoopIndex(node) && isLValueRequiredHere())
+ {
+ error(node->getLine(),
+ "Loop index cannot be statically assigned to within the body of the loop",
+ node->getSymbol().c_str());
+ }
+}
+bool ValidateLimitationsTraverser::visitBinary(Visit, TIntermBinary *node)
+{
// Check indexing.
switch (node->getOp())
{
case EOpIndexDirect:
case EOpIndexIndirect:
- if (mValidateIndexing)
- validateIndexing(node);
+ validateIndexing(node);
break;
default:
break;
@@ -96,33 +138,8 @@
return true;
}
-bool ValidateLimitations::visitUnary(Visit, TIntermUnary *node)
+bool ValidateLimitationsTraverser::visitLoop(Visit, TIntermLoop *node)
{
- // Check if loop index is modified in the loop body.
- validateOperation(node, node->getOperand());
-
- return true;
-}
-
-bool ValidateLimitations::visitAggregate(Visit, TIntermAggregate *node)
-{
- switch (node->getOp())
- {
- case EOpCallFunctionInAST:
- case EOpCallBuiltInFunction:
- validateFunctionCall(node);
- break;
- default:
- break;
- }
- return true;
-}
-
-bool ValidateLimitations::visitLoop(Visit, TIntermLoop *node)
-{
- if (!mValidateInnerLoops)
- return true;
-
if (!validateLoopType(node))
return false;
@@ -141,23 +158,23 @@
return false;
}
-void ValidateLimitations::error(TSourceLoc loc, const char *reason, const char *token)
+void ValidateLimitationsTraverser::error(TSourceLoc loc, const char *reason, const char *token)
{
mDiagnostics->error(loc, reason, token);
}
-bool ValidateLimitations::withinLoopBody() const
+bool ValidateLimitationsTraverser::withinLoopBody() const
{
return !mLoopSymbolIds.empty();
}
-bool ValidateLimitations::isLoopIndex(TIntermSymbol *symbol)
+bool ValidateLimitationsTraverser::isLoopIndex(TIntermSymbol *symbol)
{
return std::find(mLoopSymbolIds.begin(), mLoopSymbolIds.end(), symbol->getId()) !=
mLoopSymbolIds.end();
}
-bool ValidateLimitations::validateLoopType(TIntermLoop *node)
+bool ValidateLimitationsTraverser::validateLoopType(TIntermLoop *node)
{
TLoopType type = node->getType();
if (type == ELoopFor)
@@ -168,7 +185,7 @@
return false;
}
-bool ValidateLimitations::validateForLoopHeader(TIntermLoop *node)
+bool ValidateLimitationsTraverser::validateForLoopHeader(TIntermLoop *node)
{
ASSERT(node->getType() == ELoopFor);
@@ -187,7 +204,7 @@
return true;
}
-int ValidateLimitations::validateForLoopInit(TIntermLoop *node)
+int ValidateLimitationsTraverser::validateForLoopInit(TIntermLoop *node)
{
TIntermNode *init = node->getInit();
if (init == NULL)
@@ -243,7 +260,7 @@
return symbol->getId();
}
-bool ValidateLimitations::validateForLoopCond(TIntermLoop *node, int indexSymbolId)
+bool ValidateLimitationsTraverser::validateForLoopCond(TIntermLoop *node, int indexSymbolId)
{
TIntermNode *cond = node->getCondition();
if (cond == NULL)
@@ -299,7 +316,7 @@
return true;
}
-bool ValidateLimitations::validateForLoopExpr(TIntermLoop *node, int indexSymbolId)
+bool ValidateLimitationsTraverser::validateForLoopExpr(TIntermLoop *node, int indexSymbolId)
{
TIntermNode *expr = node->getExpression();
if (expr == NULL)
@@ -377,77 +394,13 @@
return true;
}
-bool ValidateLimitations::validateFunctionCall(TIntermAggregate *node)
-{
- ASSERT(node->getOp() == EOpCallFunctionInAST || node->getOp() == EOpCallBuiltInFunction);
-
- // If not within loop body, there is nothing to check.
- if (!withinLoopBody())
- return true;
-
- // List of param indices for which loop indices are used as argument.
- typedef std::vector<size_t> ParamIndex;
- ParamIndex pIndex;
- TIntermSequence *params = node->getSequence();
- for (TIntermSequence::size_type i = 0; i < params->size(); ++i)
- {
- TIntermSymbol *symbol = (*params)[i]->getAsSymbolNode();
- if (symbol && isLoopIndex(symbol))
- pIndex.push_back(i);
- }
- // If none of the loop indices are used as arguments,
- // there is nothing to check.
- if (pIndex.empty())
- return true;
-
- bool valid = true;
- TSymbolTable &symbolTable = GetGlobalParseContext()->symbolTable;
- // TODO(oetuaho@nvidia.com): It would be neater to leverage TIntermLValueTrackingTraverser to
- // keep track of out parameters, rather than doing a symbol table lookup here.
- TString mangledName = TFunction::GetMangledNameFromCall(
- node->getFunctionSymbolInfo()->getName(), *node->getSequence());
- TSymbol *symbol = symbolTable.find(mangledName, GetGlobalParseContext()->getShaderVersion());
- ASSERT(symbol && symbol->isFunction());
- TFunction *function = static_cast<TFunction *>(symbol);
- for (ParamIndex::const_iterator i = pIndex.begin(); i != pIndex.end(); ++i)
- {
- const TConstParameter ¶m = function->getParam(*i);
- TQualifier qual = param.type->getQualifier();
- if ((qual == EvqOut) || (qual == EvqInOut))
- {
- error((*params)[*i]->getLine(),
- "Loop index cannot be used as argument to a function out or inout parameter",
- (*params)[*i]->getAsSymbolNode()->getSymbol().c_str());
- valid = false;
- }
- }
-
- return valid;
-}
-
-bool ValidateLimitations::validateOperation(TIntermOperator *node, TIntermNode *operand)
-{
- // Check if loop index is modified in the loop body.
- if (!withinLoopBody() || !node->isAssignment())
- return true;
-
- TIntermSymbol *symbol = operand->getAsSymbolNode();
- if (symbol && isLoopIndex(symbol))
- {
- error(node->getLine(),
- "Loop index cannot be statically assigned to within the body of the loop",
- symbol->getSymbol().c_str());
- }
- return true;
-}
-
-bool ValidateLimitations::isConstExpr(TIntermNode *node)
+bool ValidateLimitationsTraverser::isConstExpr(TIntermNode *node)
{
ASSERT(node != nullptr);
return node->getAsConstantUnion() != nullptr && node->getAsTyped()->getQualifier() == EvqConst;
}
-bool ValidateLimitations::isConstIndexExpr(TIntermNode *node)
+bool ValidateLimitationsTraverser::isConstIndexExpr(TIntermNode *node)
{
ASSERT(node != NULL);
@@ -456,7 +409,7 @@
return validate.isValid();
}
-bool ValidateLimitations::validateIndexing(TIntermBinary *node)
+bool ValidateLimitationsTraverser::validateIndexing(TIntermBinary *node)
{
ASSERT((node->getOp() == EOpIndexDirect) || (node->getOp() == EOpIndexIndirect));
@@ -474,4 +427,17 @@
return valid;
}
+} // namespace anonymous
+
+bool ValidateLimitations(TIntermNode *root,
+ GLenum shaderType,
+ const TSymbolTable &symbolTable,
+ int shaderVersion,
+ TDiagnostics *diagnostics)
+{
+ ValidateLimitationsTraverser validate(shaderType, symbolTable, shaderVersion, diagnostics);
+ root->traverse(&validate);
+ return diagnostics->numErrors() == 0;
+}
+
} // namespace sh
diff --git a/src/compiler/translator/ValidateLimitations.h b/src/compiler/translator/ValidateLimitations.h
index 3bc79d5..6947fde 100644
--- a/src/compiler/translator/ValidateLimitations.h
+++ b/src/compiler/translator/ValidateLimitations.h
@@ -14,48 +14,13 @@
class TDiagnostics;
-// Traverses intermediate tree to ensure that the shader does not exceed the
-// minimum functionality mandated in GLSL 1.0 spec, Appendix A.
-class ValidateLimitations : public TIntermTraverser
-{
- public:
- ValidateLimitations(sh::GLenum shaderType, TDiagnostics *diagnostics);
-
- bool visitBinary(Visit, TIntermBinary *) override;
- bool visitUnary(Visit, TIntermUnary *) override;
- bool visitAggregate(Visit, TIntermAggregate *) override;
- bool visitLoop(Visit, TIntermLoop *) override;
-
- private:
- void error(TSourceLoc loc, const char *reason, const char *token);
-
- bool withinLoopBody() const;
- bool isLoopIndex(TIntermSymbol *symbol);
- bool validateLoopType(TIntermLoop *node);
-
- bool validateForLoopHeader(TIntermLoop *node);
- // If valid, return the index symbol id; Otherwise, return -1.
- int validateForLoopInit(TIntermLoop *node);
- bool validateForLoopCond(TIntermLoop *node, int indexSymbolId);
- bool validateForLoopExpr(TIntermLoop *node, int indexSymbolId);
-
- // Returns true if none of the loop indices is used as the argument to
- // the given function out or inout parameter.
- bool validateFunctionCall(TIntermAggregate *node);
- bool validateOperation(TIntermOperator *node, TIntermNode *operand);
-
- // Returns true if indexing does not exceed the minimum functionality
- // mandated in GLSL 1.0 spec, Appendix A, Section 5.
- bool isConstExpr(TIntermNode *node);
- bool isConstIndexExpr(TIntermNode *node);
- bool validateIndexing(TIntermBinary *node);
-
- sh::GLenum mShaderType;
- TDiagnostics *mDiagnostics;
- std::vector<int> mLoopSymbolIds;
- bool mValidateIndexing;
- bool mValidateInnerLoops;
-};
+// Returns true if the given shader does not exceed the minimum functionality mandated in GLSL ES
+// 1.00 spec Appendix A.
+bool ValidateLimitations(TIntermNode *root,
+ GLenum shaderType,
+ const TSymbolTable &symbolTable,
+ int shaderVersion,
+ TDiagnostics *diagnostics);
} // namespace sh
diff --git a/src/compiler/translator/ValidateOutputs.cpp b/src/compiler/translator/ValidateOutputs.cpp
index 733fe9a..27ab814 100644
--- a/src/compiler/translator/ValidateOutputs.cpp
+++ b/src/compiler/translator/ValidateOutputs.cpp
@@ -6,7 +6,6 @@
#include "compiler/translator/ValidateOutputs.h"
#include "compiler/translator/InfoSink.h"
-#include "compiler/translator/InitializeParseContext.h"
#include "compiler/translator/ParseContext.h"
namespace sh