D3D: Work around HLSL integer pow folding bug.
BUG=angleproject:851
Change-Id: I68a47b8343a29e42c0a69ca3f2a6cb5054d03782
Reviewed-on: https://chromium-review.googlesource.com/362775
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/ArrayReturnValueToOutParameter.cpp b/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
index 510ade8..3c1a71e 100644
--- a/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
+++ b/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
@@ -103,7 +103,7 @@
replacementParams->getSequence()->push_back(CreateReturnValueOutSymbol(node->getType()));
replacementParams->setLine(params->getLine());
- mReplacements.push_back(NodeUpdateEntry(node, params, replacementParams, false));
+ replaceWithParent(node, params, replacementParams);
node->setType(TType(EbtVoid));
@@ -122,7 +122,7 @@
replacement->setLine(node->getLine());
replacement->setType(TType(EbtVoid));
- mReplacements.push_back(NodeUpdateEntry(getParentNode(), node, replacement, false));
+ replace(node, replacement);
}
else if (node->getOp() == EOpFunctionCall)
{
@@ -192,7 +192,7 @@
if (rightAgg != nullptr && rightAgg->getOp() == EOpFunctionCall && rightAgg->isUserDefined())
{
TIntermAggregate *replacementCall = CreateReplacementCall(rightAgg, node->getLeft());
- mReplacements.push_back(NodeUpdateEntry(getParentNode(), node, replacementCall, false));
+ replace(node, replacementCall);
}
}
return false;
diff --git a/src/compiler/translator/ExpandIntegerPowExpressions.cpp b/src/compiler/translator/ExpandIntegerPowExpressions.cpp
new file mode 100644
index 0000000..162925a
--- /dev/null
+++ b/src/compiler/translator/ExpandIntegerPowExpressions.cpp
@@ -0,0 +1,136 @@
+//
+// Copyright (c) 2016 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.
+//
+// Implementation of the integer pow expressions HLSL bug workaround.
+// See header for more info.
+
+#include "compiler/translator/ExpandIntegerPowExpressions.h"
+
+#include <cmath>
+#include <cstdlib>
+
+#include "compiler/translator/IntermNode.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class Traverser : public TIntermTraverser
+{
+ public:
+ static void Apply(TIntermNode *root, unsigned int *tempIndex);
+
+ private:
+ Traverser();
+ bool visitAggregate(Visit visit, TIntermAggregate *node) override;
+};
+
+// static
+void Traverser::Apply(TIntermNode *root, unsigned int *tempIndex)
+{
+ Traverser traverser;
+ traverser.useTemporaryIndex(tempIndex);
+ root->traverse(&traverser);
+ traverser.updateTree();
+}
+
+Traverser::Traverser() : TIntermTraverser(true, false, false)
+{
+}
+
+bool Traverser::visitAggregate(Visit visit, TIntermAggregate *node)
+{
+ // Test 0: skip non-pow operators.
+ if (node->getOp() != EOpPow)
+ {
+ return true;
+ }
+
+ const TIntermSequence *sequence = node->getSequence();
+ ASSERT(sequence->size() == 2u);
+ const TIntermConstantUnion *constantNode = sequence->at(1)->getAsConstantUnion();
+
+ // Test 1: check for a single constant.
+ if (!constantNode || constantNode->getNominalSize() != 1)
+ {
+ return true;
+ }
+
+ const TConstantUnion *constant = constantNode->getUnionArrayPointer();
+
+ TConstantUnion asFloat;
+ asFloat.cast(EbtFloat, *constant);
+
+ float value = asFloat.getFConst();
+
+ // Test 2: value is in the problematic range.
+ if (value < -5.0f || value > 9.0f)
+ {
+ return true;
+ }
+
+ // Test 3: value is integer or pretty close to an integer.
+ float frac = std::abs(value) - std::floor(std::abs(value));
+ if (frac > 0.0001f)
+ {
+ return true;
+ }
+
+ // Test 4: skip -1, 0, and 1
+ int exponent = static_cast<int>(value);
+ int n = std::abs(exponent);
+ if (n < 2)
+ {
+ return true;
+ }
+
+ // Potential problem case detected, apply workaround.
+ nextTemporaryIndex();
+
+ TIntermTyped *lhs = sequence->at(0)->getAsTyped();
+ ASSERT(lhs);
+
+ TIntermAggregate *init = createTempInitDeclaration(lhs);
+ TIntermTyped *current = createTempSymbol(lhs->getType());
+
+ insertStatementInParentBlock(init);
+
+ // Create a chain of n-1 multiples.
+ for (int i = 1; i < n; ++i)
+ {
+ TIntermBinary *mul = new TIntermBinary(EOpMul);
+ mul->setLeft(current);
+ mul->setRight(createTempSymbol(lhs->getType()));
+ mul->setType(node->getType());
+ mul->setLine(node->getLine());
+ current = mul;
+ }
+
+ // For negative pow, compute the reciprocal of the positive pow.
+ if (exponent < 0)
+ {
+ TConstantUnion *oneVal = new TConstantUnion();
+ oneVal->setFConst(1.0f);
+ TIntermConstantUnion *oneNode = new TIntermConstantUnion(oneVal, node->getType());
+ TIntermBinary *div = new TIntermBinary(EOpDiv);
+ div->setLeft(oneNode);
+ div->setRight(current);
+ current = div;
+ }
+
+ replace(node, current);
+ return true;
+}
+
+} // anonymous namespace
+
+void ExpandIntegerPowExpressions(TIntermNode *root, unsigned int *tempIndex)
+{
+ Traverser::Apply(root, tempIndex);
+}
+
+} // namespace sh
diff --git a/src/compiler/translator/ExpandIntegerPowExpressions.h b/src/compiler/translator/ExpandIntegerPowExpressions.h
new file mode 100644
index 0000000..8bc8c96
--- /dev/null
+++ b/src/compiler/translator/ExpandIntegerPowExpressions.h
@@ -0,0 +1,28 @@
+//
+// Copyright (c) 2016 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.
+//
+// This mutating tree traversal works around a bug in the HLSL compiler optimizer with "pow" that
+// manifests under the following conditions:
+//
+// - If pow() has a literal exponent value
+// - ... and this value is integer or within 10e-6 of an integer
+// - ... and it is in {-4, -3, -2, 2, 3, 4, 5, 6, 7, 8}
+//
+// The workaround is to replace the pow with a series of multiplies.
+// See http://anglebug.com/851
+
+#ifndef COMPILER_TRANSLATOR_EXPANDINTEGERPOWEXPRESSIONS_H_
+#define COMPILER_TRANSLATOR_EXPANDINTEGERPOWEXPRESSIONS_H_
+
+class TIntermNode;
+
+namespace sh
+{
+
+void ExpandIntegerPowExpressions(TIntermNode *root, unsigned int *tempIndex);
+
+} // namespace sh
+
+#endif // COMPILER_TRANSLATOR_EXPANDINTEGERPOWEXPRESSIONS_H_
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index ccbd80f..4481729 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -2679,3 +2679,15 @@
mReplacements.clear();
mMultiReplacements.clear();
}
+
+void TIntermTraverser::replace(TIntermNode *original, TIntermNode *replacement)
+{
+ replaceWithParent(getParentNode(), original, replacement);
+}
+
+void TIntermTraverser::replaceWithParent(TIntermNode *parent,
+ TIntermNode *original,
+ TIntermNode *replacement)
+{
+ mReplacements.push_back(NodeUpdateEntry(parent, original, replacement, false));
+}
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 58c8c74..a9a59ac 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -763,18 +763,6 @@
return !mParentBlockStack.empty() && getParentNode() == mParentBlockStack.back().node;
}
- const bool preVisit;
- const bool inVisit;
- const bool postVisit;
-
- int mDepth;
- int mMaxDepth;
-
- // All the nodes from root to the current node's parent during traversing.
- TVector<TIntermNode *> mPath;
-
- bool mInGlobalScope;
-
// To replace a single node with another on the parent node
struct NodeUpdateEntry
{
@@ -828,13 +816,6 @@
TIntermSequence insertionsAfter;
};
- // During traversing, save all the changes that need to happen into
- // mReplacements/mMultiReplacements, then do them by calling updateTree().
- // Multi replacements are processed after single replacements.
- std::vector<NodeUpdateEntry> mReplacements;
- std::vector<NodeReplaceWithMultipleEntry> mMultiReplacements;
- std::vector<NodeInsertMultipleEntry> mInsertions;
-
// Helper to insert statements in the parent block (sequence) of the node currently being traversed.
// The statements will be inserted before the node being traversed once updateTree is called.
// Should only be called during PreVisit or PostVisit from sequence nodes.
@@ -847,6 +828,9 @@
void insertStatementsInParentBlock(const TIntermSequence &insertionsBefore,
const TIntermSequence &insertionsAfter);
+ // Helper to insert a single statement.
+ void insertStatementInParentBlock(TIntermNode *statement);
+
// Helper to create a temporary symbol node with the given qualifier.
TIntermSymbol *createTempSymbol(const TType &type, TQualifier qualifier);
// Helper to create a temporary symbol node.
@@ -862,6 +846,28 @@
// Increment temporary symbol index.
void nextTemporaryIndex();
+ void replace(TIntermNode *original, TIntermNode *replacement);
+ void replaceWithParent(TIntermNode *parent, TIntermNode *original, TIntermNode *replacement);
+
+ const bool preVisit;
+ const bool inVisit;
+ const bool postVisit;
+
+ int mDepth;
+ int mMaxDepth;
+
+ // All the nodes from root to the current node's parent during traversing.
+ TVector<TIntermNode *> mPath;
+
+ bool mInGlobalScope;
+
+ // During traversing, save all the changes that need to happen into
+ // mReplacements/mMultiReplacements, then do them by calling updateTree().
+ // Multi replacements are processed after single replacements.
+ std::vector<NodeUpdateEntry> mReplacements;
+ std::vector<NodeReplaceWithMultipleEntry> mMultiReplacements;
+ std::vector<NodeInsertMultipleEntry> mInsertions;
+
private:
struct ParentBlock
{
diff --git a/src/compiler/translator/IntermTraverse.cpp b/src/compiler/translator/IntermTraverse.cpp
index b785c40..889bab7 100644
--- a/src/compiler/translator/IntermTraverse.cpp
+++ b/src/compiler/translator/IntermTraverse.cpp
@@ -94,6 +94,13 @@
mInsertions.push_back(insert);
}
+void TIntermTraverser::insertStatementInParentBlock(TIntermNode *statement)
+{
+ TIntermSequence insertions;
+ insertions.push_back(statement);
+ insertStatementsInParentBlock(insertions);
+}
+
TIntermSymbol *TIntermTraverser::createTempSymbol(const TType &type, TQualifier qualifier)
{
// Each traversal uses at most one temporary variable, so the index stays the same within a single traversal.
diff --git a/src/compiler/translator/RemoveDynamicIndexing.cpp b/src/compiler/translator/RemoveDynamicIndexing.cpp
index 35f6c10..e846fcc 100644
--- a/src/compiler/translator/RemoveDynamicIndexing.cpp
+++ b/src/compiler/translator/RemoveDynamicIndexing.cpp
@@ -399,9 +399,7 @@
// Init the temp variable holding the index
TIntermAggregate *initIndex = createTempInitDeclaration(node->getRight());
- TIntermSequence insertions;
- insertions.push_back(initIndex);
- insertStatementsInParentBlock(insertions);
+ insertStatementInParentBlock(initIndex);
mUsedTreeInsertion = true;
// Replace the index with the temp variable
diff --git a/src/compiler/translator/TranslatorHLSL.cpp b/src/compiler/translator/TranslatorHLSL.cpp
index 0a45de7..65a512d 100644
--- a/src/compiler/translator/TranslatorHLSL.cpp
+++ b/src/compiler/translator/TranslatorHLSL.cpp
@@ -9,6 +9,7 @@
#include "compiler/translator/AddDefaultReturnStatements.h"
#include "compiler/translator/ArrayReturnValueToOutParameter.h"
#include "compiler/translator/EmulatePrecision.h"
+#include "compiler/translator/ExpandIntegerPowExpressions.h"
#include "compiler/translator/IntermNodePatternMatcher.h"
#include "compiler/translator/OutputHLSL.h"
#include "compiler/translator/RemoveDynamicIndexing.h"
@@ -79,6 +80,11 @@
getOutputType());
}
+ if ((compileOptions & SH_EXPAND_SELECT_HLSL_INTEGER_POW_EXPRESSIONS) != 0)
+ {
+ sh::ExpandIntegerPowExpressions(root, getTemporaryIndex());
+ }
+
sh::OutputHLSL outputHLSL(getShaderType(), getShaderVersion(), getExtensionBehavior(),
getSourcePath(), getOutputType(), numRenderTargets, getUniforms(), compileOptions);