From d50c5fb13f00ceac3353ce1a17fb5c53c5d65c03 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 18 Jan 2017 02:41:26 +0000 Subject: [PATCH] [PM] Teach LoopDeletion to correctly update the LPM when loops are deleted. I've expanded its test coverage a bit including adding one test that will crash clearly without this change. llvm-svn: 292332 --- llvm/lib/Transforms/Scalar/LoopDeletion.cpp | 30 ++++++++----- .../Transforms/LoopDeletion/invalidation.ll | 42 +++++++++++++++++++ .../LoopDeletion/multiple-exit-conditions.ll | 2 +- .../Transforms/LoopDeletion/multiple-exits.ll | 3 ++ 4 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 llvm/test/Transforms/LoopDeletion/invalidation.ll diff --git a/llvm/lib/Transforms/Scalar/LoopDeletion.cpp b/llvm/lib/Transforms/Scalar/LoopDeletion.cpp index 62702dacca5b..3089f5cedc61 100644 --- a/llvm/lib/Transforms/Scalar/LoopDeletion.cpp +++ b/llvm/lib/Transforms/Scalar/LoopDeletion.cpp @@ -100,12 +100,16 @@ static bool isLoopDead(Loop *L, ScalarEvolution &SE, /// This entire process relies pretty heavily on LoopSimplify form and LCSSA in /// order to make various safety checks work. /// -/// \returns true if any changes are made, even if the loop is not deleted. +/// \returns true if the loop is deleted. +/// +/// This also sets the \p Changed output parameter to `true` if any changes +/// were made. This may mutate the loop even if it is unable to delete it due +/// to hoisting trivially loop invariant instructions out of the loop. /// /// This also updates the relevant analysis information in \p DT, \p SE, and \p /// LI. static bool deleteLoopIfDead(Loop *L, DominatorTree &DT, ScalarEvolution &SE, - LoopInfo &LI) { + LoopInfo &LI, bool &Changed) { assert(L->isLCSSAForm(DT) && "Expected LCSSA!"); // We can only remove the loop if there is a preheader that we can @@ -135,15 +139,14 @@ static bool deleteLoopIfDead(Loop *L, DominatorTree &DT, ScalarEvolution &SE, return false; // Finally, we have to check that the loop really is dead. - bool Changed = false; if (!isLoopDead(L, SE, ExitingBlocks, ExitBlock, Changed, Preheader)) - return Changed; + return false; // Don't remove loops for which we can't solve the trip count. // They could be infinite, in which case we'd be changing program behavior. const SCEV *S = SE.getMaxBackedgeTakenCount(L); if (isa(S)) - return Changed; + return false; // Now that we know the removal is safe, remove the loop by changing the // branch from the preheader to go to the single exit block. @@ -215,15 +218,21 @@ static bool deleteLoopIfDead(Loop *L, DominatorTree &DT, ScalarEvolution &SE, ++NumDeleted; - return Changed; + return true; } PreservedAnalyses LoopDeletionPass::run(Loop &L, LoopAnalysisManager &AM, LoopStandardAnalysisResults &AR, - LPMUpdater &) { - bool Changed = deleteLoopIfDead(&L, AR.DT, AR.SE, AR.LI); - if (!Changed) + LPMUpdater &Updater) { + bool Changed = false; + + if (deleteLoopIfDead(&L, AR.DT, AR.SE, AR.LI, Changed)) { + assert(Changed && "Cannot delete a loop without changing something!"); + // Need to update the LPM about this loop going away. + Updater.markLoopAsDeleted(L); + } else if (!Changed) { return PreservedAnalyses::all(); + } return getLoopPassPreservedAnalyses(); } @@ -262,5 +271,6 @@ bool LoopDeletionLegacyPass::runOnLoop(Loop *L, LPPassManager &) { ScalarEvolution &SE = getAnalysis().getSE(); LoopInfo &LI = getAnalysis().getLoopInfo(); - return deleteLoopIfDead(L, DT, SE, LI); + bool Changed = false; + return deleteLoopIfDead(L, DT, SE, LI, Changed) || Changed; } diff --git a/llvm/test/Transforms/LoopDeletion/invalidation.ll b/llvm/test/Transforms/LoopDeletion/invalidation.ll new file mode 100644 index 000000000000..5564f90e1ea7 --- /dev/null +++ b/llvm/test/Transforms/LoopDeletion/invalidation.ll @@ -0,0 +1,42 @@ +; Ensure we don't run analyses over loops after they've been deleted. We run +; one version with a no-op loop pass to make sure that the loop doesn't get +; simplified away. +; +; RUN: opt < %s -passes='require,no-op-loop,require' -S \ +; RUN: | FileCheck %s --check-prefixes=CHECK,BEFORE +; RUN: opt < %s -passes='require,loop-deletion,require' -S \ +; RUN: | FileCheck %s --check-prefixes=CHECK,AFTER + + +define void @foo(i64 %n, i64 %m) nounwind { +; CHECK-LABEL: @foo( + +entry: + br label %bb +; CHECK: entry: +; BEFORE-NEXT: br label %bb +; AFTER-NEXT: br label %return + +bb: + %x.0 = phi i64 [ 0, %entry ], [ %t0, %bb2 ] + %t0 = add i64 %x.0, 1 + %t1 = icmp slt i64 %x.0, %n + br i1 %t1, label %bb2, label %return +; BEFORE: bb: +; BEFORE: br i1 {{.*}}, label %bb2, label %return +; AFTER-NOT: bb: +; AFTER-NOT: br + +bb2: + %t2 = icmp slt i64 %x.0, %m + br i1 %t1, label %bb, label %return +; BEFORE: bb2: +; BEFORE: br i1 {{.*}}, label %bb, label %return +; AFTER-NOT: bb2: +; AFTER-NOT: br + +return: + ret void +; CHECK: return: +; CHECK-NEXT: ret void +} diff --git a/llvm/test/Transforms/LoopDeletion/multiple-exit-conditions.ll b/llvm/test/Transforms/LoopDeletion/multiple-exit-conditions.ll index d7d6badb1650..e7b47211d570 100644 --- a/llvm/test/Transforms/LoopDeletion/multiple-exit-conditions.ll +++ b/llvm/test/Transforms/LoopDeletion/multiple-exit-conditions.ll @@ -1,5 +1,5 @@ ; RUN: opt < %s -loop-deletion -S | FileCheck %s -; RUN: opt < %s -passes='require,loop(loop-deletion)' -S | FileCheck %s +; RUN: opt < %s -passes='loop(loop-deletion)' -S | FileCheck %s ; ScalarEvolution can prove the loop iteration is finite, even though ; it can't represent the exact trip count as an expression. That's diff --git a/llvm/test/Transforms/LoopDeletion/multiple-exits.ll b/llvm/test/Transforms/LoopDeletion/multiple-exits.ll index ad9a30601f32..760c3aae4ee7 100644 --- a/llvm/test/Transforms/LoopDeletion/multiple-exits.ll +++ b/llvm/test/Transforms/LoopDeletion/multiple-exits.ll @@ -5,6 +5,9 @@ ; ; RUN: opt < %s -loop-simplify -lcssa -S | FileCheck %s --check-prefixes=CHECK,BEFORE ; RUN: opt < %s -loop-deletion -S | FileCheck %s --check-prefixes=CHECK,AFTER +; +; RUN: opt < %s -passes=no-op-loop -S | FileCheck %s --check-prefixes=CHECK,BEFORE +; RUN: opt < %s -passes=loop-deletion -S | FileCheck %s --check-prefixes=CHECK,AFTER define void @foo(i64 %n, i64 %m) nounwind {