Clean up temporary variable usage in ScalarizeVecAndMatConstructorArgs
Use common helper functions instead of manually creating temporary
variable nodes. Also clean up the interface provided by the traverser.
BUG=angleproject:1597
Test=WebGL conformance tests
Change-Id: Ifd8d3815ff9e75e1a2040d65db9d4b3d6a9a9273
Reviewed-on: https://chromium-review.googlesource.com/403950
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 6c8a9d6..c1be552 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -438,9 +438,8 @@
if (success && (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS))
{
- ScalarizeVecAndMatConstructorArgs scalarizer(
- shaderType, fragmentPrecisionHigh);
- root->traverse(&scalarizer);
+ ScalarizeVecAndMatConstructorArgs(root, shaderType, fragmentPrecisionHigh,
+ &mTemporaryIndex);
}
if (success && (compileOptions & SH_REGENERATE_STRUCT_NAMES))
diff --git a/src/compiler/translator/IntermTraverse.cpp b/src/compiler/translator/IntermTraverse.cpp
index 1e623d1..c6a199e 100644
--- a/src/compiler/translator/IntermTraverse.cpp
+++ b/src/compiler/translator/IntermTraverse.cpp
@@ -151,7 +151,11 @@
TIntermSymbol *node = new TIntermSymbol(0, symbolName, type);
node->setInternal(true);
+
+ ASSERT(qualifier == EvqTemporary || qualifier == EvqConst || qualifier == EvqGlobal);
node->getTypePointer()->setQualifier(qualifier);
+ // TODO(oetuaho): Might be useful to sanitize layout qualifier etc. on the type of the created
+ // symbol. This might need to be done in other places as well.
return node;
}
diff --git a/src/compiler/translator/ScalarizeVecAndMatConstructorArgs.cpp b/src/compiler/translator/ScalarizeVecAndMatConstructorArgs.cpp
index f12afa4..b2d33fb 100644
--- a/src/compiler/translator/ScalarizeVecAndMatConstructorArgs.cpp
+++ b/src/compiler/translator/ScalarizeVecAndMatConstructorArgs.cpp
@@ -3,6 +3,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
+// Scalarize vector and matrix constructor args, so that vectors built from components don't have
+// matrix arguments, and matrices built from components don't have vector arguments. This avoids
+// driver bugs around vector and matrix constructors.
+//
#include "common/debug.h"
#include "compiler/translator/ScalarizeVecAndMatConstructorArgs.h"
@@ -11,6 +15,7 @@
#include "angle_gl.h"
#include "common/angleutils.h"
+#include "compiler/translator/IntermNode.h"
namespace
{
@@ -52,9 +57,44 @@
TIntermTyped::CreateIndexNode(rowIndex));
}
-} // namespace anonymous
+class ScalarizeArgsTraverser : public TIntermTraverser
+{
+ public:
+ ScalarizeArgsTraverser(sh::GLenum shaderType,
+ bool fragmentPrecisionHigh,
+ unsigned int *temporaryIndex)
+ : TIntermTraverser(true, false, false),
+ mShaderType(shaderType),
+ mFragmentPrecisionHigh(fragmentPrecisionHigh)
+ {
+ useTemporaryIndex(temporaryIndex);
+ }
-bool ScalarizeVecAndMatConstructorArgs::visitAggregate(Visit visit, TIntermAggregate *node)
+ protected:
+ bool visitAggregate(Visit visit, TIntermAggregate *node) override;
+ bool visitBlock(Visit visit, TIntermBlock *node) override;
+
+ private:
+ void scalarizeArgs(TIntermAggregate *aggregate, bool scalarizeVector, bool scalarizeMatrix);
+
+ // If we have the following code:
+ // mat4 m(0);
+ // vec4 v(1, m);
+ // We will rewrite to:
+ // mat4 m(0);
+ // mat4 s0 = m;
+ // vec4 v(1, s0[0][0], s0[0][1], s0[0][2]);
+ // This function is to create nodes for "mat4 s0 = m;" and insert it to the code sequence. This
+ // way the possible side effects of the constructor argument will only be evaluated once.
+ void createTempVariable(TIntermTyped *original);
+
+ std::vector<TIntermSequence> mBlockStack;
+
+ sh::GLenum mShaderType;
+ bool mFragmentPrecisionHigh;
+};
+
+bool ScalarizeArgsTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
{
if (visit == PreVisit)
{
@@ -91,7 +131,7 @@
return true;
}
-bool ScalarizeVecAndMatConstructorArgs::visitBlock(Visit visit, TIntermBlock *node)
+bool ScalarizeArgsTraverser::visitBlock(Visit visit, TIntermBlock *node)
{
mBlockStack.push_back(TIntermSequence());
{
@@ -111,8 +151,9 @@
return false;
}
-void ScalarizeVecAndMatConstructorArgs::scalarizeArgs(
- TIntermAggregate *aggregate, bool scalarizeVector, bool scalarizeMatrix)
+void ScalarizeArgsTraverser::scalarizeArgs(TIntermAggregate *aggregate,
+ bool scalarizeVector,
+ bool scalarizeMatrix)
{
ASSERT(aggregate);
int size = 0;
@@ -163,12 +204,10 @@
ASSERT(size > 0);
TIntermTyped *node = original[ii]->getAsTyped();
ASSERT(node);
- TString varName = createTempVariable(node);
+ createTempVariable(node);
if (node->isScalar())
{
- TIntermSymbol *symbolNode =
- new TIntermSymbol(-1, varName, node->getType());
- sequence->push_back(symbolNode);
+ sequence->push_back(createTempSymbol(node->getType()));
size--;
}
else if (node->isVector())
@@ -179,8 +218,7 @@
size -= repeat;
for (int index = 0; index < repeat; ++index)
{
- TIntermSymbol *symbolNode =
- new TIntermSymbol(-1, varName, node->getType());
+ TIntermSymbol *symbolNode = createTempSymbol(node->getType());
TIntermBinary *newNode = ConstructVectorIndexBinaryNode(
symbolNode, index);
sequence->push_back(newNode);
@@ -188,8 +226,7 @@
}
else
{
- TIntermSymbol *symbolNode =
- new TIntermSymbol(-1, varName, node->getType());
+ TIntermSymbol *symbolNode = createTempSymbol(node->getType());
sequence->push_back(symbolNode);
size -= node->getNominalSize();
}
@@ -204,8 +241,7 @@
size -= repeat;
while (repeat > 0)
{
- TIntermSymbol *symbolNode =
- new TIntermSymbol(-1, varName, node->getType());
+ TIntermSymbol *symbolNode = createTempSymbol(node->getType());
TIntermBinary *newNode = ConstructMatrixIndexBinaryNode(
symbolNode, colIndex, rowIndex);
sequence->push_back(newNode);
@@ -220,8 +256,7 @@
}
else
{
- TIntermSymbol *symbolNode =
- new TIntermSymbol(-1, varName, node->getType());
+ TIntermSymbol *symbolNode = createTempSymbol(node->getType());
sequence->push_back(symbolNode);
size -= node->getCols() * node->getRows();
}
@@ -229,29 +264,13 @@
}
}
-TString ScalarizeVecAndMatConstructorArgs::createTempVariable(TIntermTyped *original)
+void ScalarizeArgsTraverser::createTempVariable(TIntermTyped *original)
{
- TString tempVarName = "_webgl_tmp_";
- if (original->isScalar())
- {
- tempVarName += "scalar_";
- }
- else if (original->isVector())
- {
- tempVarName += "vec_";
- }
- else
- {
- ASSERT(original->isMatrix());
- tempVarName += "mat_";
- }
- tempVarName += Str(mTempVarCount).c_str();
- mTempVarCount++;
-
ASSERT(original);
- TType type = original->getType();
- type.setQualifier(EvqTemporary);
+ nextTemporaryIndex();
+ TIntermDeclaration *decl = createTempInitDeclaration(original);
+ TType type = original->getType();
if (mShaderType == GL_FRAGMENT_SHADER &&
type.getBasicType() == EbtFloat &&
type.getPrecision() == EbpUndefined)
@@ -259,18 +278,24 @@
// We use the highest available precision for the temporary variable
// to avoid computing the actual precision using the rules defined
// in GLSL ES 1.0 Section 4.5.2.
- type.setPrecision(mFragmentPrecisionHigh ? EbpHigh : EbpMedium);
+ TIntermBinary *init = decl->getSequence()->at(0)->getAsBinaryNode();
+ init->getTypePointer()->setPrecision(mFragmentPrecisionHigh ? EbpHigh : EbpMedium);
+ init->getLeft()->getTypePointer()->setPrecision(mFragmentPrecisionHigh ? EbpHigh
+ : EbpMedium);
}
- TIntermSymbol *symbolNode = new TIntermSymbol(-1, tempVarName, type);
- TIntermBinary *init = new TIntermBinary(EOpInitialize, symbolNode, original);
-
- TIntermDeclaration *decl = new TIntermDeclaration();
- decl->appendDeclarator(init);
-
ASSERT(mBlockStack.size() > 0);
TIntermSequence &sequence = mBlockStack.back();
sequence.push_back(decl);
-
- return tempVarName;
}
+
+} // namespace anonymous
+
+void ScalarizeVecAndMatConstructorArgs(TIntermBlock *root,
+ sh::GLenum shaderType,
+ bool fragmentPrecisionHigh,
+ unsigned int *temporaryIndex)
+{
+ ScalarizeArgsTraverser scalarizer(shaderType, fragmentPrecisionHigh, temporaryIndex);
+ root->traverse(&scalarizer);
+}
\ No newline at end of file
diff --git a/src/compiler/translator/ScalarizeVecAndMatConstructorArgs.h b/src/compiler/translator/ScalarizeVecAndMatConstructorArgs.h
index eac6125..a168abf 100644
--- a/src/compiler/translator/ScalarizeVecAndMatConstructorArgs.h
+++ b/src/compiler/translator/ScalarizeVecAndMatConstructorArgs.h
@@ -3,47 +3,21 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
+// Scalarize vector and matrix constructor args, so that vectors built from components don't have
+// matrix arguments, and matrices built from components don't have vector arguments. This avoids
+// driver bugs around vector and matrix constructors.
+//
#ifndef COMPILER_TRANSLATOR_SCALARIZEVECANDMATCONSTRUCTORARGS_H_
#define COMPILER_TRANSLATOR_SCALARIZEVECANDMATCONSTRUCTORARGS_H_
-#include "compiler/translator/IntermNode.h"
+#include "GLSLANG/ShaderLang.h"
-class ScalarizeVecAndMatConstructorArgs : public TIntermTraverser
-{
- public:
- ScalarizeVecAndMatConstructorArgs(sh::GLenum shaderType,
- bool fragmentPrecisionHigh)
- : TIntermTraverser(true, false, false),
- mTempVarCount(0),
- mShaderType(shaderType),
- mFragmentPrecisionHigh(fragmentPrecisionHigh) {}
+class TIntermBlock;
- protected:
- bool visitAggregate(Visit visit, TIntermAggregate *node) override;
- bool visitBlock(Visit visit, TIntermBlock *node) override;
-
- private:
- void scalarizeArgs(TIntermAggregate *aggregate,
- bool scalarizeVector, bool scalarizeMatrix);
-
- // If we have the following code:
- // mat4 m(0);
- // vec4 v(1, m);
- // We will rewrite to:
- // mat4 m(0);
- // mat4 _webgl_tmp_mat_0 = m;
- // vec4 v(1, _webgl_tmp_mat_0[0][0], _webgl_tmp_mat_0[0][1], _webgl_tmp_mat_0[0][2]);
- // This function is to create nodes for "mat4 _webgl_tmp_mat_0 = m;" and insert it to
- // the code sequence.
- // Return the temporary variable name.
- TString createTempVariable(TIntermTyped *original);
-
- std::vector<TIntermSequence> mBlockStack;
- int mTempVarCount;
-
- sh::GLenum mShaderType;
- bool mFragmentPrecisionHigh;
-};
+void ScalarizeVecAndMatConstructorArgs(TIntermBlock *root,
+ sh::GLenum shaderType,
+ bool fragmentPrecisionHigh,
+ unsigned int *temporaryIndex);
#endif // COMPILER_TRANSLATOR_SCALARIZEVECANDMATCONSTRUCTORARGS_H_