SPV: Implement short circuiting of && and || when emitting SPIR-V.

SPIRV/GlslangToSpv.cpp

@@ -115,6 +115,9 @@ protected:
     void addDecoration(spv::Id id, spv::Decoration dec);
     void addMemberDecoration(spv::Id id, int member, spv::Decoration dec);
     spv::Id createSpvConstant(const glslang::TType& type, const glslang::TConstUnionArray&, int& nextConst);
+    bool isTrivialLeaf(const glslang::TIntermTyped* node);
+    bool isTrivial(const glslang::TIntermTyped* node);
+    spv::Id createShortCircuit(glslang::TOperator, glslang::TIntermTyped& left, glslang::TIntermTyped& right);
 
     spv::Function* shaderEntry;
     int sequenceDepth;
@@ -725,6 +728,21 @@ bool TGlslangToSpvTraverser::visitBinary(glslang::TVisit /* visit */, glslang::T
             builder.accessChainPushSwizzle(swizzle, convertGlslangToSpvType(node->getLeft()->getType()));
         }
         return false;
+    case glslang::EOpLogicalOr:
+    case glslang::EOpLogicalAnd:
+        {
+
+            // These may require short circuiting, but can sometimes be done as straight
+            // binary operations.  The right operand must be short circuited if it has
+            // side effects, and should probably be if it is complex.
+            if (isTrivial(node->getRight()->getAsTyped()))
+                break; // handle below as a normal binary operation
+            // otherwise, we need to do dynamic short circuiting on the right operand
+            spv::Id result = createShortCircuit(node->getOp(), *node->getLeft()->getAsTyped(), *node->getRight()->getAsTyped());
+            builder.clearAccessChain();
+            builder.setAccessChainRValue(result);
+        }
+        return false;
     default:
         break;
     }
@@ -2177,7 +2195,9 @@ spv::Id TGlslangToSpvTraverser::createBinaryOperation(glslang::TOperator op, spv
         break;
     }
 
