Disable optimizations for shaders with conditional discard in D3D9, and

projects/src/translator.vcxproj

@@ -150,6 +150,7 @@
     <ClInclude Include="..\..\src\compiler\translator\VersionGLSL.h"/>
     <ClInclude Include="..\..\src\compiler\translator\HashNames.h"/>
     <ClInclude Include="..\..\src\compiler\translator\DirectiveHandler.h"/>
+    <ClInclude Include="..\..\src\compiler\translator\NodeSearch.h"/>
     <ClInclude Include="..\..\src\compiler\translator\TranslatorESSL.h"/>
     <ClInclude Include="..\..\src\compiler\translator\InitializeDll.h"/>
     <ClInclude Include="..\..\src\compiler\translator\localintermediate.h"/>

projects/src/translator.vcxproj.filters

@@ -222,6 +222,9 @@
     <ClCompile Include="..\..\src\compiler\translator\ValidateOutputs.cpp">
       <Filter>..\..\src\compiler\translator</Filter>
     </ClCompile>
+    <ClInclude Include="..\..\src\compiler\translator\NodeSearch.h">
+      <Filter>..\..\src\compiler\translator</Filter>
+    </ClInclude>
     <ClInclude Include="..\..\src\compiler\translator\TranslatorESSL.h">
       <Filter>..\..\src\compiler\translator</Filter>
     </ClInclude>

projects/src/translator_static.vcxproj

@@ -150,6 +150,7 @@
     <ClInclude Include="..\..\src\compiler\translator\VersionGLSL.h"/>
     <ClInclude Include="..\..\src\compiler\translator\HashNames.h"/>
     <ClInclude Include="..\..\src\compiler\translator\DirectiveHandler.h"/>
+    <ClInclude Include="..\..\src\compiler\translator\NodeSearch.h"/>
     <ClInclude Include="..\..\src\compiler\translator\TranslatorESSL.h"/>
     <ClInclude Include="..\..\src\compiler\translator\InitializeDll.h"/>
     <ClInclude Include="..\..\src\compiler\translator\localintermediate.h"/>

projects/src/translator_static.vcxproj.filters

@@ -222,6 +222,9 @@
     <ClCompile Include="..\..\src\compiler\translator\ValidateOutputs.cpp">
       <Filter>..\..\src\compiler\translator</Filter>
     </ClCompile>
+    <ClInclude Include="..\..\src\compiler\translator\NodeSearch.h">
+      <Filter>..\..\src\compiler\translator</Filter>
+    </ClInclude>
     <ClInclude Include="..\..\src\compiler\translator\TranslatorESSL.h">
       <Filter>..\..\src\compiler\translator</Filter>
     </ClInclude>

src/common/version.h

 #define MAJOR_VERSION 2
 #define MINOR_VERSION 0
 #define BUILD_VERSION 0
-#define BUILD_REVISION 2016
+#define BUILD_REVISION 2017
 
 #define STRINGIFY(x) #x
 #define MACRO_STRINGIFY(x) STRINGIFY(x)

src/compiler/translator/Intermediate.cpp

@@ -774,7 +774,7 @@ void TIntermediate::remove(TIntermNode* root)
 //
 // Returns true if state is modified.
 //
