D3D: Work around HLSL integer pow folding bug.

include/GLSLANG/ShaderLang.h

@@ -48,7 +48,7 @@ typedef unsigned int GLenum;
 
 // Version number for shader translation API.
 // It is incremented every time the API changes.
-#define ANGLE_SH_VERSION 150
+#define ANGLE_SH_VERSION 151
 
 typedef enum {
     SH_GLES2_SPEC,
@@ -211,6 +211,11 @@ typedef enum {
     // This flag works around bugs in Mac drivers related to do-while by
     // transforming them into an other construct.
     SH_REWRITE_DO_WHILE_LOOPS = 0x400000,
+
+    // This flag works around a bug in the HLSL compiler optimizer that folds certain
+    // constant pow expressions incorrectly. Only applies to the HLSL back-end. It works
+    // by expanding the integer pow expressions into a series of multiplies.
+    SH_EXPAND_SELECT_HLSL_INTEGER_POW_EXPRESSIONS = 0x800000,
 } ShCompileOptions;
 
 // Defines alternate strategies for implementing array index clamping.

src/compiler.gypi

@@ -42,6 +42,8 @@
             'compiler/translator/DirectiveHandler.h',
             'compiler/translator/EmulatePrecision.cpp',
             'compiler/translator/EmulatePrecision.h',
+            'compiler/translator/ExpandIntegerPowExpressions.cpp',
+            'compiler/translator/ExpandIntegerPowExpressions.h',
             'compiler/translator/ExtensionBehavior.h',
             'compiler/translator/FlagStd140Structs.cpp',
             'compiler/translator/FlagStd140Structs.h',

src/compiler/translator/ArrayReturnValueToOutParameter.cpp

@@ -103,7 +103,7 @@ bool ArrayReturnValueToOutParameterTraverser::visitAggregate(Visit visit, TInter
                 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 @@ bool ArrayReturnValueToOutParameterTraverser::visitAggregate(Visit visit, TInter
                 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 @@ bool ArrayReturnValueToOutParameterTraverser::visitBinary(Visit visit, TIntermBi
         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;

src/compiler/translator/ExpandIntegerPowExpressions.cpp

+//
+// 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

src/compiler/translator/ExpandIntegerPowExpressions.h

+//
+// 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_

src/compiler/translator/IntermNode.cpp

@@ -2679,3 +2679,15 @@ void TIntermTraverser::updateTree()
     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));
+}

src/compiler/translator/IntermNode.h

@@ -763,18 +763,6 @@ class TIntermTraverser : angle::NonCopyable
         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 @@ class TIntermTraverser : angle::NonCopyable
         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 @@ class TIntermTraverser : angle::NonCopyable
     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 @@ class TIntermTraverser : angle::NonCopyable
     // 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
     {

src/compiler/translator/IntermTraverse.cpp

@@ -94,6 +94,13 @@ void TIntermTraverser::insertStatementsInParentBlock(const TIntermSequence &inse
     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.

src/compiler/translator/RemoveDynamicIndexing.cpp

@@ -399,9 +399,7 @@ bool RemoveDynamicIndexingTraverser::visitBinary(Visit visit, TIntermBinary *nod
 
             // 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

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 @@ void TranslatorHLSL::translate(TIntermNode *root, int compileOptions)
                                                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);
 

src/libANGLE/renderer/d3d/ShaderD3D.cpp

@@ -40,9 +40,15 @@ const char *GetShaderTypeString(GLenum type)
 namespace rx
 {
 
-ShaderD3D::ShaderD3D(const gl::ShaderState &data) : ShaderImpl(data)
+ShaderD3D::ShaderD3D(const gl::ShaderState &data, const WorkaroundsD3D &workarounds)
+    : ShaderImpl(data), mAdditionalOptions(0)
 {
     uncompile();
+
+    if (workarounds.expandIntegerPowExpressions)
+    {
+        mAdditionalOptions |= SH_EXPAND_SELECT_HLSL_INTEGER_POW_EXPRESSIONS;
+    }
 }
 
 ShaderD3D::~ShaderD3D()
@@ -135,6 +141,8 @@ int ShaderD3D::prepareSourceAndReturnOptions(std::stringstream *shaderSourceStre
     }
 #endif
 
+    additionalOptions |= mAdditionalOptions;
+
     *shaderSourceStream << source;
     return additionalOptions;
 }

src/libANGLE/renderer/d3d/ShaderD3D.h

@@ -19,11 +19,12 @@ class DynamicHLSL;
 class RendererD3D;
 struct D3DCompilerWorkarounds;
 struct D3DUniform;
+struct WorkaroundsD3D;
 
 class ShaderD3D : public ShaderImpl
 {
   public:
-    ShaderD3D(const gl::ShaderState &data);
+    ShaderD3D(const gl::ShaderState &data, const WorkaroundsD3D &workarounds);
     virtual ~ShaderD3D();
 
     // ShaderImpl implementation
@@ -76,7 +77,8 @@ class ShaderD3D : public ShaderImpl
     mutable std::string mDebugInfo;
     std::map<std::string, unsigned int> mUniformRegisterMap;
     std::map<std::string, unsigned int> mInterfaceBlockRegisterMap;
+    int mAdditionalOptions;
 };
-}
+}  // namespace rx
 
 #endif  // LIBANGLE_RENDERER_D3D_SHADERD3D_H_

src/libANGLE/renderer/d3d/WorkaroundsD3D.h

@@ -52,6 +52,11 @@ struct WorkaroundsD3D
     // from a staging texture to a depth/stencil texture triggers a timeout/TDR. The workaround
     // is to use UpdateSubresource to trigger an extra copy.
     bool depthStencilBlitExtraCopy = false;
+
+    // The HLSL optimizer has a bug with optimizing "pow" in certain integer-valued expressions.
+    // We can work around this by expanding the pow into a series of multiplies if we're running
+    // under the affected compiler.
+    bool expandIntegerPowExpressions = false;
 };
 
 }  // namespace rx