+    // handle mapped binary operations (should be non-comparison)
     if (binOp != spv::OpNop) {
+        assert(comparison == false);
         if (builder.isMatrix(left) || builder.isMatrix(right)) {
             switch (binOp) {
             case spv::OpMatrixTimesScalar:
@@ -2215,7 +2235,7 @@ spv::Id TGlslangToSpvTraverser::createBinaryOperation(glslang::TOperator op, spv
     if (! comparison)
         return 0;
 
-    // Comparison instructions
+    // Handle comparison instructions
 
     if (reduceComparison && (builder.isVector(left) || builder.isMatrix(left) || builder.isAggregate(left))) {
         assert(op == glslang::EOpEqual || op == glslang::EOpNotEqual);
@@ -3025,6 +3045,133 @@ spv::Id TGlslangToSpvTraverser::createSpvConstant(const glslang::TType& glslangT
     return builder.makeCompositeConstant(typeId, spvConsts);
 }
 
+// Return true if the node is a constant or symbol whose reading has no
+// non-trivial observable cost or effect.
+bool TGlslangToSpvTraverser::isTrivialLeaf(const glslang::TIntermTyped* node)
+{
+    // don't know what this is
+    if (node == nullptr)
+        return false;
+
+    // a constant is safe
+    if (node->getAsConstantUnion() != nullptr)
+        return true;
+
+    // not a symbol means non-trivial
+    if (node->getAsSymbolNode() == nullptr)
+        return false;
+
+    // a symbol, depends on what's being read
+    switch (node->getType().getQualifier().storage) {
+    case glslang::EvqTemporary:
+    case glslang::EvqGlobal:
+    case glslang::EvqIn:
+    case glslang::EvqInOut:
+    case glslang::EvqConst:
+    case glslang::EvqConstReadOnly:
+    case glslang::EvqUniform:
+        return true;
+    default:
+        return false;
+    }
+} 
+
+// A node is trivial if it is a single operation with no side effects.
+// Error on the side of saying non-trivial.
+// Return true if trivial.
+bool TGlslangToSpvTraverser::isTrivial(const glslang::TIntermTyped* node)
+{
+    if (node == nullptr)
+        return false;
+
+    // symbols and constants are trivial
+    if (isTrivialLeaf(node))
+        return true;
+
+    // otherwise, it needs to be a simple operation or one or two leaf nodes
+
+    // not a simple operation
+    const glslang::TIntermBinary* binaryNode = node->getAsBinaryNode();
+    const glslang::TIntermUnary* unaryNode = node->getAsUnaryNode();
+    if (binaryNode == nullptr && unaryNode == nullptr)
+        return false;
+
+    // not on leaf nodes
+    if (binaryNode && (! isTrivialLeaf(binaryNode->getLeft()) || ! isTrivialLeaf(binaryNode->getRight())))
+        return false;
+
+    if (unaryNode && ! isTrivialLeaf(unaryNode->getOperand())) {
+        return false;
+    }
+
+    switch (node->getAsOperator()->getOp()) {
+    case glslang::EOpLogicalNot:
+    case glslang::EOpConvIntToBool:
+    case glslang::EOpConvUintToBool:
+    case glslang::EOpConvFloatToBool:
+    case glslang::EOpConvDoubleToBool:
+    case glslang::EOpEqual:
+    case glslang::EOpNotEqual:
+    case glslang::EOpLessThan:
+    case glslang::EOpGreaterThan:
+    case glslang::EOpLessThanEqual:
+    case glslang::EOpGreaterThanEqual:
+    case glslang::EOpIndexDirect:
+    case glslang::EOpIndexDirectStruct:
+    case glslang::EOpLogicalXor:
+    case glslang::EOpAny:
+    case glslang::EOpAll:
+        return true;
+    default:
+        return false;
+    }
+}
+
+// Emit short-circuiting code, where 'right' is never evaluated unless
+// the left side is true (for &&) or false (for ||).
+spv::Id TGlslangToSpvTraverser::createShortCircuit(glslang::TOperator op, glslang::TIntermTyped& left, glslang::TIntermTyped& right)
+{
+    spv::Id boolTypeId = builder.makeBoolType();
+
+    // emit left operand
+    builder.clearAccessChain();
+    left.traverse(this);
+    spv::Id leftId = builder.accessChainLoad(boolTypeId);
+
+    // Operands to accumulate OpPhi operands
+    std::vector<spv::Id> phiOperands;
+    // accumulate left operand's phi information
+    phiOperands.push_back(leftId);
+    phiOperands.push_back(builder.getBuildPoint()->getId());
+
+    // Make the two kinds of operation symmetric with a "!"
+    //   || => emit "if (! left) result = right"
+    //   && => emit "if (  left) result = right"
+    //
+    // TODO: this runtime "not" for || could be avoided by adding functionality
+    // to 'builder' to have an "else" without an "then"
+    if (op == glslang::EOpLogicalOr)
+        leftId = builder.createUnaryOp(spv::OpLogicalNot, boolTypeId, leftId);
+
+    // make an "if" based on the left value
+    spv::Builder::If ifBuilder(leftId, builder);
+
+    // emit right operand as the "then" part of the "if"
+    builder.clearAccessChain();
+    right.traverse(this);
+    spv::Id rightId = builder.accessChainLoad(boolTypeId);
+
+    // accumulate left operand's phi information
+    phiOperands.push_back(rightId);
+    phiOperands.push_back(builder.getBuildPoint()->getId());
+
+    // finish the "if"
+    ifBuilder.makeEndIf();
+
+    // phi together the two results
+    return builder.createOp(spv::OpPhi, boolTypeId, phiOperands);
+}
+
 };  // end anonymous namespace
 
 namespace glslang {

Test/baseResults/spv.300layoutp.vert.out

@@ -5,7 +5,7 @@ Linked vertex stage:
 
 // Module Version 99
 // Generated by (magic number): 51a00bb
-// Id's are bound by 111
+// Id's are bound by 114
 
                               Source ESSL 300
                               Capability Shader
@@ -40,8 +40,8 @@ Linked vertex stage:
                               MemberName 77(S) 0  "c"
                               MemberName 77(S) 1  "f"
                               Name 79  "s"
-                              Name 109  "gl_VertexID"
-                              Name 110  "gl_InstanceID"
+                              Name 112  "gl_VertexID"
+                              Name 113  "gl_InstanceID"
                               Decorate 9(pos) Smooth
                               Decorate 11(p) Location 3
                               MemberDecorate 17(Transform) 0 RowMajor
@@ -67,10 +67,10 @@ Linked vertex stage:
                               Decorate 53(c) Location 7
                               Decorate 61(iout) Flat
                               Decorate 73(aiv2) Location 9
-                              Decorate 109(gl_VertexID) BuiltIn VertexId
-                              Decorate 109(gl_VertexID) NoStaticUse
-                              Decorate 110(gl_InstanceID) BuiltIn InstanceId
-                              Decorate 110(gl_InstanceID) NoStaticUse
+                              Decorate 112(gl_VertexID) BuiltIn VertexId
+                              Decorate 112(gl_VertexID) NoStaticUse
+                              Decorate 113(gl_InstanceID) BuiltIn InstanceId
+                              Decorate 113(gl_InstanceID) NoStaticUse
                2:             TypeVoid
                3:             TypeFunction 2
                6:             TypeFloat 32
@@ -124,12 +124,12 @@ Linked vertex stage:
               89:    6(float) Constant 1065353216
               90:   14(fvec3) ConstantComposite 89 89 89
               91:             TypeVector 42(bool) 3
-              94:             TypePointer Uniform 30(ivec3)
-              97:     29(int) Constant 5
-              98:   30(ivec3) ConstantComposite 97 97 97
-             108:             TypePointer Input 16(int)
-109(gl_VertexID):    108(ptr) Variable Input
-110(gl_InstanceID):    108(ptr) Variable Input
+              97:             TypePointer Uniform 30(ivec3)
+             100:     29(int) Constant 5
+             101:   30(ivec3) ConstantComposite 100 100 100
+             111:             TypePointer Input 16(int)
+112(gl_VertexID):    111(ptr) Variable Input
+113(gl_InstanceID):    111(ptr) Variable Input
          4(main):           2 Function None 3
                5:             Label
               12:    7(fvec4) Load 11(p)
@@ -174,20 +174,26 @@ Linked vertex stage:
               88:   14(fvec3) Load 87
               92:   91(bvec3) FOrdNotEqual 88 90
               93:    42(bool) Any 92
-              95:     94(ptr) AccessChain 35 62 55
-              96:   30(ivec3) Load 95
-              99:   91(bvec3) INotEqual 96 98
-             100:    42(bool) Any 99
-             101:    42(bool) LogicalOr 93 100
-                              SelectionMerge 103 None
-                              BranchConditional 101 102 103
-             102:               Label
-             104:     50(ptr)   AccessChain 79(s) 20
-             105:   14(fvec3)   Load 104
-             106:   14(fvec3)   CompositeConstruct 89 89 89
-             107:   14(fvec3)   FAdd 105 106
-                                Store 104 107
-                                Branch 103
-             103:             Label
+              94:    42(bool) LogicalNot 93
+                              SelectionMerge 96 None
+                              BranchConditional 94 95 96
+              95:               Label
+              98:     97(ptr)   AccessChain 35 62 55
+              99:   30(ivec3)   Load 98
+             102:   91(bvec3)   INotEqual 99 101
+             103:    42(bool)   Any 102
+                                Branch 96
+              96:             Label
+             104:    42(bool) Phi 93 5 103 95
+                              SelectionMerge 106 None
+                              BranchConditional 104 105 106
+             105:               Label
+             107:     50(ptr)   AccessChain 79(s) 20
+             108:   14(fvec3)   Load 107
+             109:   14(fvec3)   CompositeConstruct 89 89 89
+             110:   14(fvec3)   FAdd 108 109
+                                Store 107 110
+                                Branch 106
+             106:             Label
                               Return
                               FunctionEnd

Test/spv.shortCircuit.frag

+#version 400
+
+uniform ivec4 uiv4;
+uniform vec4 uv4;
+uniform bool ub;
+uniform bool uba;
+uniform bvec4 ub41, ub42;
+uniform float uf;
+uniform int ui;
+
+out float of1;
+out vec4  of4;
+
+bool foo() { ++of1; return of1 > 10.0; }
+
+void main()
+{
+    of1 = 0.0;
+    of4 = vec4(0.0);
+
+    if (ub || ui > 2)  // not worth short circuiting
+        ++of1;
+
+    if (ub && !uba)  // not worth short circuiting
+        ++of1;
+
+    if (ub || foo())   // must short circuit
+        ++of1;
+
+    if (ub && foo())   // must short circuit
+        ++of1;
+
+    if (foo() || ub)   // not worth short circuiting
+        ++of1;
+
+    if (foo() && ub)   // not worth short circuiting
+        ++of1;
+
+    if (ub || ++of1 > 1.0)   // must short circuit
+        ++of4;
+
+    if (++of1 > 1.0 || ub)   // not worth short circuiting
+        ++of4;
+
+    if (ub || sin(uf) * 4.0 > of1)  // worth short circuiting
+        ++of1;
+
+    if (ub && sin(uf) * 4.0 > of1)  // worth short circuiting
+        ++of1;
+}

Test/test-spirv-list

@@ -82,3 +82,6 @@ spv.whileLoop.frag
 spv.atomic.comp
 spv.AofA.frag
 spv.queryL.frag
+spv.shortCircuit.frag
+# GLSL-level semantics
+vulkan.frag

glslang/Include/revision.h

@@ -2,5 +2,5 @@
 // For the version, it uses the latest git tag followed by the number of commits.
 // For the date, it uses the current date (when then script is run).
 
-#define GLSLANG_REVISION "3.0.788"
-#define GLSLANG_DATE "14-Oct-2015"
+#define GLSLANG_REVISION "3.0.789"
+#define GLSLANG_DATE "15-Oct-2015"