Skip to content

Commit 2ad1dfa

Browse files
HerrCai0907atc-github
authored andcommitted
fix(cfg): provide correct inside loop BB when there are branch in loop (#112)
The original DFS does not consider the cases that there are branch inside loop. Then the BB in first branch reach the loop entry BB. the branch merge BB will be treated as visited and be skipped when searching the second branch BB. This results in the BB of the other branch not being correctly added to the collection of inside loops.
1 parent bd8d512 commit 2ad1dfa

File tree

1 file changed

+52
-5
lines changed

1 file changed

+52
-5
lines changed

passes/helper/CFG.cpp

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <algorithm>
44
#include <cassert>
55
#include <iterator>
6+
#include <map>
67
#include <optional>
78
#include <string>
89
#include <utility>
@@ -185,20 +186,24 @@ DynBitset CFG::getBlockInsideLoop() const {
185186
DynBitset visited_;
186187
std::vector<BasicBlock const *> stack_;
187188
DynBitset insideLoop_;
189+
std::map<BasicBlock const *, BasicBlock const *> insideLoopMappedLoopEntry_;
188190

189191
explicit BasicBlockDeepFirstVisitor(size_t size) : active_{size}, visited_{size}, insideLoop_{size} {}
190192
void visit(BasicBlock const *bb) {
193+
// in wasm, only natural loop supportted
191194
uint32_t const index = bb->getIndex();
192195
// if the BB is already in loop, skip it
193-
if (insideLoop_.get(index))
196+
if (insideLoop_.get(index)) {
197+
markLoop(insideLoopMappedLoopEntry_.at(bb));
194198
return;
199+
}
200+
if (visited_.get(index))
201+
return;
202+
195203
active_.set(index, true);
196204
stack_.push_back(bb);
197205
for (BasicBlock const *succ : bb->succs()) {
198206
size_t const succIndex = succ->getIndex();
199-
// in wasm, only natural loop supportted
200-
if (visited_.get(succIndex))
201-
continue;
202207
// back edge means loop, back edge targeted bb is loop entry
203208
if (active_.get(succIndex)) {
204209
markLoop(succ);
@@ -216,8 +221,11 @@ DynBitset CFG::getBlockInsideLoop() const {
216221
auto const loopEntryIt = std::find(stack_.rbegin(), stack_.rend(), loopEntry);
217222
assert(loopEntryIt != stack_.rend());
218223
insideLoop_.set(loopEntry->getIndex(), true);
219-
for (auto it = stack_.rbegin(); it != loopEntryIt; ++it)
224+
insideLoopMappedLoopEntry_.insert_or_assign(loopEntry, loopEntry);
225+
for (auto it = stack_.rbegin(); it != loopEntryIt; ++it) {
220226
insideLoop_.set((*it)->getIndex(), true);
227+
insideLoopMappedLoopEntry_.insert_or_assign(*it, loopEntry);
228+
}
221229
}
222230
};
223231
BasicBlockDeepFirstVisitor visitor{size()};
@@ -301,6 +309,45 @@ TEST(CFGTestGetBlockInsideLoop, Branch) {
301309
EXPECT_FALSE(insideLoop.get(e));
302310
}
303311

312+
TEST(CFGTestGetBlockInsideLoop, BranchInLoop) {
313+
CFGTestWrapper cfg{};
314+
/*
315+
entry
316+
|
317+
a<-----
318+
/ \ |
319+
b c |
320+
\ / |
321+
d------
322+
|
323+
exit
324+
*/
325+
size_t a = cfg.addBB();
326+
size_t b = cfg.addBB();
327+
size_t c = cfg.addBB();
328+
size_t d = cfg.addBB();
329+
size_t exit = cfg.addExitBB();
330+
331+
cfg.linkBBs(cfg.entry_, a);
332+
cfg.linkBBs(a, b);
333+
cfg.linkBBs(a, c);
334+
cfg.linkBBs(b, d);
335+
cfg.linkBBs(c, d);
336+
cfg.linkBBs(d, a);
337+
cfg.linkBBs(d, exit);
338+
339+
DynBitset const insideLoop = cfg.raw_.getBlockInsideLoop();
340+
ASSERT_EQ(insideLoop.size(), cfg.size());
341+
342+
EXPECT_TRUE(insideLoop.get(a));
343+
EXPECT_TRUE(insideLoop.get(b));
344+
EXPECT_TRUE(insideLoop.get(c));
345+
EXPECT_TRUE(insideLoop.get(d));
346+
347+
EXPECT_FALSE(insideLoop.get(cfg.entry_));
348+
EXPECT_FALSE(insideLoop.get(exit));
349+
}
350+
304351
} // namespace warpo::passes::ut
305352

306353
#endif // WARPO_ENABLE_UNIT_TESTS

0 commit comments

Comments
 (0)