From 65d59b42658fdea49898c125f48f20a93de9d156 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 1 Jul 2022 16:11:39 +0200 Subject: [PATCH] [LoopDeletion] Fix deletion with unusual predecessor terminator (PR56266) LoopSimplify only requires that the loop predecessor has a single successor and is safe to hoist into -- it doesn't necessarily have to be an unconditional BranchInst. Adjust LoopDeletion to assert conditions closer to what it actually needs for correctness, namely a single successor and a side-effect-free terminator (as the terminator is getting dropped). Fixes https://github.com/llvm/llvm-project/issues/56266. --- llvm/lib/Transforms/Utils/LoopUtils.cpp | 14 ++++++++------ llvm/test/Transforms/LoopDeletion/pr56266.ll | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 llvm/test/Transforms/LoopDeletion/pr56266.ll diff --git a/llvm/lib/Transforms/Utils/LoopUtils.cpp b/llvm/lib/Transforms/Utils/LoopUtils.cpp index df30dbc..ec898c4 100644 --- a/llvm/lib/Transforms/Utils/LoopUtils.cpp +++ b/llvm/lib/Transforms/Utils/LoopUtils.cpp @@ -491,9 +491,11 @@ void llvm::deleteDeadLoop(Loop *L, DominatorTree *DT, ScalarEvolution *SE, if (SE) SE->forgetLoop(L); - auto *OldBr = dyn_cast(Preheader->getTerminator()); - assert(OldBr && "Preheader must end with a branch"); - assert(OldBr->isUnconditional() && "Preheader must have a single successor"); + Instruction *OldTerm = Preheader->getTerminator(); + assert(!OldTerm->mayHaveSideEffects() && + "Preheader must end with a side-effect-free terminator"); + assert(OldTerm->getNumSuccessors() == 1 && + "Preheader must have a single successor"); // Connect the preheader to the exit block. Keep the old edge to the header // around to perform the dominator tree update in two separate steps // -- #1 insertion of the edge preheader -> exit and #2 deletion of the edge @@ -519,7 +521,7 @@ void llvm::deleteDeadLoop(Loop *L, DominatorTree *DT, ScalarEvolution *SE, // coming to this inner loop, this will break the outer loop structure (by // deleting the backedge of the outer loop). If the outer loop is indeed a // non-loop, it will be deleted in a future iteration of loop deletion pass. - IRBuilder<> Builder(OldBr); + IRBuilder<> Builder(OldTerm); auto *ExitBlock = L->getUniqueExitBlock(); DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Eager); @@ -529,7 +531,7 @@ void llvm::deleteDeadLoop(Loop *L, DominatorTree *DT, ScalarEvolution *SE, Builder.CreateCondBr(Builder.getFalse(), L->getHeader(), ExitBlock); // Remove the old branch. The conditional branch becomes a new terminator. - OldBr->eraseFromParent(); + OldTerm->eraseFromParent(); // Rewrite phis in the exit block to get their inputs from the Preheader // instead of the exiting block. @@ -573,7 +575,7 @@ void llvm::deleteDeadLoop(Loop *L, DominatorTree *DT, ScalarEvolution *SE, assert(L->hasNoExitBlocks() && "Loop should have either zero or one exit blocks."); - Builder.SetInsertPoint(OldBr); + Builder.SetInsertPoint(OldTerm); Builder.CreateUnreachable(); Preheader->getTerminator()->eraseFromParent(); } diff --git a/llvm/test/Transforms/LoopDeletion/pr56266.ll b/llvm/test/Transforms/LoopDeletion/pr56266.ll new file mode 100644 index 0000000..9a70440 --- /dev/null +++ b/llvm/test/Transforms/LoopDeletion/pr56266.ll @@ -0,0 +1,18 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt -S -loop-deletion < %s | FileCheck %s + +define void @test() { +; CHECK-LABEL: @test( +; CHECK-NEXT: br label [[EXIT:%.*]] +; CHECK: exit: +; CHECK-NEXT: ret void +; + switch i16 0, label %loop [ + ] + +loop: + br i1 true, label %exit, label %loop + +exit: + ret void +} -- 2.7.4