Skip to content

Commit 3d0ba4b

Browse files
tobiasgrossertobiasgrosser
committed
[PostDom] document the current handling of infinite loops and unreachables
Summary: As we are in the process of changing the behavior of how the post-dominator tree is computed, make sure we have some more test coverage in this area. Current inconsistencies: - Newly unreachable nodes are not added as new roots, in case the PDT is updated but not rebuilt. - Newly unreachable loops are not added to the CFG at all (neither when building from scratch nor when updating the CFG). This is inconsistent with the fact that unreachables are added to the PDT, but unreachable loops not. On the other side, PDT relationships are not loosened at the moment in cases where new unreachable loops are built. This commit is providing additional test coverage for https://reviews.llvm.org/D35851 Reviewers: dberlin, kuhar Reviewed By: kuhar Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D36107 llvm-svn: 309684
1 parent 295cf4d commit 3d0ba4b

File tree

1 file changed

+268
-0
lines changed

1 file changed

+268
-0
lines changed

‎llvm/unittests/IR/DominatorTreeTest.cpp‎

Lines changed: 268 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,274 @@ TEST(DominatorTree, NonUniqueEdges) {
326326
});
327327
}
328328

329+
// Verify that the PDT is correctly updated in case an edge removal results
330+
// in a new unreachable CFG node.
331+
//
332+
// For the following input code and initial PDT:
333+
//
334+
// CFG PDT
335+
//
336+
// A Exit
337+
// | |
338+
// _B D
339+
// / | \ |
340+
// ^ v \ B
341+
// \ / D / \
342+
// C \ C A
343+
// v
344+
// Exit
345+
//
346+
// we verify that CFG' and PDT-updated is obtained after removal of edge C -> B.
347+
//
348+
// CFG' PDT-updated
349+
//
350+
// A Exit
351+
// | |
352+
// B D
353+
// | \ |
354+
// v \ B
355+
// / D \
356+
// C \ A
357+
// | v
358+
// unreachable Exit
359+
//
360+
// WARNING: PDT-updated is inconsistent with PDT-recalculated, which is
361+
// constructed from CFG' when recalculating the PDT from scratch.
362+
//
363+
// PDT-recalculated
364+
//
365+
// Exit
366+
// / | \
367+
// C B D
368+
// |
369+
// A
370+
//
371+
// TODO: document the wanted behavior after resolving this inconsistency.
372+
TEST(DominatorTree, DeletingEdgesIntroducesUnreachables) {
373+
StringRef ModuleString =
374+
"define void @f() {\n"
375+
"A:\n"
376+
" br label %B\n"
377+
"B:\n"
378+
" br i1 undef, label %D, label %C\n"
379+
"C:\n"
380+
" br label %B\n"
381+
"D:\n"
382+
" ret void\n"
383+
"}\n";
384+
385+
// Parse the module.
386+
LLVMContext Context;
387+
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
388+
389+
runWithDomTree(
390+
*M, "f", [&](Function &F, DominatorTree *DT, PostDomTree *PDT) {
391+
Function::iterator FI = F.begin();
392+
393+
FI++;
394+
BasicBlock *B = &*FI++;
395+
BasicBlock *C = &*FI++;
396+
BasicBlock *D = &*FI++;
397+
398+
assert(PDT->dominates(PDT->getNode(D), PDT->getNode(B)));
399+
400+
C->getTerminator()->eraseFromParent();
401+
new UnreachableInst(C->getContext(), C);
402+
403+
DT->deleteEdge(C, B);
404+
PDT->deleteEdge(C, B);
405+
406+
EXPECT_TRUE(PDT->dominates(PDT->getNode(D), PDT->getNode(B)));
407+
EXPECT_EQ(PDT->getNode(C), nullptr);
408+
409+
PDT->recalculate(F);
410+
411+
EXPECT_FALSE(PDT->dominates(PDT->getNode(D), PDT->getNode(B)));
412+
EXPECT_NE(PDT->getNode(C), nullptr);
413+
});
414+
}
415+
416+
// Verify that the PDT is correctly updated in case an edge removal results
417+
// in an infinite loop.
418+
//
419+
// Test case:
420+
//
421+
// CFG PDT
422+
//
423+
// A Exit
424+
// | |
425+
// _B D
426+
// / | \ |
427+
// ^ v \ B
428+
// \ / D / \
429+
// C \ C A
430+
// / \ v
431+
// ^ v Exit
432+
// \_/
433+
//
434+
// After deleting the edge C->B, C is part of an infinite reverse-unreachable
435+
// loop:
436+
//
437+
// CFG' PDT'
438+
//
439+
// A Exit
440+
// | |
441+
// B D
442+
// | \ |
443+
// v \ B
444+
// / D \
445+
// C \ A
446+
// / \ v
447+
// ^ v Exit
448+
// \_/
449+
//
450+
// In PDT, D post-dominates B. We verify that this post-dominance
451+
// relation is preserved _after_ deleting the edge C->B from CFG.
452+
//
453+
// As C now becomes reverse-unreachable, it is not anymore part of the
454+
// PDT. We also verify this property.
455+
//
456+
// TODO: Can we change the PDT definition such that C remains part of the
457+
// CFG, at best without loosing the dominance relation D postdom B.
458+
TEST(DominatorTree, DeletingEdgesIntroducesInfiniteLoop) {
459+
StringRef ModuleString =
460+
"define void @f() {\n"
461+
"A:\n"
462+
" br label %B\n"
463+
"B:\n"
464+
" br i1 undef, label %D, label %C\n"
465+
"C:\n"
466+
" switch i32 undef, label %C [\n"
467+
" i32 0, label %B\n"
468+
" ]\n"
469+
"D:\n"
470+
" ret void\n"
471+
"}\n";
472+
473+
// Parse the module.
474+
LLVMContext Context;
475+
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
476+
477+
runWithDomTree(
478+
*M, "f", [&](Function &F, DominatorTree *DT, PostDomTree *PDT) {
479+
Function::iterator FI = F.begin();
480+
481+
FI++;
482+
BasicBlock *B = &*FI++;
483+
BasicBlock *C = &*FI++;
484+
BasicBlock *D = &*FI++;
485+
486+
assert(PDT->dominates(PDT->getNode(D), PDT->getNode(B)));
487+
488+
auto SwitchC = cast<SwitchInst>(C->getTerminator());
489+
SwitchC->removeCase(SwitchC->case_begin());
490+
DT->deleteEdge(C, B);
491+
PDT->deleteEdge(C, B);
492+
493+
EXPECT_TRUE(PDT->dominates(PDT->getNode(D), PDT->getNode(B)));
494+
EXPECT_EQ(PDT->getNode(C), nullptr);
495+
496+
PDT->recalculate(F);
497+
498+
EXPECT_TRUE(PDT->dominates(PDT->getNode(D), PDT->getNode(B)));
499+
EXPECT_EQ(PDT->getNode(C), nullptr);
500+
});
501+
}
502+
503+
// Verify that the PDT is correctly updated in case an edge removal results
504+
// in an infinite loop.
505+
//
506+
// Test case:
507+
//
508+
// CFG PDT
509+
//
510+
// A Exit
511+
// | / | \
512+
// B-- C B D
513+
// | \ |
514+
// v \ A
515+
// / D
516+
// C--C2 \
517+
// / \ \ v
518+
// ^ v --Exit
519+
// \_/
520+
//
521+
// After deleting the edge C->E, C is part of an infinite reverse-unreachable
522+
// loop:
523+
//
524+
// CFG' PDT'
525+
//
526+
// A Exit
527+
// | |
528+
// B D
529+
// | \ |
530+
// v \ B
531+
// / D \
532+
// C \ A
533+
// / \ v
534+
// ^ v Exit
535+
// \_/
536+
//
537+
// In PDT, D does not post-dominate B. After the edge C->E is removed, a new
538+
// post-dominance relation is introduced.
539+
//
540+
// As C now becomes reverse-unreachable, it is not anymore part of the
541+
// PDT. We also verify this property.
542+
//
543+
// TODO: Can we change the PDT definition such that C remains part of the
544+
// CFG, at best without loosing the dominance relation D postdom B.
545+
TEST(DominatorTree, DeletingEdgesIntroducesInfiniteLoop2) {
546+
StringRef ModuleString =
547+
"define void @f() {\n"
548+
"A:\n"
549+
" br label %B\n"
550+
"B:\n"
551+
" br i1 undef, label %D, label %C\n"
552+
"C:\n"
553+
" switch i32 undef, label %C [\n"
554+
" i32 0, label %C2\n"
555+
" ]\n"
556+
"C2:\n"
557+
" ret void\n"
558+
"D:\n"
559+
" ret void\n"
560+
"}\n";
561+
562+
// Parse the module.
563+
LLVMContext Context;
564+
std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
565+
566+
runWithDomTree(
567+
*M, "f", [&](Function &F, DominatorTree *DT, PostDomTree *PDT) {
568+
Function::iterator FI = F.begin();
569+
570+
FI++;
571+
BasicBlock *B = &*FI++;
572+
BasicBlock *C = &*FI++;
573+
BasicBlock *C2 = &*FI++;
574+
BasicBlock *D = &*FI++;
575+
576+
auto SwitchC = cast<SwitchInst>(C->getTerminator());
577+
SwitchC->removeCase(SwitchC->case_begin());
578+
DT->deleteEdge(C, C2);
579+
PDT->deleteEdge(C, C2);
580+
C2->eraseFromParent();
581+
582+
EXPECT_EQ(DT->getNode(C2), nullptr);
583+
PDT->eraseNode(C2);
584+
585+
EXPECT_TRUE(PDT->dominates(PDT->getNode(D), PDT->getNode(B)));
586+
EXPECT_EQ(PDT->getNode(C), nullptr);
587+
EXPECT_EQ(PDT->getNode(C2), nullptr);
588+
589+
PDT->recalculate(F);
590+
591+
EXPECT_TRUE(PDT->dominates(PDT->getNode(D), PDT->getNode(B)));
592+
EXPECT_EQ(PDT->getNode(C), nullptr);
593+
EXPECT_EQ(PDT->getNode(C2), nullptr);
594+
});
595+
}
596+
329597
namespace {
330598
const auto Insert = CFGBuilder::ActionKind::Insert;
331599
const auto Delete = CFGBuilder::ActionKind::Delete;

0 commit comments

Comments
 (0)