From 5fe789b4afb8d1e8067e3a91001399572cbd4db3 Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Sun, 17 Jan 2016 23:27:45 -0500 Subject: [PATCH 01/13] Add Block::successors. --- SPIRV/SpvBuilder.cpp | 2 +- SPIRV/spvIR.h | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp index 4d5ce419..29f48602 100755 --- a/SPIRV/SpvBuilder.cpp +++ b/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 diff --git a/SPIRV/spvIR.h b/SPIRV/spvIR.h index e763dbb9..38e51971 100755 --- a/SPIRV/spvIR.h +++ b/SPIRV/spvIR.h @@ -162,9 +162,10 @@ public: Function& getParent() const { return parent; } void addInstruction(std::unique_ptr 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 inst) { localVariables.push_back(std::move(inst)); } - int getNumPredecessors() const { return (int)predecessors.size(); } + const std::vector getPredecessors() const { return predecessors; } + const std::vector getSuccessors() const { return successors; } void setUnreachable() { unreachable = true; } bool isUnreachable() const { return unreachable; } @@ -206,7 +207,7 @@ protected: friend Function; std::vector > instructions; - std::vector predecessors; + std::vector predecessors, successors; std::vector > localVariables; Function& parent; From 454796e008b1c541734eda997a1afc9d6da257bd Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Mon, 18 Jan 2016 16:18:01 -0500 Subject: [PATCH 02/13] Call addPredecessor() on OpSwitch blocks. --- SPIRV/SpvBuilder.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp index 29f48602..5536a55a 100755 --- a/SPIRV/SpvBuilder.cpp +++ b/SPIRV/SpvBuilder.cpp @@ -1759,10 +1759,13 @@ void Builder::makeSwitch(Id selector, int numSegments, std::vector& 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(switchInst)); From 44bfb0d0cd3417f9d2494d9055d89b45cbf30ff8 Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Mon, 18 Jan 2016 16:18:37 -0500 Subject: [PATCH 03/13] Implement inReadableOrder(). --- SPIRV/CMakeLists.txt | 1 + SPIRV/InReadableOrder.cpp | 64 +++++++++++++++++++++++++++++++++++++++ SPIRV/spvIR.h | 28 ++++++++++++++--- 3 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 SPIRV/InReadableOrder.cpp diff --git a/SPIRV/CMakeLists.txt b/SPIRV/CMakeLists.txt index 945350e7..50cda686 100755 --- a/SPIRV/CMakeLists.txt +++ b/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 diff --git a/SPIRV/InReadableOrder.cpp b/SPIRV/InReadableOrder.cpp new file mode 100644 index 00000000..f0812126 --- /dev/null +++ b/SPIRV/InReadableOrder.cpp @@ -0,0 +1,64 @@ +// 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 +#include +#include +#include + +using spv::Block; +using spv::Id; +using BlockSet = std::unordered_set; +using IdToBool = std::unordered_map; + +namespace { +// True if any of prerequisites have not yet been visited. +bool delay(const BlockSet& prereqs, const IdToBool& visited) { + return std::any_of(prereqs.begin(), prereqs.end(), + [&visited](Id b) { return !visited.count(b); }); +} +} + +void spv::inReadableOrder(Block* root, std::function callback) { + // Prerequisites for a merge block; must be completed prior to visiting the + // merge block. + std::unordered_map prereqs; + IdToBool visited; // Whether a block has already been visited. + std::deque worklist; // DFS worklist + worklist.push_back(root); + while (!worklist.empty()) { + Block* current = worklist.front(); + worklist.pop_front(); + // Nodes may be pushed repeadetly (before they're first visited) if they + // have multiple predecessors. Skip the already-visited ones. + if (visited[current->getId()]) continue; + callback(current); + visited[current->getId()] = true; + if (auto merge = current->getMergeInstruction()) { + auto& mergePrereqs = prereqs[merge->getIdOperand(0)]; + // Delay visiting merge blocks until all branches are visited. + for (const auto succ : current->getSuccessors()) + mergePrereqs.insert(succ->getId()); + } + for (auto succ : current->getSuccessors()) { + if (!visited[succ->getId()] && !delay(prereqs[succ->getId()], visited)) { + worklist.push_back(succ); + } + } + } +} diff --git a/SPIRV/spvIR.h b/SPIRV/spvIR.h index 38e51971..a854384f 100755 --- a/SPIRV/spvIR.h +++ b/SPIRV/spvIR.h @@ -52,10 +52,11 @@ #include "spirv.hpp" -#include +#include +#include #include #include -#include +#include namespace spv { @@ -157,7 +158,7 @@ public: virtual ~Block() { } - + Id getId() { return instructions.front()->getResultId(); } Function& getParent() const { return parent; } @@ -168,6 +169,19 @@ public: const std::vector 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); + switch (nextToLast->getOpCode()) { + case OpSelectionMerge: + case OpLoopMerge: + return nextToLast; + default: + return nullptr; + } + return nullptr; + } bool isTerminated() const { @@ -217,6 +231,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 callback); + // // SPIR-V IR Function. // @@ -253,8 +272,7 @@ public: parameterInstructions[p]->dump(out); // Blocks - for (int b = 0; b < (int)blocks.size(); ++b) - blocks[b]->dump(out); + inReadableOrder(blocks[0], [&out](const Block* b) { b->dump(out); }); Instruction end(0, 0, OpFunctionEnd); end.dump(out); } From baa55a15916cc0a58d981f053c3604766173451e Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Mon, 18 Jan 2016 17:12:59 -0500 Subject: [PATCH 04/13] Add spv.branch-return.vert and fix inReadableOrder(). --- SPIRV/InReadableOrder.cpp | 7 ++- Test/baseResults/spv.branch-return.vert.out | 63 +++++++++++++++++++++ Test/spv.branch-return.vert | 10 ++++ Test/test-spirv-list | 1 + 4 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 Test/baseResults/spv.branch-return.vert.out create mode 100644 Test/spv.branch-return.vert diff --git a/SPIRV/InReadableOrder.cpp b/SPIRV/InReadableOrder.cpp index f0812126..f88e41a8 100644 --- a/SPIRV/InReadableOrder.cpp +++ b/SPIRV/InReadableOrder.cpp @@ -27,10 +27,11 @@ using BlockSet = std::unordered_set; using IdToBool = std::unordered_map; namespace { -// True if any of prerequisites have not yet been visited. -bool delay(const BlockSet& prereqs, const IdToBool& visited) { +// True if any of prerequisites have not yet been visited. May add new, +// zero-initialized, values to visited, but doesn't modify any existing values. +bool delay(const BlockSet& prereqs, IdToBool& visited) { return std::any_of(prereqs.begin(), prereqs.end(), - [&visited](Id b) { return !visited.count(b); }); + [&visited](Id b) { return !visited[b]; }); } } diff --git a/Test/baseResults/spv.branch-return.vert.out b/Test/baseResults/spv.branch-return.vert.out new file mode 100644 index 00000000..75ebefe1 --- /dev/null +++ b/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 diff --git a/Test/spv.branch-return.vert b/Test/spv.branch-return.vert new file mode 100644 index 00000000..0b5897bf --- /dev/null +++ b/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; +} diff --git a/Test/test-spirv-list b/Test/test-spirv-list index af962627..13ff7c0a 100644 --- a/Test/test-spirv-list +++ b/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 From 9c591487adfce2b92a92b4a682a11de6560e3b6a Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Mon, 18 Jan 2016 17:33:25 -0500 Subject: [PATCH 05/13] Fix spv.branch-return.vert. --- SPIRV/InReadableOrder.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/SPIRV/InReadableOrder.cpp b/SPIRV/InReadableOrder.cpp index f88e41a8..b24d3ef2 100644 --- a/SPIRV/InReadableOrder.cpp +++ b/SPIRV/InReadableOrder.cpp @@ -39,7 +39,7 @@ void spv::inReadableOrder(Block* root, std::function callback) { // Prerequisites for a merge block; must be completed prior to visiting the // merge block. std::unordered_map prereqs; - IdToBool visited; // Whether a block has already been visited. + IdToBool visited; // Whether a block has already been visited. std::deque worklist; // DFS worklist worklist.push_back(root); while (!worklist.empty()) { @@ -48,18 +48,22 @@ void spv::inReadableOrder(Block* root, std::function callback) { // Nodes may be pushed repeadetly (before they're first visited) if they // have multiple predecessors. Skip the already-visited ones. if (visited[current->getId()]) continue; + if (delay(prereqs[current->getId()], visited)) { + // Not every prerequisite has been visited -- push it off for later. + worklist.push_back(current); + continue; + } callback(current); visited[current->getId()] = true; if (auto merge = current->getMergeInstruction()) { - auto& mergePrereqs = prereqs[merge->getIdOperand(0)]; + Id mergeId = merge->getIdOperand(0); + auto& mergePrereqs = prereqs[mergeId]; // Delay visiting merge blocks until all branches are visited. for (const auto succ : current->getSuccessors()) - mergePrereqs.insert(succ->getId()); + if (succ->getId() != mergeId) mergePrereqs.insert(succ->getId()); } for (auto succ : current->getSuccessors()) { - if (!visited[succ->getId()] && !delay(prereqs[succ->getId()], visited)) { - worklist.push_back(succ); - } + if (!visited[succ->getId()]) worklist.push_back(succ); } } } From 38d039d0637a20c01f55645c3961a564d9f29e3f Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Tue, 19 Jan 2016 10:01:27 -0500 Subject: [PATCH 06/13] Rework inReadableOrder() as a recursive descent. Add a test for unreachable merge block. Update test results with the new order: mainly delaying merge blocks and removing unreachable ones. --- SPIRV/InReadableOrder.cpp | 77 +++++++++---------- SPIRV/spvIR.h | 13 +++- Test/baseResults/spv.always-discard.frag.out | 19 ----- .../spv.do-while-continue-break.vert.out | 6 -- .../spv.for-continue-break.vert.out | 6 -- .../spv.merge-unreachable.frag.out | 47 +++++++++++ Test/baseResults/spv.switch.frag.out | 56 +++++++------- .../spv.while-continue-break.vert.out | 6 -- Test/spv.merge-unreachable.frag | 7 ++ Test/test-spirv-list | 1 + 10 files changed, 131 insertions(+), 107 deletions(-) mode change 100755 => 100644 Test/baseResults/spv.always-discard.frag.out mode change 100755 => 100644 Test/baseResults/spv.do-while-continue-break.vert.out mode change 100755 => 100644 Test/baseResults/spv.for-continue-break.vert.out create mode 100644 Test/baseResults/spv.merge-unreachable.frag.out mode change 100755 => 100644 Test/baseResults/spv.while-continue-break.vert.out create mode 100644 Test/spv.merge-unreachable.frag diff --git a/SPIRV/InReadableOrder.cpp b/SPIRV/InReadableOrder.cpp index b24d3ef2..de506f41 100644 --- a/SPIRV/InReadableOrder.cpp +++ b/SPIRV/InReadableOrder.cpp @@ -17,53 +17,52 @@ #include "spvIR.h" #include +#include #include #include -#include using spv::Block; using spv::Id; -using BlockSet = std::unordered_set; -using IdToBool = std::unordered_map; namespace { -// True if any of prerequisites have not yet been visited. May add new, -// zero-initialized, values to visited, but doesn't modify any existing values. -bool delay(const BlockSet& prereqs, IdToBool& visited) { - return std::any_of(prereqs.begin(), prereqs.end(), - [&visited](Id b) { return !visited[b]; }); -} +// 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 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()) + if (succ != mergeBlock) visit(succ); + if (mergeBlock) { + delayed_[mergeBlock] = false; + visit(mergeBlock); + } + } + + private: + std::function callback_; + // Whether a block has already been visited or is being delayed. + std::unordered_map visited_, delayed_; +}; } void spv::inReadableOrder(Block* root, std::function callback) { - // Prerequisites for a merge block; must be completed prior to visiting the - // merge block. - std::unordered_map prereqs; - IdToBool visited; // Whether a block has already been visited. - std::deque worklist; // DFS worklist - worklist.push_back(root); - while (!worklist.empty()) { - Block* current = worklist.front(); - worklist.pop_front(); - // Nodes may be pushed repeadetly (before they're first visited) if they - // have multiple predecessors. Skip the already-visited ones. - if (visited[current->getId()]) continue; - if (delay(prereqs[current->getId()], visited)) { - // Not every prerequisite has been visited -- push it off for later. - worklist.push_back(current); - continue; - } - callback(current); - visited[current->getId()] = true; - if (auto merge = current->getMergeInstruction()) { - Id mergeId = merge->getIdOperand(0); - auto& mergePrereqs = prereqs[mergeId]; - // Delay visiting merge blocks until all branches are visited. - for (const auto succ : current->getSuccessors()) - if (succ->getId() != mergeId) mergePrereqs.insert(succ->getId()); - } - for (auto succ : current->getSuccessors()) { - if (!visited[succ->getId()]) worklist.push_back(succ); - } - } + ReadableOrderTraverser(callback).visit(root); } diff --git a/SPIRV/spvIR.h b/SPIRV/spvIR.h index a854384f..307711bf 100755 --- a/SPIRV/spvIR.h +++ b/SPIRV/spvIR.h @@ -60,6 +60,7 @@ namespace spv { +class Block; class Function; class Module; @@ -108,6 +109,8 @@ public: addImmediateOperand(word); } } + void setBlock(Block* b) { block = b; } + Block* getBlock() { return block; } Op getOpCode() const { return opCode; } int getNumOperands() const { return (int)operands.size(); } Id getResultId() const { return resultId; } @@ -146,6 +149,7 @@ protected: Op opCode; std::vector operands; std::string originalString; // could be optimized away; convenience for getting string operand + Block* block; }; // @@ -165,8 +169,8 @@ public: void addInstruction(std::unique_ptr inst); void addPredecessor(Block* pred) { predecessors.push_back(pred); pred->successors.push_back(this);} void addLocalVariable(std::unique_ptr inst) { localVariables.push_back(std::move(inst)); } - const std::vector getPredecessors() const { return predecessors; } - const std::vector getSuccessors() const { return successors; } + const std::vector& getPredecessors() const { return predecessors; } + const std::vector& getSuccessors() const { return successors; } void setUnreachable() { unreachable = true; } bool isUnreachable() const { return unreachable; } // Returns the block's merge instruction, if one exists (otherwise null). @@ -370,12 +374,15 @@ __inline void Function::addLocalVariable(std::unique_ptr inst) __inline Block::Block(Id id, Function& parent) : parent(parent), unreachable(false) { instructions.push_back(std::unique_ptr(new Instruction(id, NoType, OpLabel))); + instructions.back()->setBlock(this); + parent.getParent().mapInstruction(instructions.back()); } __inline void Block::addInstruction(std::unique_ptr 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); } diff --git a/Test/baseResults/spv.always-discard.frag.out b/Test/baseResults/spv.always-discard.frag.out old mode 100755 new mode 100644 index a639515b..73e34bdc --- a/Test/baseResults/spv.always-discard.frag.out +++ b/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 diff --git a/Test/baseResults/spv.do-while-continue-break.vert.out b/Test/baseResults/spv.do-while-continue-break.vert.out old mode 100755 new mode 100644 index c483861c..44a1730d --- a/Test/baseResults/spv.do-while-continue-break.vert.out +++ b/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 diff --git a/Test/baseResults/spv.for-continue-break.vert.out b/Test/baseResults/spv.for-continue-break.vert.out old mode 100755 new mode 100644 index 92976eb5..c759fc65 --- a/Test/baseResults/spv.for-continue-break.vert.out +++ b/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) diff --git a/Test/baseResults/spv.merge-unreachable.frag.out b/Test/baseResults/spv.merge-unreachable.frag.out new file mode 100644 index 00000000..65429768 --- /dev/null +++ b/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 diff --git a/Test/baseResults/spv.switch.frag.out b/Test/baseResults/spv.switch.frag.out index 0d430832..918c1d66 100755 --- a/Test/baseResults/spv.switch.frag.out +++ b/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 diff --git a/Test/baseResults/spv.while-continue-break.vert.out b/Test/baseResults/spv.while-continue-break.vert.out old mode 100755 new mode 100644 index 5d6094f5..b2ccbaaa --- a/Test/baseResults/spv.while-continue-break.vert.out +++ b/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 diff --git a/Test/spv.merge-unreachable.frag b/Test/spv.merge-unreachable.frag new file mode 100644 index 00000000..12f57cd1 --- /dev/null +++ b/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; +} diff --git a/Test/test-spirv-list b/Test/test-spirv-list index 13ff7c0a..d0661245 100644 --- a/Test/test-spirv-list +++ b/Test/test-spirv-list @@ -57,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 From 377f0cab2613de3f9c67cd881db7170b63eb862d Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Tue, 19 Jan 2016 10:17:33 -0500 Subject: [PATCH 07/13] Fix merge issues. --- SPIRV/spvIR.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SPIRV/spvIR.h b/SPIRV/spvIR.h index 307711bf..91e42aa3 100755 --- a/SPIRV/spvIR.h +++ b/SPIRV/spvIR.h @@ -176,7 +176,7 @@ public: // 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); + const Instruction* nextToLast = (instructions.cend() - 2)->get(); switch (nextToLast->getOpCode()) { case OpSelectionMerge: case OpLoopMerge: @@ -375,7 +375,7 @@ __inline Block::Block(Id id, Function& parent) : parent(parent), unreachable(fal { instructions.push_back(std::unique_ptr(new Instruction(id, NoType, OpLabel))); instructions.back()->setBlock(this); - parent.getParent().mapInstruction(instructions.back()); + parent.getParent().mapInstruction(instructions.back().get()); } __inline void Block::addInstruction(std::unique_ptr inst) From fa242904b04edd7f0c21cc005b0a25fe8c48784f Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Tue, 19 Jan 2016 11:31:55 -0500 Subject: [PATCH 08/13] Make Instruction::getBlock() const. --- SPIRV/spvIR.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SPIRV/spvIR.h b/SPIRV/spvIR.h index 91e42aa3..688068da 100755 --- a/SPIRV/spvIR.h +++ b/SPIRV/spvIR.h @@ -110,7 +110,7 @@ public: } } void setBlock(Block* b) { block = b; } - Block* getBlock() { return block; } + Block* getBlock() const { return block; } Op getOpCode() const { return opCode; } int getNumOperands() const { return (int)operands.size(); } Id getResultId() const { return resultId; } From 57bbde4a99b17865498e915d2eb29190def6f4b7 Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Tue, 19 Jan 2016 11:44:53 -0500 Subject: [PATCH 09/13] Add copyright, remove unused #includes. --- SPIRV/InReadableOrder.cpp | 40 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/SPIRV/InReadableOrder.cpp b/SPIRV/InReadableOrder.cpp index de506f41..a87622a8 100644 --- a/SPIRV/InReadableOrder.cpp +++ b/SPIRV/InReadableOrder.cpp @@ -1,3 +1,41 @@ +// +//Copyright (C) 2016 LunarG, 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. @@ -16,9 +54,7 @@ #include "spvIR.h" -#include #include -#include #include using spv::Block; From a7e734962ec9d27aeb56a8db8109454f3a78475a Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Tue, 19 Jan 2016 11:49:37 -0500 Subject: [PATCH 10/13] Remove a redundant check before visit(succ). --- SPIRV/InReadableOrder.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/SPIRV/InReadableOrder.cpp b/SPIRV/InReadableOrder.cpp index a87622a8..36594530 100644 --- a/SPIRV/InReadableOrder.cpp +++ b/SPIRV/InReadableOrder.cpp @@ -84,8 +84,7 @@ class ReadableOrderTraverser { block->getParent().getParent().getInstruction(mergeId)->getBlock(); delayed_[mergeBlock] = true; } - for (const auto succ : block->getSuccessors()) - if (succ != mergeBlock) visit(succ); + for (const auto succ : block->getSuccessors()) visit(succ); if (mergeBlock) { delayed_[mergeBlock] = false; visit(mergeBlock); From 34bc6c389690df7937dfeb9cad7d473775ecb1fd Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Tue, 19 Jan 2016 14:08:32 -0500 Subject: [PATCH 11/13] Explicitly initialize Instruction::block. --- SPIRV/spvIR.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SPIRV/spvIR.h b/SPIRV/spvIR.h index 688068da..ea8548e3 100755 --- a/SPIRV/spvIR.h +++ b/SPIRV/spvIR.h @@ -77,8 +77,8 @@ const MemorySemanticsMask MemorySemanticsAllMemory = (MemorySemanticsMask)0x3FF; class Instruction { public: - Instruction(Id resultId, Id typeId, Op opCode) : resultId(resultId), typeId(typeId), opCode(opCode) { } - explicit Instruction(Op opCode) : resultId(NoResult), typeId(NoType), 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), block(nullptr) { } virtual ~Instruction() {} void addIdOperand(Id id) { operands.push_back(id); } void addImmediateOperand(unsigned int immediate) { operands.push_back(immediate); } From cce6a8acaf61d3de38e0f5b0934b577e7648cc57 Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Tue, 19 Jan 2016 14:50:12 -0500 Subject: [PATCH 12/13] (C) Google --- SPIRV/InReadableOrder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SPIRV/InReadableOrder.cpp b/SPIRV/InReadableOrder.cpp index 36594530..6299b4ef 100644 --- a/SPIRV/InReadableOrder.cpp +++ b/SPIRV/InReadableOrder.cpp @@ -1,5 +1,5 @@ // -//Copyright (C) 2016 LunarG, Inc. +//Copyright (C) 2016 Google, Inc. // //All rights reserved. // From 159b59faa717d85b90b94aa4315306972c026f8f Mon Sep 17 00:00:00 2001 From: Dejan Mircevski Date: Tue, 19 Jan 2016 14:52:31 -0500 Subject: [PATCH 13/13] Reformat to better match existing style. --- SPIRV/InReadableOrder.cpp | 63 ++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/SPIRV/InReadableOrder.cpp b/SPIRV/InReadableOrder.cpp index 6299b4ef..4eb56d64 100644 --- a/SPIRV/InReadableOrder.cpp +++ b/SPIRV/InReadableOrder.cpp @@ -64,40 +64,41 @@ 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 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; +public: + explicit ReadableOrderTraverser(std::function 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); + } } - for (const auto succ : block->getSuccessors()) visit(succ); - if (mergeBlock) { - delayed_[mergeBlock] = false; - visit(mergeBlock); - } - } - private: - std::function callback_; - // Whether a block has already been visited or is being delayed. - std::unordered_map visited_, delayed_; +private: + std::function callback_; + // Whether a block has already been visited or is being delayed. + std::unordered_map visited_, delayed_; }; } -void spv::inReadableOrder(Block* root, std::function callback) { - ReadableOrderTraverser(callback).visit(root); +void spv::inReadableOrder(Block* root, std::function callback) +{ + ReadableOrderTraverser(callback).visit(root); }