src/libANGLE/renderer/d3d/d3d11/Context11.cpp

@@ -55,7 +55,7 @@ CompilerImpl *Context11::createCompiler()
 
 ShaderImpl *Context11::createShader(const gl::ShaderState &data)
 {
-    return new ShaderD3D(data);
+    return new ShaderD3D(data, mRenderer->getWorkarounds());
 }
 
 ProgramImpl *Context11::createProgram(const gl::ProgramState &data)

src/libANGLE/renderer/d3d/d3d11/renderer11_utils.cpp

@@ -1520,6 +1520,9 @@ WorkaroundsD3D GenerateWorkarounds(const Renderer11DeviceCaps &deviceCaps,
     // TODO(jmadill): Narrow problematic driver range.
     workarounds.depthStencilBlitExtraCopy = (adapterDesc.VendorId == VENDOR_ID_NVIDIA);
 
+    // TODO(jmadill): Disable workaround when we have a fixed compiler DLL.
+    workarounds.expandIntegerPowExpressions = true;
+
     return workarounds;
 }
 

src/libANGLE/renderer/d3d/d3d9/Context9.cpp

@@ -48,7 +48,7 @@ CompilerImpl *Context9::createCompiler()
 
 ShaderImpl *Context9::createShader(const gl::ShaderState &data)
 {
-    return new ShaderD3D(data);
+    return new ShaderD3D(data, mRenderer->getWorkarounds());
 }
 
 ProgramImpl *Context9::createProgram(const gl::ProgramState &data)

src/libANGLE/renderer/d3d/d3d9/renderer9_utils.cpp

@@ -646,9 +646,13 @@ WorkaroundsD3D GenerateWorkarounds()
     workarounds.mrtPerfWorkaround = true;
     workarounds.setDataFasterThanImageUpload = false;
     workarounds.useInstancedPointSpriteEmulation = false;
+
+    // TODO(jmadill): Disable workaround when we have a fixed compiler DLL.
+    workarounds.expandIntegerPowExpressions = true;
+
     return workarounds;
 }
 
-}
+}  // namespace d3d9
 
-}
+}  // namespace rx

src/tests/gl_tests/GLSLTest.cpp

@@ -8,6 +8,7 @@
 
 #include "libANGLE/Context.h"
 #include "libANGLE/Program.h"
+#include "test_utils/gl_raii.h"
 
 using namespace angle;
 
@@ -1607,9 +1608,8 @@ TEST_P(GLSLTest, DepthRangeUniforms)
 }
 
 // Covers the WebGL test 'glsl/bugs/pow-of-small-constant-in-user-defined-function'
-// See https://code.google.com/p/angleproject/issues/detail?id=851
-// TODO(jmadill): ANGLE constant folding can fix this
-TEST_P(GLSLTest, DISABLED_PowOfSmallConstant)
+// See http://anglebug.com/851
+TEST_P(GLSLTest, PowOfSmallConstant)
 {
     const std::string &fragmentShaderSource = SHADER_SOURCE
     (
@@ -1639,11 +1639,10 @@ TEST_P(GLSLTest, DISABLED_PowOfSmallConstant)
         }
     );
 
-    GLuint program = CompileProgram(mSimpleVSSource, fragmentShaderSource);
-    EXPECT_NE(0u, program);
+    ANGLE_GL_PROGRAM(program, mSimpleVSSource, fragmentShaderSource);
 
-    drawQuad(program, "inputAttribute", 0.5f);
-    EXPECT_PIXEL_EQ(0, 0, 0, 255, 0, 255);
+    drawQuad(program.get(), "inputAttribute", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
     EXPECT_GL_NO_ERROR();
 }