Initial support for compiler AST validation
This is to be able to catch issues introduced through AST manipulation
earlier than shader compilation time, improving debuggability.
Bug: angleproject:2733
Change-Id: Ic57bc72f2ab438e60f32553d602074f3d72cc4f5
Reviewed-on: https://chromium-review.googlesource.com/c/1199922
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 95082d9..afc8901 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -493,6 +493,10 @@
FoldExpressions(root, &mDiagnostics);
// Folding should only be able to generate warnings.
ASSERT(mDiagnostics.numErrors() == 0);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
// We prune no-ops to work around driver bugs and to keep AST processing and output simple.
// The following kinds of no-ops are pruned:
@@ -502,6 +506,10 @@
// invalid ESSL.
// After this empty declarations are not allowed in the AST.
PruneNoOps(root, &mSymbolTable);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
// Create the function DAG and check there is no recursion
if (!initCallDag(root))
@@ -525,6 +533,10 @@
if (!(compileOptions & SH_DONT_PRUNE_UNUSED_FUNCTIONS))
{
pruneUnusedFunctions(root);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (mShaderVersion >= 310 && !ValidateVaryingLocations(root, &mDiagnostics, mShaderType))
@@ -558,29 +570,57 @@
{
DeclareAndInitBuiltinsForInstancedMultiview(root, mNumViews, mShaderType, compileOptions,
mOutputType, &mSymbolTable);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
// This pass might emit short circuits so keep it before the short circuit unfolding
if (compileOptions & SH_REWRITE_DO_WHILE_LOOPS)
+ {
RewriteDoWhile(root, &mSymbolTable);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
+ }
if (compileOptions & SH_ADD_AND_TRUE_TO_LOOP_CONDITION)
+ {
AddAndTrueToLoopCondition(root);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
+ }
if (compileOptions & SH_UNFOLD_SHORT_CIRCUIT)
{
UnfoldShortCircuitAST(root);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (compileOptions & SH_REMOVE_POW_WITH_CONSTANT_EXPONENT)
{
RemovePow(root, &mSymbolTable);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (compileOptions & SH_REGENERATE_STRUCT_NAMES)
{
RegenerateStructNames gen(&mSymbolTable);
root->traverse(&gen);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (mShaderType == GL_VERTEX_SHADER &&
@@ -590,6 +630,10 @@
{
EmulateGLDrawID(root, &mSymbolTable, &mUniforms,
shouldCollectVariables(compileOptions));
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
}
@@ -599,6 +643,10 @@
{
EmulateGLFragColorBroadcast(root, mResources.MaxDrawBuffers, &mOutputVariables,
&mSymbolTable, mShaderVersion);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
int simplifyScalarized = (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS)
@@ -612,17 +660,38 @@
IntermNodePatternMatcher::kMultiDeclaration |
IntermNodePatternMatcher::kArrayLengthMethod | simplifyScalarized,
&getSymbolTable());
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
// Note that separate declarations need to be run before other AST transformations that
// generate new statements from expressions.
SeparateDeclarations(root);
+ mValidateASTOptions.validateMultiDeclarations = true;
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod | simplifyScalarized,
&getSymbolTable());
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
RemoveArrayLengthMethod(root);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
RemoveUnreferencedVariables(root, &mSymbolTable);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
// In case the last case inside a switch statement is a certain type of no-op, GLSL compilers in
// drivers may not accept it. In this case we clean up the dead code from the end of switch
@@ -631,6 +700,10 @@
// invalid state. Relies on that PruneNoOps and RemoveUnreferencedVariables have already been
// run.
PruneEmptyCases(root);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
// Built-in function emulation needs to happen after validateLimitations pass.
// TODO(jmadill): Remove global pool allocator.
@@ -644,6 +717,10 @@
if (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS)
{
ScalarizeVecAndMatConstructorArgs(root, mShaderType, highPrecisionSupported, &mSymbolTable);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (shouldCollectVariables(compileOptions))
@@ -672,6 +749,10 @@
if ((compileOptions & SH_INIT_OUTPUT_VARIABLES) && (mShaderType != GL_COMPUTE_SHADER))
{
initializeOutputVariables(root);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
}
@@ -680,6 +761,10 @@
if (RemoveInvariant(mShaderType, mShaderVersion, mOutputType, compileOptions))
{
RemoveInvariantDeclaration(root);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
// gl_Position is always written in compatibility output mode.
@@ -690,6 +775,10 @@
{
initializeGLPosition(root);
mGLPositionInitialized = true;
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
// DeferGlobalInitializers needs to be run before other AST transformations that generate new
@@ -701,6 +790,10 @@
bool canUseLoopsToInitialize = !(compileOptions & SH_DONT_USE_LOOPS_TO_INITIALIZE_VARIABLES);
DeferGlobalInitializers(root, initializeLocalsAndGlobals, canUseLoopsToInitialize,
highPrecisionSupported, &mSymbolTable);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
if (initializeLocalsAndGlobals)
{
@@ -718,30 +811,54 @@
IntermNodePatternMatcher::kArrayDeclaration |
IntermNodePatternMatcher::kNamelessStructDeclaration,
&getSymbolTable());
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
InitializeUninitializedLocals(root, getShaderVersion(), canUseLoopsToInitialize,
highPrecisionSupported, &getSymbolTable());
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (getShaderType() == GL_VERTEX_SHADER && (compileOptions & SH_CLAMP_POINT_SIZE))
{
ClampPointSize(root, mResources.MaxPointSize, &getSymbolTable());
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (getShaderType() == GL_FRAGMENT_SHADER && (compileOptions & SH_CLAMP_FRAG_DEPTH))
{
ClampFragDepth(root, &getSymbolTable());
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (compileOptions & SH_REWRITE_REPEATED_ASSIGN_TO_SWIZZLED)
{
sh::RewriteRepeatedAssignToSwizzled(root);
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
if (compileOptions & SH_REWRITE_VECTOR_SCALAR_ARITHMETIC)
{
VectorizeVectorScalarArithmetic(root, &getSymbolTable());
+ if (!ValidateAST(root, &mDiagnostics, mValidateASTOptions))
+ {
+ return false;
+ }
}
return true;
diff --git a/src/compiler/translator/Compiler.h b/src/compiler/translator/Compiler.h
index 2eef387..86590b3 100644
--- a/src/compiler/translator/Compiler.h
+++ b/src/compiler/translator/Compiler.h
@@ -24,6 +24,7 @@
#include "compiler/translator/InfoSink.h"
#include "compiler/translator/Pragma.h"
#include "compiler/translator/SymbolTable.h"
+#include "compiler/translator/ValidateAST.h"
#include "third_party/compiler/ArrayBoundsClamper.h"
namespace sh
@@ -277,6 +278,9 @@
NameMap mNameMap;
TPragma mPragma;
+
+ // Track what should be validated given passes currently applied.
+ ValidateASTOptions mValidateASTOptions;
};
//
diff --git a/src/compiler/translator/ValidateAST.cpp b/src/compiler/translator/ValidateAST.cpp
new file mode 100644
index 0000000..c0ec114
--- /dev/null
+++ b/src/compiler/translator/ValidateAST.cpp
@@ -0,0 +1,251 @@
+//
+// Copyright 2019 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/ValidateAST.h"
+
+#include "compiler/translator/Diagnostics.h"
+#include "compiler/translator/tree_util/IntermTraverse.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class ValidateAST : public TIntermTraverser
+{
+ public:
+ static bool validate(TIntermNode *root,
+ TDiagnostics *diagnostics,
+ const ValidateASTOptions &options);
+
+ void visitSymbol(TIntermSymbol *node) override;
+ void visitConstantUnion(TIntermConstantUnion *node) override;
+ bool visitSwizzle(Visit visit, TIntermSwizzle *node) override;
+ bool visitBinary(Visit visit, TIntermBinary *node) override;
+ bool visitUnary(Visit visit, TIntermUnary *node) override;
+ bool visitTernary(Visit visit, TIntermTernary *node) override;
+ bool visitIfElse(Visit visit, TIntermIfElse *node) override;
+ bool visitSwitch(Visit visit, TIntermSwitch *node) override;
+ bool visitCase(Visit visit, TIntermCase *node) override;
+ void visitFunctionPrototype(TIntermFunctionPrototype *node) override;
+ bool visitFunctionDefinition(Visit visit, TIntermFunctionDefinition *node) override;
+ bool visitAggregate(Visit visit, TIntermAggregate *node) override;
+ bool visitBlock(Visit visit, TIntermBlock *node) override;
+ bool visitInvariantDeclaration(Visit visit, TIntermInvariantDeclaration *node) override;
+ bool visitDeclaration(Visit visit, TIntermDeclaration *node) override;
+ bool visitLoop(Visit visit, TIntermLoop *node) override;
+ bool visitBranch(Visit visit, TIntermBranch *node) override;
+ void visitPreprocessorDirective(TIntermPreprocessorDirective *node) override;
+
+ private:
+ ValidateAST(TIntermNode *root, TDiagnostics *diagnostics, const ValidateASTOptions &options);
+
+ // Visit as a generic node
+ void visitNode(Visit visit, TIntermNode *node);
+
+ void expectNonNullChildren(Visit visit, TIntermNode *node, size_t least_count);
+
+ bool validateInternal();
+
+ ValidateASTOptions mOptions;
+ TDiagnostics *mDiagnostics;
+
+ // For validateSingleParent:
+ std::map<TIntermNode *, TIntermNode *> mParent;
+ bool mSingleParentFailed = false;
+
+ // For validateNullNodes
+ bool mNullNodesFailed = false;
+};
+
+bool ValidateAST::validate(TIntermNode *root,
+ TDiagnostics *diagnostics,
+ const ValidateASTOptions &options)
+{
+ ValidateAST validate(root, diagnostics, options);
+ root->traverse(&validate);
+ return validate.validateInternal();
+}
+
+ValidateAST::ValidateAST(TIntermNode *root,
+ TDiagnostics *diagnostics,
+ const ValidateASTOptions &options)
+ : TIntermTraverser(true, false, true, nullptr), mOptions(options), mDiagnostics(diagnostics)
+{
+ if (mOptions.validateSingleParent)
+ {
+ mParent[root] = nullptr;
+ }
+}
+
+void ValidateAST::visitNode(Visit visit, TIntermNode *node)
+{
+ if (visit == PreVisit && mOptions.validateSingleParent)
+ {
+ size_t childCount = node->getChildCount();
+ for (size_t i = 0; i < childCount; ++i)
+ {
+ TIntermNode *child = node->getChildNode(i);
+ if (mParent.find(child) != mParent.end())
+ {
+ // If child is visited twice but through the same parent, the problem is in one of
+ // the ancestors.
+ if (mParent[child] != node)
+ {
+ mDiagnostics->error(node->getLine(), "Found child with two parents",
+ "<validateSingleParent>");
+ mSingleParentFailed = true;
+ }
+ }
+
+ mParent[child] = node;
+ }
+ }
+}
+
+void ValidateAST::expectNonNullChildren(Visit visit, TIntermNode *node, size_t least_count)
+{
+ if (visit == PreVisit && mOptions.validateNullNodes)
+ {
+ size_t childCount = node->getChildCount();
+ if (childCount < least_count)
+ {
+ mDiagnostics->error(node->getLine(), "Too few children", "<validateNullNodes>");
+ mNullNodesFailed = true;
+ }
+
+ for (size_t i = 0; i < childCount; ++i)
+ {
+ if (node->getChildNode(i) == nullptr)
+ {
+ mDiagnostics->error(node->getLine(), "Found nullptr child", "<validateNullNodes>");
+ mNullNodesFailed = true;
+ }
+ }
+ }
+}
+
+void ValidateAST::visitSymbol(TIntermSymbol *node)
+{
+ visitNode(PreVisit, node);
+}
+
+void ValidateAST::visitConstantUnion(TIntermConstantUnion *node)
+{
+ visitNode(PreVisit, node);
+}
+
+bool ValidateAST::visitSwizzle(Visit visit, TIntermSwizzle *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitBinary(Visit visit, TIntermBinary *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitUnary(Visit visit, TIntermUnary *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitTernary(Visit visit, TIntermTernary *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitIfElse(Visit visit, TIntermIfElse *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitSwitch(Visit visit, TIntermSwitch *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitCase(Visit visit, TIntermCase *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+void ValidateAST::visitFunctionPrototype(TIntermFunctionPrototype *node)
+{
+ visitNode(PreVisit, node);
+}
+
+bool ValidateAST::visitFunctionDefinition(Visit visit, TIntermFunctionDefinition *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitAggregate(Visit visit, TIntermAggregate *node)
+{
+ visitNode(visit, node);
+ expectNonNullChildren(visit, node, 0);
+ return true;
+}
+
+bool ValidateAST::visitBlock(Visit visit, TIntermBlock *node)
+{
+ visitNode(visit, node);
+ expectNonNullChildren(visit, node, 0);
+ return true;
+}
+
+bool ValidateAST::visitInvariantDeclaration(Visit visit, TIntermInvariantDeclaration *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitDeclaration(Visit visit, TIntermDeclaration *node)
+{
+ visitNode(visit, node);
+ expectNonNullChildren(visit, node, 0);
+ return true;
+}
+
+bool ValidateAST::visitLoop(Visit visit, TIntermLoop *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+bool ValidateAST::visitBranch(Visit visit, TIntermBranch *node)
+{
+ visitNode(visit, node);
+ return true;
+}
+
+void ValidateAST::visitPreprocessorDirective(TIntermPreprocessorDirective *node)
+{
+ visitNode(PreVisit, node);
+}
+
+bool ValidateAST::validateInternal()
+{
+ return !mSingleParentFailed && !mNullNodesFailed;
+}
+
+} // anonymous namespace
+
+bool ValidateAST(TIntermNode *root, TDiagnostics *diagnostics, const ValidateASTOptions &options)
+{
+ return ValidateAST::validate(root, diagnostics, options);
+}
+
+} // namespace sh
diff --git a/src/compiler/translator/ValidateAST.h b/src/compiler/translator/ValidateAST.h
new file mode 100644
index 0000000..7314f6e
--- /dev/null
+++ b/src/compiler/translator/ValidateAST.h
@@ -0,0 +1,58 @@
+//
+// Copyright 2019 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_VALIDATEAST_H_
+#define COMPILER_TRANSLATOR_VALIDATEAST_H_
+
+#include "compiler/translator/BaseTypes.h"
+#include "compiler/translator/Common.h"
+
+namespace sh
+{
+class TDiagnostics;
+class TIntermNode;
+
+// The following options (stored in Compiler) tell the validator what to validate. Some validations
+// are conditional to certain passes.
+struct ValidateASTOptions
+{
+ // TODO: add support for the flags marked with TODO. http://anglebug.com/2733
+
+ // Check that every node always has only one parent,
+ bool validateSingleParent = true;
+ // Check that all EOpCallFunctionInAST have their corresponding function definitions in the AST,
+ // with matching symbol ids. There should also be at least a prototype declaration before the
+ // function is called.
+ bool validateFunctionCall = true; // TODO
+ // Check that there are no null nodes where they are not allowed, for example as children of
+ // TIntermDeclaration or TIntermBlock.
+ bool validateNullNodes = true;
+ // Check that symbols that reference variables have consistent qualifiers and symbol ids with
+ // the variable declaration. For example, references to function out parameters should be
+ // EvqOut.
+ bool validateQualifiers = true; // TODO
+ // Check that variable declarations that can't have initializers don't have initializers
+ // (varyings, uniforms for example).
+ bool validateInitializers = true; // TODO
+ // Check that there is only one TFunction with each function name referenced in the nodes (no
+ // two TFunctions with the same name, taking internal/non-internal namespaces into account).
+ bool validateUniqueFunctions = true; // TODO
+ // Check that references to user-defined structs are matched with the corresponding struct
+ // declaration.
+ bool validateStructUsage = true; // TODO
+ // Check that expression nodes have the correct type considering their operand(s).
+ bool validateExpressionTypes = true; // TODO
+ // If SeparateDeclarations has been run, check for the absence of multi declarations as well.
+ bool validateMultiDeclarations = false; // TODO
+};
+
+// Check for errors and output error messages on the context.
+// Returns true if there are no errors.
+bool ValidateAST(TIntermNode *root, TDiagnostics *diagnostics, const ValidateASTOptions &options);
+
+} // namespace sh
+
+#endif // COMPILER_TRANSLATOR_VALIDATESWITCH_H_