Fix back-branch target for do-while loops.
To ensure back branches always go to a header block, create a header
block even for !testFirst loops. Then unify common code between the
testFirst/!testFirst cases.
Generate the header-block code first, so update golden files.
Realize that certain infinite loops generate invalid SPIR-V, so put a
TODO to instead abort code generation in such cases.
Change-Id: I1e173c8f73daad186cfc666b7d72bd563ed7665d
diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp
index e5e2169..00369d2 100755
--- a/SPIRV/GlslangToSpv.cpp
+++ b/SPIRV/GlslangToSpv.cpp
@@ -1342,19 +1342,17 @@
bool TGlslangToSpvTraverser::visitLoop(glslang::TVisit /* visit */, glslang::TIntermLoop* node)
{
auto blocks = builder.makeNewLoop();
+ builder.createBranch(&blocks.head);
if (node->testFirst() && node->getTest()) {
- spv::Block& head = builder.makeNewBlock();
- builder.createBranch(&head);
-
- builder.setBuildPoint(&head);
+ builder.setBuildPoint(&blocks.head);
node->getTest()->traverse(this);
spv::Id condition =
builder.accessChainLoad(convertGlslangToSpvType(node->getTest()->getType()));
builder.createLoopMerge(&blocks.merge, &blocks.continue_target, spv::LoopControlMaskNone);
builder.createConditionalBranch(condition, &blocks.body, &blocks.merge);
- breakForLoop.push(true);
builder.setBuildPoint(&blocks.body);
+ breakForLoop.push(true);
if (node->getBody())
node->getBody()->traverse(this);
builder.createBranch(&blocks.continue_target);
@@ -1363,8 +1361,14 @@
builder.setBuildPoint(&blocks.continue_target);
if (node->getTerminal())
node->getTerminal()->traverse(this);
- builder.createBranch(&head);
+ builder.createBranch(&blocks.head);
} else {
+ // Spec requires back edges to target header blocks, and every header
+ // block must dominate its merge block. Create an empty header block
+ // here to ensure these conditions are met even when body contains
+ // non-trivial control flow.
+ builder.setBuildPoint(&blocks.head);
+ builder.createLoopMerge(&blocks.merge, &blocks.continue_target, spv::LoopControlMaskNone);
builder.createBranch(&blocks.body);
breakForLoop.push(true);
@@ -1381,14 +1385,17 @@
node->getTest()->traverse(this);
spv::Id condition =
builder.accessChainLoad(convertGlslangToSpvType(node->getTest()->getType()));
- builder.createLoopMerge(&blocks.merge, &blocks.continue_target,
- spv::LoopControlMaskNone);
- builder.createConditionalBranch(condition, &blocks.body, &blocks.merge);
+ builder.createConditionalBranch(condition, &blocks.head, &blocks.merge);
} else {
- builder.createBranch(&blocks.body);
+ // TODO: unless there was a break instruction somewhere in the body,
+ // this is an infinite loop, so we should abort code generation with
+ // a warning. As it stands now, nothing will jump to the merge
+ // block, and it may be dropped as unreachable by the SPIR-V dumper.
+ // That, in turn, will result in a non-existing %ID in the LoopMerge
+ // above.
+ builder.createBranch(&blocks.head);
}
}
-
builder.setBuildPoint(&blocks.merge);
builder.closeLoop();
return false;
diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp
index eb8c4d3..f9c4ae6 100755
--- a/SPIRV/SpvBuilder.cpp
+++ b/SPIRV/SpvBuilder.cpp
@@ -1763,7 +1763,7 @@
Builder::LoopBlocks& Builder::makeNewLoop()
{
- loops.push({makeNewBlock(), makeNewBlock(), makeNewBlock()});
+ loops.push({makeNewBlock(), makeNewBlock(), makeNewBlock(), makeNewBlock()});
return loops.top();
}
diff --git a/SPIRV/SpvBuilder.h b/SPIRV/SpvBuilder.h
index a1ed84c..fa3600b 100755
--- a/SPIRV/SpvBuilder.h
+++ b/SPIRV/SpvBuilder.h
@@ -374,7 +374,7 @@
void endSwitch(std::vector<Block*>& segmentBB);
struct LoopBlocks {
- Block &body, &merge, &continue_target;
+ Block &head, &body, &merge, &continue_target;
};
// Start a new loop and prepare the builder to generate code for it. Until