Disable optimizations for shaders with conditional discard in D3D9, and only use expanded

src/common/version.h

 #define MAJOR_VERSION 1
 #define MINOR_VERSION 2
 #define BUILD_VERSION 0
-#define BUILD_REVISION 2449
+#define BUILD_REVISION 2450
 
 #define STRINGIFY(x) #x
 #define MACRO_STRINGIFY(x) STRINGIFY(x)

src/compiler/Intermediate.cpp

@@ -806,7 +806,7 @@ bool TIntermSelection::replaceChildNode(
 //
 // Returns true if state is modified.
 //
-bool TIntermOperator::modifiesState() const
+bool TIntermOperator::hasSideEffects() const
 {
     switch (op) {
         case EOpPostIncrement:

src/compiler/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/OutputHLSL.cpp

@@ -12,6 +12,7 @@
 #include "compiler/InfoSink.h"
 #include "compiler/SearchSymbol.h"
 #include "compiler/UnfoldShortCircuit.h"
+#include "compiler/NodeSearch.h"
 
 #include <algorithm>
 #include <cfloat>
@@ -72,6 +73,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;
 
@@ -196,6 +198,11 @@ void OutputHLSL::header()
         attributes += "static " + typeString(type) + " " + decorate(name) + arrayString(type) + " = " + initializer(type) + ";\n";
     }
 
+    if (mUsesDiscardRewriting)
+    {
+        out << "#define ANGLE_USES_DISCARD_REWRITING" << "\n";
+    }
+
     if (shaderType == SH_FRAGMENT_SHADER)
     {
         TExtensionBehavior::const_iterator iter = mContext.extensionBehavior().find("GL_EXT_draw_buffers");
@@ -1299,15 +1306,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:
-        out << "s" << mUnfoldShortCircuit->getNextTemporaryIndex();
-        return false;
+        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:
-        out << "s" << mUnfoldShortCircuit->getNextTemporaryIndex();
-        return false;
+        if (node->getRight()->hasSideEffects())
+        {
+            out << "s" << mUnfoldShortCircuit->getNextTemporaryIndex();
+            return false;
+        }
+        else
+        {
+           outputTriplet(visit, "(", " && ", ")");
+           return true;
+        }
       default: UNREACHABLE();
     }
 
@@ -1944,7 +1967,7 @@ bool OutputHLSL::visitSelection(Visit visit, TIntermSelection *node)
     {
         mUnfoldShortCircuit->traverse(node->getCondition());
 
-        out << "if(";
+        out << "if (";
 
         node->getCondition()->traverse(this);
 
@@ -1953,9 +1976,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);
@@ -1973,6 +2001,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;
         }
     }
 
@@ -2070,7 +2107,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)
         {
@@ -2293,7 +2332,7 @@ bool OutputHLSL::handleExcessiveLoop(TIntermLoop *node)
 
                 if (!firstLoopFragment)
                 {
-                    out << "if(!Break";
+                    out << "if (!Break";
                     index->traverse(this);
                     out << ") {\n";
                 }

src/compiler/OutputHLSL.h

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

src/compiler/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/ValidateLimitations.cpp

@@ -457,7 +457,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/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/intermediate.h

@@ -252,6 +252,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; }
@@ -354,6 +356,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; }
 
@@ -376,6 +380,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(int index) const { return unionArrayPointer ? unionArrayPointer[index].getIConst() : 0; }
@@ -400,7 +406,7 @@ public:
     TOperator getOp() const { return op; }
     void setOp(TOperator o) { op = o; }
 
-    bool modifiesState() const;
+    virtual bool hasSideEffects() const;
     bool isConstructor() const;
 
 protected:
@@ -421,6 +427,8 @@ public:
     virtual bool replaceChildNode(
         TIntermNode *original, TIntermNode *replacement);
 
+    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 bool replaceChildNode(
         TIntermNode *original, TIntermNode *replacement);
 
+    virtual bool hasSideEffects() const { return (TIntermOperator::hasSideEffects() || operand->hasSideEffects()); }
+
     void setOperand(TIntermTyped* o) { operand = o; }
     TIntermTyped* getOperand() { return operand; }    
     bool promote(TInfoSink&);
@@ -483,6 +493,9 @@ public:
     virtual bool replaceChildNode(
         TIntermNode *original, TIntermNode *replacement);
 
+    // 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; }
@@ -528,6 +541,9 @@ public:
     virtual bool replaceChildNode(
         TIntermNode *original, TIntermNode *replacement);
 
+    // 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/compiler/translator.vcxproj

@@ -263,6 +263,7 @@
     <ClInclude Include="localintermediate.h" />
     <ClInclude Include="MapLongVariableNames.h" />
     <ClInclude Include="MMap.h" />
+    <ClInclude Include="NodeSearch.h" />
     <ClInclude Include="osinclude.h" />
     <ClInclude Include="OutputESSL.h" />
     <ClInclude Include="OutputGLSL.h" />

src/compiler/translator.vcxproj.filters

@@ -346,6 +346,9 @@
     <ClInclude Include="ParseContext.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="NodeSearch.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <CustomBuild Include="glslang.l">

src/libGLESv2/ProgramBinary.cpp

@@ -34,6 +34,11 @@ std::string str(int i)
     return buffer;
 }
 
+static rx::D3DWorkaroundType DiscardWorkaround(bool usesDiscard)
+{
+    return (usesDiscard ? rx::ANGLE_D3D_WORKAROUND_SM3_OPTIMIZER : rx::ANGLE_D3D_WORKAROUND_NONE);
+}
+
 UniformLocation::UniformLocation(const std::string &name, unsigned int element, unsigned int index) 
     : name(name), element(element), index(index)
 {
@@ -1962,13 +1967,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

@@ -307,6 +307,7 @@ void Shader::parseVaryings()
         mUsesPointCoord = strstr(mHlsl, "GL_USES_POINT_COORD") != NULL;
         mUsesDepthRange = strstr(mHlsl, "GL_USES_DEPTH_RANGE") != NULL;
         mUsesFragDepth = strstr(mHlsl, "GL_USES_FRAG_DEPTH") != NULL;
+        mUsesDiscardRewriting = strstr(mHlsl, "ANGLE_USES_DISCARD_REWRITING") != NULL;
     }
 }
 
@@ -340,6 +341,7 @@ void Shader::uncompile()
     mUsesPointCoord = false;
     mUsesDepthRange = false;
     mUsesFragDepth = false;
+    mUsesDiscardRewriting = false;
 
     mActiveUniforms.clear();
 }

src/libGLESv2/Shader.h

@@ -107,6 +107,7 @@ class Shader
     bool mUsesPointCoord;
     bool mUsesDepthRange;
     bool mUsesFragDepth;
+    bool mUsesDiscardRewriting;
 
     static void *mFragmentCompiler;
     static void *mVertexCompiler;

src/libGLESv2/renderer/Renderer.h

@@ -93,6 +93,12 @@ enum ShaderType
     SHADER_GEOMETRY
 };
 
+enum D3DWorkaroundType
+{
+    ANGLE_D3D_WORKAROUND_NONE,
+    ANGLE_D3D_WORKAROUND_SM3_OPTIMIZER
+};
+
 class Renderer
 {
   public:
@@ -207,7 +213,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/Renderer11.cpp

@@ -2830,7 +2830,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/Renderer11.h

@@ -155,7 +155,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/Renderer9.cpp

@@ -3129,7 +3129,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;
 
@@ -3146,7 +3146,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/Renderer9.h

@@ -170,7 +170,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();