-bool TIntermOperator::modifiesState() const
+bool TIntermOperator::hasSideEffects() const
 {
     switch (op) {
         case EOpPostIncrement:

src/compiler/translator/NodeSearch.h

+//
+// Copyright (c) 2002-2013 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.
+//
+// NodeSearch.h: Utilities for searching translator node graphs
+//
+
+#ifndef TRANSLATOR_NODESEARCH_H_
+#define TRANSLATOR_NODESEARCH_H_
+
+namespace sh
+{
+
+template <class Parent>
+class NodeSearchTraverser : public TIntermTraverser
+{
+  public:
+    NodeSearchTraverser()
+        : mFound(false)
+    {}
+
+    bool found() const { return mFound; }
+
+    static bool search(TIntermNode *node)
+    {
+        Parent searchTraverser;
+        node->traverse(&searchTraverser);
+        return searchTraverser.found();
+    }
+
+  protected:
+    bool mFound;
+};
+
+class FindDiscard : public NodeSearchTraverser<FindDiscard>
+{
+  public:
+    virtual bool visitBranch(Visit visit, TIntermBranch *node)
+    {
+        switch (node->getFlowOp())
+        {
+          case EOpKill:
+            mFound = true;
+            break;
+
+          default: break;
+        }
+
+        return !mFound;
+    }
+};
+
+class FindSideEffectRewriting : public NodeSearchTraverser<FindSideEffectRewriting>
+{
+  public:
+    virtual bool visitBinary(Visit visit, TIntermBinary *node)
+    {
+        switch (node->getOp())
+        {
+          case EOpLogicalOr:
+          case EOpLogicalAnd:
+            if (node->getRight()->hasSideEffects())
+            {
+                mFound = true;
+            }
+            break;
+
+          default: break;
+        }
+
+        return !mFound;
+    }
+};
+
+}
+
+#endif // TRANSLATOR_NODESEARCH_H_

src/compiler/translator/OutputHLSL.cpp

@@ -15,6 +15,7 @@
 #include "compiler/translator/UnfoldShortCircuit.h"
 #include "compiler/translator/HLSLLayoutEncoder.h"
 #include "compiler/translator/FlagStd140Structs.h"
+#include "compiler/translator/NodeSearch.h"
 
 #include <algorithm>
 #include <cfloat>
@@ -121,6 +122,7 @@ OutputHLSL::OutputHLSL(TParseContext &context, const ShBuiltInResources& resourc
     mUsesAtan2_2 = false;
     mUsesAtan2_3 = false;
     mUsesAtan2_4 = false;
+    mUsesDiscardRewriting = false;
 
     mNumRenderTargets = resources.EXT_draw_buffers ? resources.MaxDrawBuffers : 1;
 
@@ -663,6 +665,11 @@ void OutputHLSL::header()
         out << *constructor;
     }
 
+    if (mUsesDiscardRewriting)
+    {
+        out << "#define ANGLE_USES_DISCARD_REWRITING" << "\n";
+    }
+
     if (mContext.shaderType == SH_FRAGMENT_SHADER)
     {
         TExtensionBehavior::const_iterator iter = mContext.extensionBehavior().find("GL_EXT_draw_buffers");
@@ -1883,15 +1890,31 @@ bool OutputHLSL::visitBinary(Visit visit, TIntermBinary *node)
       case EOpMatrixTimesVector: outputTriplet(visit, "mul(transpose(", "), ", ")"); break;
       case EOpMatrixTimesMatrix: outputTriplet(visit, "transpose(mul(transpose(", "), transpose(", ")))"); break;
       case EOpLogicalOr:
+        if (node->getRight()->hasSideEffects())
+        {
             out << "s" << mUnfoldShortCircuit->getNextTemporaryIndex();
             return false;
+        }
+        else
+        {
+           outputTriplet(visit, "(", " || ", ")");
+           return true;
+        }
       case EOpLogicalXor:
         mUsesXor = true;
         outputTriplet(visit, "xor(", ", ", ")");
         break;
       case EOpLogicalAnd:
+        if (node->getRight()->hasSideEffects())
+        {
             out << "s" << mUnfoldShortCircuit->getNextTemporaryIndex();
             return false;
+        }
+        else
+        {
+           outputTriplet(visit, "(", " && ", ")");
+           return true;
+        }
       default: UNREACHABLE();
     }
 
@@ -2483,7 +2506,7 @@ bool OutputHLSL::visitSelection(Visit visit, TIntermSelection *node)
     {
         mUnfoldShortCircuit->traverse(node->getCondition());
 
-        out << "if(";
+        out << "if (";
 
         node->getCondition()->traverse(this);
 
@@ -2492,9 +2515,14 @@ bool OutputHLSL::visitSelection(Visit visit, TIntermSelection *node)
         outputLineDirective(node->getLine().first_line);
         out << "{\n";
 
+        bool discard = false;
+
         if (node->getTrueBlock())
         {
             traverseStatements(node->getTrueBlock());
+
+            // Detect true discard
+            discard = (discard || FindDiscard::search(node->getTrueBlock()));
         }
 
         outputLineDirective(node->getLine().first_line);
@@ -2512,6 +2540,15 @@ bool OutputHLSL::visitSelection(Visit visit, TIntermSelection *node)
 
             outputLineDirective(node->getFalseBlock()->getLine().first_line);
             out << ";\n}\n";
+
+            // Detect false discard
+            discard = (discard || FindDiscard::search(node->getFalseBlock()));
+        }
+
+        // ANGLE issue 486: Detect problematic conditional discard
+        if (discard && FindSideEffectRewriting::search(node))
+        {
+            mUsesDiscardRewriting = true;
         }
     }
 
@@ -2609,7 +2646,9 @@ bool OutputHLSL::visitBranch(Visit visit, TIntermBranch *node)
 
     switch (node->getFlowOp())
     {
-      case EOpKill:     outputTriplet(visit, "discard;\n", "", "");  break;
+      case EOpKill:
+        outputTriplet(visit, "discard;\n", "", "");
+        break;
       case EOpBreak:
         if (visit == PreVisit)
         {
@@ -2832,7 +2871,7 @@ bool OutputHLSL::handleExcessiveLoop(TIntermLoop *node)
 
                 if (!firstLoopFragment)
                 {
-                    out << "if(!Break";
+                    out << "if (!Break";
                     index->traverse(this);
                     out << ") {\n";
                 }

src/compiler/translator/OutputHLSL.h

@@ -145,6 +145,7 @@ class OutputHLSL : public TIntermTraverser
     bool mUsesAtan2_2;
     bool mUsesAtan2_3;
     bool mUsesAtan2_4;
+    bool mUsesDiscardRewriting;
 
     int mNumRenderTargets;
 

src/compiler/translator/UnfoldShortCircuit.cpp

@@ -31,6 +31,14 @@ bool UnfoldShortCircuit::visitBinary(Visit visit, TIntermBinary *node)
 {
     TInfoSinkBase &out = mOutputHLSL->getBodyStream();
 
+    // If our right node doesn't have side effects, we know we don't need to unfold this
+    // expression: there will be no short-circuiting side effects to avoid
+    // (note: unfolding doesn't depend on the left node -- it will always be evaluated)
+    if (!node->getRight()->hasSideEffects())
+    {
+        return true;
+    }
+
     switch (node->getOp())
     {
       case EOpLogicalOr:
@@ -49,7 +57,7 @@ bool UnfoldShortCircuit::visitBinary(Visit visit, TIntermBinary *node)
             mTemporaryIndex = i + 1;
             node->getLeft()->traverse(mOutputHLSL);
             out << ";\n";
-            out << "if(!s" << i << ")\n"
+            out << "if (!s" << i << ")\n"
                    "{\n";
             mTemporaryIndex = i + 1;
             node->getRight()->traverse(this);
@@ -80,7 +88,7 @@ bool UnfoldShortCircuit::visitBinary(Visit visit, TIntermBinary *node)
             mTemporaryIndex = i + 1;
             node->getLeft()->traverse(mOutputHLSL);
             out << ";\n";
-            out << "if(s" << i << ")\n"
+            out << "if (s" << i << ")\n"
                    "{\n";
             mTemporaryIndex = i + 1;
             node->getRight()->traverse(this);
@@ -115,7 +123,7 @@ bool UnfoldShortCircuit::visitSelection(Visit visit, TIntermSelection *node)
 
         mTemporaryIndex = i + 1;
         node->getCondition()->traverse(this);
-        out << "if(";
+        out << "if (";
         mTemporaryIndex = i + 1;
         node->getCondition()->traverse(mOutputHLSL);
         out << ")\n"

src/compiler/translator/ValidateLimitations.cpp

@@ -461,7 +461,7 @@ bool ValidateLimitations::validateFunctionCall(TIntermAggregate* node)
 bool ValidateLimitations::validateOperation(TIntermOperator* node,
                                             TIntermNode* operand) {
     // Check if loop index is modified in the loop body.
-    if (!withinLoopBody() || !node->modifiesState())
+    if (!withinLoopBody() || !node->hasSideEffects())
         return true;
 
     const TIntermSymbol* symbol = operand->getAsSymbolNode();

src/compiler/translator/depgraph/DependencyGraphBuilder.cpp

@@ -94,7 +94,7 @@ void TDependencyGraphBuilder::visitSymbol(TIntermSymbol* intermSymbol)
 bool TDependencyGraphBuilder::visitBinary(Visit visit, TIntermBinary* intermBinary)
 {
     TOperator op = intermBinary->getOp();
-    if (op == EOpInitialize || intermBinary->modifiesState())
+    if (op == EOpInitialize || intermBinary->hasSideEffects())
         visitAssignment(intermBinary);
     else if (op == EOpLogicalAnd || op == EOpLogicalOr)
         visitLogicalOp(intermBinary);

src/compiler/translator/intermediate.h

@@ -258,6 +258,8 @@ public:
     TIntermTyped(const TType& t) : type(t)  { }
     virtual TIntermTyped* getAsTyped() { return this; }
 
+    virtual bool hasSideEffects() const = 0;
+
     void setType(const TType& t) { type = t; }
     const TType& getType() const { return type; }
     TType* getTypePointer() { return &type; }
@@ -359,6 +361,8 @@ public:
     TIntermSymbol(int i, const TString& sym, const TType& t) : 
             TIntermTyped(t), id(i)  { symbol = sym; originalSymbol = sym; } 
 
+    virtual bool hasSideEffects() const { return false; }
+
     int getId() const { return id; }
     const TString& getSymbol() const { return symbol; }
 
@@ -380,6 +384,8 @@ class TIntermConstantUnion : public TIntermTyped {
 public:
     TIntermConstantUnion(ConstantUnion *unionPointer, const TType& t) : TIntermTyped(t), unionArrayPointer(unionPointer) { }
 
+    virtual bool hasSideEffects() const { return false; }
+
     ConstantUnion* getUnionArrayPointer() const { return unionArrayPointer; }
     
     int getIConst(size_t index) const { return unionArrayPointer ? unionArrayPointer[index].getIConst() : 0; }
@@ -404,7 +410,7 @@ public:
     TOperator getOp() const { return op; }
     void setOp(TOperator o) { op = o; }
 
-    bool modifiesState() const;
+    virtual bool hasSideEffects() const;
     bool isConstructor() const;
 
 protected:
@@ -423,6 +429,8 @@ public:
     virtual TIntermBinary* getAsBinaryNode() { return this; }
     virtual void traverse(TIntermTraverser*);
 
+    virtual bool hasSideEffects() const { return (TIntermOperator::hasSideEffects() || left->hasSideEffects() || right->hasSideEffects()); }
+
     void setLeft(TIntermTyped* n) { left = n; }
     void setRight(TIntermTyped* n) { right = n; }
     TIntermTyped* getLeft() const { return left; }
@@ -451,6 +459,8 @@ public:
     virtual void traverse(TIntermTraverser*);
     virtual TIntermUnary* getAsUnaryNode() { return this; }
 
+    virtual bool hasSideEffects() const { return (TIntermOperator::hasSideEffects() || operand->hasSideEffects()); }
+
     void setOperand(TIntermTyped* o) { operand = o; }
     TIntermTyped* getOperand() { return operand; }    
     bool promote(TInfoSink&);
@@ -481,6 +491,9 @@ public:
     virtual TIntermAggregate* getAsAggregate() { return this; }
     virtual void traverse(TIntermTraverser*);
 
+    // Conservatively assume function calls and other aggregate operators have side-effects
+    virtual bool hasSideEffects() const { return true; }
+
     TIntermSequence& getSequence() { return sequence; }
 
     void setName(const TString& n) { name = n; }
@@ -524,6 +537,9 @@ public:
 
     virtual void traverse(TIntermTraverser*);
 
+    // Conservatively assume selections have side-effects
+    virtual bool hasSideEffects() const { return true; }
+
     bool usesTernaryOperator() const { return getBasicType() != EbtVoid; }
     TIntermNode* getCondition() const { return condition; }
     TIntermNode* getTrueBlock() const { return trueBlock; }

src/libGLESv2/ProgramBinary.cpp

@@ -99,6 +99,11 @@ unsigned int parseAndStripArrayIndex(std::string* name)
     return subscript;
 }
 
+static rx::D3DWorkaroundType DiscardWorkaround(bool usesDiscard)
+{
+    return (usesDiscard ? rx::ANGLE_D3D_WORKAROUND_SM3_OPTIMIZER : rx::ANGLE_D3D_WORKAROUND_NONE);
+}
+
 }
 
 VariableLocation::VariableLocation(const std::string &name, unsigned int element, unsigned int index) 
@@ -2060,13 +2065,13 @@ bool ProgramBinary::link(InfoLog &infoLog, const AttributeBindings &attributeBin
 
     if (success)
     {
-        mVertexExecutable = mRenderer->compileToExecutable(infoLog, vertexHLSL.c_str(), rx::SHADER_VERTEX);
-        mPixelExecutable = mRenderer->compileToExecutable(infoLog, pixelHLSL.c_str(), rx::SHADER_PIXEL);
+        mVertexExecutable = mRenderer->compileToExecutable(infoLog, vertexHLSL.c_str(), rx::SHADER_VERTEX, DiscardWorkaround(vertexShader->mUsesDiscardRewriting));
+        mPixelExecutable = mRenderer->compileToExecutable(infoLog, pixelHLSL.c_str(), rx::SHADER_PIXEL, DiscardWorkaround(fragmentShader->mUsesDiscardRewriting));
 
         if (usesGeometryShader())
         {
             std::string geometryHLSL = generateGeometryShaderHLSL(registers, packing, fragmentShader, vertexShader);
-            mGeometryExecutable = mRenderer->compileToExecutable(infoLog, geometryHLSL.c_str(), rx::SHADER_GEOMETRY);
+            mGeometryExecutable = mRenderer->compileToExecutable(infoLog, geometryHLSL.c_str(), rx::SHADER_GEOMETRY, rx::ANGLE_D3D_WORKAROUND_NONE);
         }
 
         if (!mVertexExecutable || !mPixelExecutable || (usesGeometryShader() && !mGeometryExecutable))

src/libGLESv2/Shader.cpp

@@ -232,6 +232,7 @@ void Shader::parseVaryings(void *compiler)
         mUsesPointCoord            = mHlsl.find("GL_USES_POINT_COORD")  != std::string::npos;
         mUsesDepthRange            = mHlsl.find("GL_USES_DEPTH_RANGE")  != std::string::npos;
         mUsesFragDepth             = mHlsl.find("GL_USES_FRAG_DEPTH")   != std::string::npos;
+        mUsesDiscardRewriting      = mHlsl.find("ANGLE_USES_DISCARD_REWRITING") != std::string::npos;
     }
 }
 
@@ -263,6 +264,7 @@ void Shader::uncompile()
     mUsesDepthRange = false;
     mUsesFragDepth = false;
     mShaderVersion = 100;
+    mUsesDiscardRewriting = false;
 
     mActiveUniforms.clear();
     mActiveInterfaceBlocks.clear();

src/libGLESv2/Shader.h

@@ -92,6 +92,7 @@ class Shader
     bool mUsesDepthRange;
     bool mUsesFragDepth;
     int mShaderVersion;
+    bool mUsesDiscardRewriting;
 
     static void *mFragmentCompiler;
     static void *mVertexCompiler;

src/libGLESv2/renderer/Renderer.h

@@ -96,6 +96,12 @@ enum ShaderType
     SHADER_GEOMETRY
 };
 
+enum D3DWorkaroundType
+{
+    ANGLE_D3D_WORKAROUND_NONE,
+    ANGLE_D3D_WORKAROUND_SM3_OPTIMIZER
+};
+
 class Renderer
 {
   public:
@@ -236,7 +242,7 @@ class Renderer
 
     // Shader operations
     virtual ShaderExecutable *loadExecutable(const void *function, size_t length, rx::ShaderType type) = 0;
-    virtual ShaderExecutable *compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type) = 0;
+    virtual ShaderExecutable *compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type, D3DWorkaroundType workaround) = 0;
 
     // Image operations
     virtual Image *createImage() = 0;

src/libGLESv2/renderer/d3d11/Renderer11.cpp

@@ -2758,7 +2758,7 @@ ShaderExecutable *Renderer11::loadExecutable(const void *function, size_t length
     return executable;
 }
 
-ShaderExecutable *Renderer11::compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type)
+ShaderExecutable *Renderer11::compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type, D3DWorkaroundType workaround)
 {
     const char *profile = NULL;
 

src/libGLESv2/renderer/d3d11/Renderer11.h

@@ -181,7 +181,7 @@ class Renderer11 : public Renderer
 
     // Shader operations
     virtual ShaderExecutable *loadExecutable(const void *function, size_t length, rx::ShaderType type);
-    virtual ShaderExecutable *compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type);
+    virtual ShaderExecutable *compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type, D3DWorkaroundType workaround);
 
     // Image operations
     virtual Image *createImage();

src/libGLESv2/renderer/d3d9/Renderer9.cpp

@@ -3226,7 +3226,7 @@ ShaderExecutable *Renderer9::loadExecutable(const void *function, size_t length,
     return executable;
 }
 
-ShaderExecutable *Renderer9::compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type)
+ShaderExecutable *Renderer9::compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type, D3DWorkaroundType workaround)
 {
     const char *profile = NULL;
 
@@ -3243,7 +3243,11 @@ ShaderExecutable *Renderer9::compileToExecutable(gl::InfoLog &infoLog, const cha
         return NULL;
     }
 
-    ID3DBlob *binary = (ID3DBlob*)compileToBinary(infoLog, shaderHLSL, profile, ANGLE_COMPILE_OPTIMIZATION_LEVEL, true);
+    // ANGLE issue 486:
+    // Work-around a D3D9 compiler bug that presents itself when using conditional discard, by disabling optimization
+    UINT optimizationFlags = (workaround == ANGLE_D3D_WORKAROUND_SM3_OPTIMIZER ? D3DCOMPILE_SKIP_OPTIMIZATION : ANGLE_COMPILE_OPTIMIZATION_LEVEL);
+
+    ID3DBlob *binary = (ID3DBlob*)compileToBinary(infoLog, shaderHLSL, profile, optimizationFlags, true);
     if (!binary)
     {
         return NULL;

src/libGLESv2/renderer/d3d9/Renderer9.h

@@ -197,7 +197,7 @@ class Renderer9 : public Renderer
 
     // Shader operations
     virtual ShaderExecutable *loadExecutable(const void *function, size_t length, rx::ShaderType type);
-    virtual ShaderExecutable *compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type);
+    virtual ShaderExecutable *compileToExecutable(gl::InfoLog &infoLog, const char *shaderHLSL, rx::ShaderType type, D3DWorkaroundType workaround);
 
     // Image operations
     virtual Image *createImage();