Merge pull request #135 from dekimir/topo

SPV: Code-generate SPIR-V blocks in a more readable order

SPIRV/CMakeLists.txt

@@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 2.8)
 
 set(SOURCES
     GlslangToSpv.cpp
+    InReadableOrder.cpp
     SpvBuilder.cpp
     SPVRemapper.cpp
     doc.cpp

SPIRV/InReadableOrder.cpp

@@ -0,0 +1,104 @@
+//
+//Copyright (C) 2016 Google, Inc.
+//
+//All rights reserved.
+//
+//Redistribution and use in source and binary forms, with or without
+//modification, are permitted provided that the following conditions
+//are met:
+//
+//    Redistributions of source code must retain the above copyright
+//    notice, this list of conditions and the following disclaimer.
+//
+//    Redistributions in binary form must reproduce the above
+//    copyright notice, this list of conditions and the following
+//    disclaimer in the documentation and/or other materials provided
+//    with the distribution.
+//
+//    Neither the name of 3Dlabs Inc. Ltd. nor the names of its
+//    contributors may be used to endorse or promote products derived
+//    from this software without specific prior written permission.
+//
+//THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+//"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+//LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+//FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+//COPYRIGHT HOLDERS OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+//INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+//BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+//LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+//CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+//LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+//ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+//POSSIBILITY OF SUCH DAMAGE.
+
+//
+// Author: Dejan Mircevski, Google
+//
+
+// The SPIR-V spec requires code blocks to appear in an order satisfying the
+// dominator-tree direction (ie, dominator before the dominated).  This is,
+// actually, easy to achieve: any pre-order CFG traversal algorithm will do it.
+// Because such algorithms visit a block only after traversing some path to it
+// from the root, they necessarily visit the block's idom first.
+//
+// But not every graph-traversal algorithm outputs blocks in an order that
+// appears logical to human readers.  The problem is that unrelated branches may
+// be interspersed with each other, and merge blocks may come before some of the
+// branches being merged.
+//
+// A good, human-readable order of blocks may be achieved by performing
+// depth-first search but delaying merge nodes until after all their branches
+// have been visited.  This is implemented below by the inReadableOrder()
+// function.
+
+#include "spvIR.h"
+
+#include <cassert>
+#include <unordered_map>
+
+using spv::Block;
+using spv::Id;
+
+namespace {
+// Traverses CFG in a readable order, invoking a pre-set callback on each block.
+// Use by calling visit() on the root block.
+class ReadableOrderTraverser {
+public:
+    explicit ReadableOrderTraverser(std::function<void(Block*)> callback) : callback_(callback) {}
+    // Visits the block if it hasn't been visited already and isn't currently
+    // being delayed.  Invokes callback(block), then descends into its successors.
+    // Delays merge-block processing until all the branches have been completed.
+    void visit(Block* block)
+    {
+        assert(block);
+        if (visited_[block] || delayed_[block])
+            return;
+        callback_(block);
+        visited_[block] = true;
+        Block* mergeBlock = nullptr;
+        auto mergeInst = block->getMergeInstruction();
+        if (mergeInst) {
+            Id mergeId = mergeInst->getIdOperand(0);
+            mergeBlock = block->getParent().getParent().getInstruction(mergeId)->getBlock();
+            delayed_[mergeBlock] = true;
+        }
+        for (const auto succ : block->getSuccessors())
+            visit(succ);
+        if (mergeBlock) {
+            delayed_[mergeBlock] = false;
+            visit(mergeBlock);
+        }
+    }
+
+private:
+    std::function<void(Block*)> callback_;
+    // Whether a block has already been visited or is being delayed.
+    std::unordered_map<Block *, bool> visited_, delayed_;
+};
+}
+
+void spv::inReadableOrder(Block* root, std::function<void(Block*)> callback)
+{
+    ReadableOrderTraverser(callback).visit(root);
+}

SPIRV/SpvBuilder.cpp

@@ -857,7 +857,7 @@ void Builder::leaveFunction()
     if (! block->isTerminated()) {
 
         // Whether we're in an unreachable (non-entry) block.
-        bool unreachable = function.getEntryBlock() != block && block->getNumPredecessors() == 0;
+        bool unreachable = function.getEntryBlock() != block && block->getPredecessors().empty();
 
         if (unreachable) {
             // Given that this block is at the end of a function, it must be right after an
@@ -1759,10 +1759,13 @@ void Builder::makeSwitch(Id selector, int numSegments, std::vector<int>& caseVal
     // make the switch instruction
     Instruction* switchInst = new Instruction(NoResult, NoType, OpSwitch);
     switchInst->addIdOperand(selector);
-    switchInst->addIdOperand(defaultSegment >= 0 ? segmentBlocks[defaultSegment]->getId() : mergeBlock->getId());
+    auto defaultOrMerge = (defaultSegment >= 0) ? segmentBlocks[defaultSegment] : mergeBlock;
+    switchInst->addIdOperand(defaultOrMerge->getId());
+    defaultOrMerge->addPredecessor(buildPoint);
     for (int i = 0; i < (int)caseValues.size(); ++i) {
         switchInst->addImmediateOperand(caseValues[i]);
         switchInst->addIdOperand(segmentBlocks[valueIndexToSegment[i]]->getId());
+        segmentBlocks[valueIndexToSegment[i]]->addPredecessor(buildPoint);
     }
     buildPoint->addInstruction(std::unique_ptr<Instruction>(switchInst));
 

SPIRV/spvIR.h

@@ -52,13 +52,15 @@
 
 #include "spirv.hpp"
 
-#include <vector>
+#include <cassert>
+#include <functional>
 #include <iostream>
 #include <memory>
-#include <assert.h>
+#include <vector>
 
 namespace spv {
 
+class Block;
 class Function;
 class Module;
 
@@ -75,8 +77,8 @@ const MemorySemanticsMask MemorySemanticsAllMemory = (MemorySemanticsMask)0x3FF;
 
 class Instruction {
 public:
-    Instruction(Id resultId, Id typeId, Op opCode) : resultId(resultId), typeId(typeId), opCode(opCode) { }
+    Instruction(Id resultId, Id typeId, Op opCode) : resultId(resultId), typeId(typeId), opCode(opCode), block(nullptr) { }
-    explicit Instruction(Op opCode) : resultId(NoResult), typeId(NoType), opCode(opCode) { }
+    explicit Instruction(Op opCode) : resultId(NoResult), typeId(NoType), opCode(opCode), block(nullptr) { }
     virtual ~Instruction() {}
     void addIdOperand(Id id) { operands.push_back(id); }
     void addImmediateOperand(unsigned int immediate) { operands.push_back(immediate); }
@@ -107,6 +109,8 @@ public:
             addImmediateOperand(word);
         }
     }
+    void setBlock(Block* b) { block = b; }
+    Block* getBlock() const { return block; }
     Op getOpCode() const { return opCode; }
     int getNumOperands() const { return (int)operands.size(); }
     Id getResultId() const { return resultId; }
@@ -145,6 +149,7 @@ protected:
     Op opCode;
     std::vector<Id> operands;
     std::string originalString;        // could be optimized away; convenience for getting string operand
+    Block* block;
 };
 
 //
@@ -162,11 +167,25 @@ public:
 
     Function& getParent() const { return parent; }
     void addInstruction(std::unique_ptr<Instruction> inst);
-    void addPredecessor(Block* pred) { predecessors.push_back(pred); }
+    void addPredecessor(Block* pred) { predecessors.push_back(pred); pred->successors.push_back(this);}
     void addLocalVariable(std::unique_ptr<Instruction> inst) { localVariables.push_back(std::move(inst)); }
-    int getNumPredecessors() const { return (int)predecessors.size(); }
+    const std::vector<Block*>& getPredecessors() const { return predecessors; }
+    const std::vector<Block*>& getSuccessors() const { return successors; }
     void setUnreachable() { unreachable = true; }
     bool isUnreachable() const { return unreachable; }
+    // Returns the block's merge instruction, if one exists (otherwise null).
+    const Instruction* getMergeInstruction() const {
+        if (instructions.size() < 2) return nullptr;
+        const Instruction* nextToLast = (instructions.cend() - 2)->get();
+        switch (nextToLast->getOpCode()) {
+            case OpSelectionMerge:
+            case OpLoopMerge:
+                return nextToLast;
+            default:
+                return nullptr;
+        }
+        return nullptr;
+    }
 
     bool isTerminated() const
     {
@@ -206,7 +225,7 @@ protected:
     friend Function;
 
     std::vector<std::unique_ptr<Instruction> > instructions;
-    std::vector<Block*> predecessors;
+    std::vector<Block*> predecessors, successors;
     std::vector<std::unique_ptr<Instruction> > localVariables;
     Function& parent;
 
@@ -216,6 +235,11 @@ protected:
     bool unreachable;
 };
 
+// Traverses the control-flow graph rooted at root in an order suited for
+// readable code generation.  Invokes callback at every node in the traversal
+// order.
+void inReadableOrder(Block* root, std::function<void(Block*)> callback);
+
 //
 // SPIR-V IR Function.
 //
@@ -252,8 +276,7 @@ public:
             parameterInstructions[p]->dump(out);
 
         // Blocks
-        for (int b = 0; b < (int)blocks.size(); ++b)
+        inReadableOrder(blocks[0], [&out](const Block* b) { b->dump(out); });
-            blocks[b]->dump(out);
         Instruction end(0, 0, OpFunctionEnd);
         end.dump(out);
     }
@@ -351,12 +374,15 @@ __inline void Function::addLocalVariable(std::unique_ptr<Instruction> inst)
 __inline Block::Block(Id id, Function& parent) : parent(parent), unreachable(false)
 {
     instructions.push_back(std::unique_ptr<Instruction>(new Instruction(id, NoType, OpLabel)));
+    instructions.back()->setBlock(this);
+    parent.getParent().mapInstruction(instructions.back().get());
 }
 
 __inline void Block::addInstruction(std::unique_ptr<Instruction> inst)
 {
-  Instruction* raw_instruction = inst.get();
+    Instruction* raw_instruction = inst.get();
     instructions.push_back(std::move(inst));
+    raw_instruction->setBlock(this);
     if (raw_instruction->getResultId())
         parent.getParent().mapInstruction(raw_instruction);
 }

Test/baseResults/spv.always-discard.frag.out

@@ -110,23 +110,4 @@ Linked fragment stage:
                                 Branch 49
               49:             Label
                               Kill
-              69:             Label
-              70:    6(float) Load 36(radius)
-              72:    46(bool) FOrdGreaterThanEqual 70 71
-                              SelectionMerge 74 None
-                              BranchConditional 72 73 74
-              73:               Label
-              75:    6(float)   Load 36(radius)
-              77:    6(float)   ExtInst 1(GLSL.std.450) 26(Pow) 75 76
-              78:    6(float)   FDiv 77 27
-              79:    6(float)   ExtInst 1(GLSL.std.450) 4(FAbs) 78
-              80:    7(fvec4)   Load 15(color)
-              81:    7(fvec4)   CompositeConstruct 79 79 79 79
-              82:    7(fvec4)   FSub 80 81
-                                Store 15(color) 82
-                                Branch 74
-              74:             Label
-              83:    7(fvec4) Load 15(color)
-                              Store 59(gl_FragColor) 83
-                              Return
                               FunctionEnd

Test/baseResults/spv.branch-return.vert.out

@@ -0,0 +1,63 @@
+spv.branch-return.vert
+
+Linked vertex stage:
+
+
+// Module Version 10000
+// Generated by (magic number): 80001
+// Id's are bound by 35
+
+                              Capability Shader
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel Logical GLSL450
+                              EntryPoint Vertex 4  "main" 8 19 34
+                              Source ESSL 300
+                              Name 4  "main"
+                              Name 8  "gl_InstanceID"
+                              Name 19  "gl_Position"
+                              Name 34  "gl_VertexID"
+                              Decorate 8(gl_InstanceID) BuiltIn InstanceId
+                              Decorate 19(gl_Position) BuiltIn Position
+                              Decorate 34(gl_VertexID) BuiltIn VertexId
+               2:             TypeVoid
+               3:             TypeFunction 2
+               6:             TypeInt 32 1
+               7:             TypePointer Input 6(int)
+8(gl_InstanceID):      7(ptr) Variable Input
+              16:             TypeFloat 32
+              17:             TypeVector 16(float) 4
+              18:             TypePointer Output 17(fvec4)
+ 19(gl_Position):     18(ptr) Variable Output
+              20:   16(float) Constant 0
+              21:   17(fvec4) ConstantComposite 20 20 20 20
+              26:   16(float) Constant 1039918957
+              27:             TypeInt 32 0
+              28:     27(int) Constant 0
+              29:             TypePointer Output 16(float)
+ 34(gl_VertexID):      7(ptr) Variable Input
+         4(main):           2 Function None 3
+               5:             Label
+               9:      6(int) Load 8(gl_InstanceID)
+                              SelectionMerge 14 None
+                              Switch 9 14 
+                                     case 0: 10
+                                     case 1: 11
+                                     case 2: 12
+                                     case 3: 13
+              10:               Label
+                                Return
+              11:               Label
+                                Store 19(gl_Position) 21
+                                Branch 14
+              12:               Label
+                                Return
+              13:               Label
+                                Return
+              14:             Label
+              30:     29(ptr) AccessChain 19(gl_Position) 28
+              31:   16(float) Load 30
+              32:   16(float) FAdd 31 26
+              33:     29(ptr) AccessChain 19(gl_Position) 28
+                              Store 33 32
+                              Return
+                                FunctionEnd

Test/baseResults/spv.do-while-continue-break.vert.out

@@ -81,9 +81,6 @@ Linked vertex stage:
               28:               Label
                                 Store 30(B) 19
                                 Branch 10
-              32:               Label
-                                Store 33(C) 26
-                                Branch 29
               29:             Label
               34:      6(int) Load 8(i)
               36:    14(bool) IEqual 34 35
@@ -92,9 +89,6 @@ Linked vertex stage:
               37:               Label
                                 Store 39(D) 40
                                 Branch 11
-              41:               Label
-                                Store 42(E) 43
-                                Branch 38
               38:             Label
                               Store 44(F) 45
                               Branch 10

Test/baseResults/spv.for-continue-break.vert.out

@@ -70,9 +70,6 @@ Linked vertex stage:
               27:      6(int)     IAdd 26 18
                                   Store 8(i) 27
                                   Branch 10
-              28:                 Label
-                                  Store 29(C) 18
-                                  Branch 24
               24:               Label
               30:      6(int)   Load 8(i)
               32:      6(int)   SMod 30 31
@@ -82,9 +79,6 @@ Linked vertex stage:
               34:                 Label
                                   Store 36(D) 18
                                   Branch 11
-              37:                 Label
-                                  Store 38(E) 18
-                                  Branch 35
               35:               Label
                                 Store 39(F) 40
               41:      6(int)   Load 8(i)

Test/baseResults/spv.merge-unreachable.frag.out

@@ -0,0 +1,47 @@
+spv.merge-unreachable.frag
+Warning, version 450 is not yet complete; most version-specific features are present, but some are missing.
+
+
+Linked fragment stage:
+
+
+// Module Version 10000
+// Generated by (magic number): 80001
+// Id's are bound by 25
+
+                              Capability Shader
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel Logical GLSL450
+                              EntryPoint Fragment 4  "main" 9
+                              ExecutionMode 4 OriginLowerLeft
+                              Source GLSL 450
+                              Name 4  "main"
+                              Name 9  "v"
+                              Decorate 9(v) Location 1
+               2:             TypeVoid
+               3:             TypeFunction 2
+               6:             TypeFloat 32
+               7:             TypeVector 6(float) 4
+               8:             TypePointer Input 7(fvec4)
+            9(v):      8(ptr) Variable Input
+              11:    6(float) Constant 1036831949
+              12:    6(float) Constant 1045220557
+              13:    6(float) Constant 1050253722
+              14:    6(float) Constant 1053609165
+              15:    7(fvec4) ConstantComposite 11 12 13 14
+              16:             TypeBool
+              17:             TypeVector 16(bool) 4
+         4(main):           2 Function None 3
+               5:             Label
+              10:    7(fvec4) Load 9(v)
+              18:   17(bvec4) FOrdEqual 10 15
+              19:    16(bool) All 18
+                              SelectionMerge 21 None
+                              BranchConditional 19 20 23
+              20:               Label
+                                Kill
+              23:               Label
+                                Return
+              21:             Label
+                              Return
+                              FunctionEnd

Test/baseResults/spv.switch.frag.out

@@ -106,6 +106,11 @@ Linked fragment stage:
                               Switch 65 68 
                                      case 1: 66
                                      case 2: 67
+              68:               Label
+              80:    6(float)   Load 73(x)
+              81:    6(float)   ExtInst 1(GLSL.std.450) 15(Tan) 80
+                                Store 71(f) 81
+                                Branch 69
               66:               Label
               74:    6(float)   Load 73(x)
               75:    6(float)   ExtInst 1(GLSL.std.450) 13(Sin) 74
@@ -116,17 +121,19 @@ Linked fragment stage:
               78:    6(float)   ExtInst 1(GLSL.std.450) 14(Cos) 77
                                 Store 71(f) 78
                                 Branch 69
-              68:               Label
-              80:    6(float)   Load 73(x)
-              81:    6(float)   ExtInst 1(GLSL.std.450) 15(Tan) 80
-                                Store 71(f) 81
-                                Branch 69
               69:             Label
               83:      9(int) Load 60(c)
                               SelectionMerge 87 None
                               Switch 83 86 
                                      case 1: 84
                                      case 2: 85
+              86:               Label
+              97:    6(float)   Load 73(x)
+              98:    6(float)   ExtInst 1(GLSL.std.450) 15(Tan) 97
+              99:    6(float)   Load 71(f)
+             100:    6(float)   FAdd 99 98
+                                Store 71(f) 100
+                                Branch 87
               84:               Label
               88:    6(float)   Load 73(x)
               89:    6(float)   ExtInst 1(GLSL.std.450) 13(Sin) 88
@@ -141,13 +148,6 @@ Linked fragment stage:
               95:    6(float)   FAdd 94 93
                                 Store 71(f) 95
                                 Branch 87
-              86:               Label
-              97:    6(float)   Load 73(x)
-              98:    6(float)   ExtInst 1(GLSL.std.450) 15(Tan) 97
-              99:    6(float)   Load 71(f)
-             100:    6(float)   FAdd 99 98
-                                Store 71(f) 100
-                                Branch 87
               87:             Label
              102:      9(int) Load 60(c)
                               SelectionMerge 105 None
@@ -174,6 +174,13 @@ Linked fragment stage:
                               Switch 117 120 
                                      case 1: 118
                                      case 2: 119
+             120:               Label
+             148:    6(float)   Load 73(x)
+             149:    6(float)   ExtInst 1(GLSL.std.450) 15(Tan) 148
+             150:    6(float)   Load 71(f)
+             151:    6(float)   FAdd 150 149
+                                Store 71(f) 151
+                                Branch 121
              118:               Label
              122:    6(float)   Load 73(x)
              123:    6(float)   ExtInst 1(GLSL.std.450) 13(Sin) 122
@@ -207,13 +214,6 @@ Linked fragment stage:
                                   Branch 131
              131:               Label
                                 Branch 121
-             120:               Label
-             148:    6(float)   Load 73(x)
-             149:    6(float)   ExtInst 1(GLSL.std.450) 15(Tan) 148
-             150:    6(float)   Load 71(f)
-             151:    6(float)   FAdd 150 149
-                                Store 71(f) 151
-                                Branch 121
              121:             Label
                               Store 153(i) 154
                               Branch 155
@@ -228,6 +228,13 @@ Linked fragment stage:
                                 Switch 162 165 
                                        case 1: 163
                                        case 2: 164
+             165:                 Label
+             196:    6(float)     Load 73(x)
+             197:    6(float)     ExtInst 1(GLSL.std.450) 15(Tan) 196
+             198:    6(float)     Load 71(f)
+             199:    6(float)     FAdd 198 197
+                                  Store 71(f) 199
+                                  Branch 166
              163:                 Label
              167:    6(float)     Load 73(x)
              168:    6(float)     ExtInst 1(GLSL.std.450) 13(Sin) 167
@@ -265,13 +272,6 @@ Linked fragment stage:
              193:    6(float)     FAdd 192 191
                                   Store 71(f) 193
                                   Branch 166
-             165:                 Label
-             196:    6(float)     Load 73(x)
-             197:    6(float)     ExtInst 1(GLSL.std.450) 15(Tan) 196
-             198:    6(float)     Load 71(f)
-             199:    6(float)     FAdd 198 197
-                                  Store 71(f) 199
-                                  Branch 166
              166:               Label
              201:    6(float)   Load 71(f)
              203:   160(bool)   FOrdLessThan 201 202
@@ -331,10 +331,10 @@ Linked fragment stage:
                               SelectionMerge 254 None
                               Switch 251 253 
                                      case 0: 252
-             252:               Label
-                                Branch 254
              253:               Label
                                 Branch 254
+             252:               Label
+                                Branch 254
              254:             Label
              258:      9(int) Load 60(c)
                               SelectionMerge 260 None

Test/baseResults/spv.while-continue-break.vert.out

@@ -60,9 +60,6 @@ Linked vertex stage:
               23:                 Label
                                   Store 25(B) 20
                                   Branch 10
-              26:                 Label
-                                  Store 27(C) 20
-                                  Branch 24
               24:               Label
               28:      6(int)   Load 8(i)
               30:      6(int)   SMod 28 29
@@ -72,9 +69,6 @@ Linked vertex stage:
               32:                 Label
                                   Store 25(B) 20
                                   Branch 11
-              34:                 Label
-                                  Store 27(C) 20
-                                  Branch 33
               33:               Label
               35:      6(int)   Load 8(i)
               36:      6(int)   IAdd 35 18

Test/spv.branch-return.vert

@@ -0,0 +1,10 @@
+#version 300 es
+void main() {
+  switch (gl_InstanceID) {
+    case 0: return;
+    case 1: gl_Position = vec4(0.0); break;
+    case 2: return;
+    case 3: return;
+  }
+  gl_Position.x += 0.123;
+}

Test/spv.merge-unreachable.frag

@@ -0,0 +1,7 @@
+#version 450
+layout(location=1) in highp vec4 v;
+void main (void)
+{
+  if (v == vec4(0.1,0.2,0.3,0.4)) discard;
+  else return;
+}

Test/test-spirv-list

@@ -32,6 +32,7 @@ spv.always-discard.frag
 spv.always-discard2.frag
 spv.bitCast.frag
 spv.bool.vert
+spv.branch-return.vert
 spv.conditionalDiscard.frag
 spv.conversion.frag
 spv.dataOut.frag
@@ -56,6 +57,7 @@ spv.loopsArtificial.frag
 spv.matFun.vert
 spv.matrix.frag
 spv.matrix2.frag
+spv.merge-unreachable.frag
 spv.newTexture.frag
 spv.nonSquare.vert
 spv.Operations.frag