Index: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp =================================================================== --- lib/Transforms/Scalar/SimpleLoopUnswitch.cpp +++ lib/Transforms/Scalar/SimpleLoopUnswitch.cpp @@ -1740,8 +1740,9 @@ /// well by cloning the loop if the result is small enough. static bool unswitchLoop(Loop &L, DominatorTree &DT, LoopInfo &LI, AssumptionCache &AC, - TargetTransformInfo &TTI, bool NonTrivial, + TargetTransformInfo &TTI, ScalarEvolution *SE, bool NonTrivial, function_ref)> UnswitchCB) { + assert(!L.isInvalid() && "Unswitching invalid loop?"); assert(L.isRecursivelyLCSSAForm(DT, LI) && "Loops must be in LCSSA form before unswitching."); @@ -1754,6 +1755,9 @@ // If we unswitched successfully we will want to clean up the loop before // processing it further so just mark it as unswitched and return. UnswitchCB(/*CurrentLoopValid*/ true, {}); + assert(!L.isInvalid() && "Invalid loop after unswitching?"); + if (SE) + SE->forgetTopmostLoop(&L); return true; } @@ -1898,8 +1902,18 @@ LLVM_DEBUG(dbgs() << " Trying to unswitch non-trivial (cost = " << BestUnswitchCost << ") branch: " << *BestUnswitchTI << "\n"); - return unswitchInvariantBranch(L, cast(*BestUnswitchTI), DT, LI, - AC, UnswitchCB); + bool Changed = unswitchInvariantBranch(L, cast(*BestUnswitchTI), + DT, LI, AC, UnswitchCB); + // FIXME: There should be assert on !L.isInvalid() here. However it seems that + // we have a bug that may lead to construction of an invalid loop in the call + // of unswitchInvariantBranch. Specifically, without this check two existing + // unit tests will fail this assert: + // LLVM :: Transforms/SimpleLoopUnswitch/nontrivial-unswitch-cost.ll + // LLVM :: Transforms/SimpleLoopUnswitch/nontrivial-unswitch.ll + // Once it is fixed, the restriction should be lifted and turned into assert. + if (Changed && SE && !L.isInvalid()) + SE->forgetTopmostLoop(&L); + return Changed; } PreservedAnalyses SimpleLoopUnswitchPass::run(Loop &L, LoopAnalysisManager &AM, @@ -1929,7 +1943,7 @@ U.markLoopAsDeleted(L, LoopName); }; - if (!unswitchLoop(L, AR.DT, AR.LI, AR.AC, AR.TTI, NonTrivial, + if (!unswitchLoop(L, AR.DT, AR.LI, AR.AC, AR.TTI, &AR.SE, NonTrivial, UnswitchCB)) return PreservedAnalyses::all(); @@ -1993,8 +2007,11 @@ LPM.markLoopAsDeleted(*L); }; + ScalarEvolution *SE = nullptr; + if (auto *SEWP = getAnalysisIfAvailable()) + SE = &SEWP->getSE(); bool Changed = - unswitchLoop(*L, DT, LI, AC, TTI, NonTrivial, UnswitchCB); + unswitchLoop(*L, DT, LI, AC, TTI, SE, NonTrivial, UnswitchCB); // If anything was unswitched, also clear any cached information about this // loop. Index: test/Transforms/SimpleLoopUnswitch/pr37651.ll =================================================================== --- /dev/null +++ test/Transforms/SimpleLoopUnswitch/pr37651.ll @@ -0,0 +1,37 @@ +; REQUIRES: asserts +; RUN: opt -iv-users -simple-loop-unswitch -indvars -S < %s | FileCheck %s +; RUN: opt -passes='print,loop(unswitch),print' -S < %s | FileCheck %s +; PR37651. Exercise two scenarios: the original scenario how the bug was +; reported and the scenario that shows that the problem lies in unswitching. + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + + +; Make sure that SCEV gets invalidated properly after simple unswitching. +define void @f1() { + +; CHECK-LABEL: @f1 + +vector.ph: + br label %bb3 + +bb3: ; preds = %bb13, %vector.ph + %_tmp5329 = phi i8 [ %_tmp54, %bb13 ], [ undef, %vector.ph ] + %_tmp40 = icmp eq i8 undef, 0 + br label %bb11 + +bb11: ; preds = %bb12, %bb3 + br i1 %_tmp40, label %bb12, label %bb13 + +bb12: ; preds = %bb11 + br i1 false, label %bb11, label %bb13 + +bb13: ; preds = %bb12, %bb11 + %_tmp54 = add nsw i8 %_tmp5329, 1 + %_tmp57 = icmp eq i8 %_tmp54, 0 + br i1 %_tmp57, label %bb14.loopexit, label %bb3 + +bb14.loopexit: ; preds = %bb13 + ret void +}