diff --git a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h --- a/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h +++ b/llvm/include/llvm/Transforms/Utils/ScalarEvolutionExpander.h @@ -499,10 +499,9 @@ void fixupInsertPoints(Instruction *I); - /// If required, create LCSSA PHIs for \p Users' operand \p OpIdx. If new - /// LCSSA PHIs have been created, return the LCSSA PHI available at \p User. - /// If no PHIs have been created, return the unchanged operand \p OpIdx. - Value *fixupLCSSAFormFor(Instruction *User, unsigned OpIdx); + /// Create LCSSA PHIs for \p V, if it is required for uses at the Builder's + /// current insertion point. + Value *fixupLCSSAFormFor(Value *V); }; /// Helper to remove instructions inserted during SCEV expansion, unless they diff --git a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp --- a/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp +++ b/llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp @@ -14,6 +14,7 @@ #include "llvm/Transforms/Utils/ScalarEvolutionExpander.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallSet.h" #include "llvm/Analysis/InstructionSimplify.h" #include "llvm/Analysis/LoopInfo.h" @@ -1742,30 +1743,6 @@ // Expand the code for this SCEV. Value *V = expand(SH); - if (PreserveLCSSA) { - if (auto *Inst = dyn_cast(V)) { - // Create a temporary instruction to at the current insertion point, so we - // can hand it off to the helper to create LCSSA PHIs if required for the - // new use. - // FIXME: Ideally formLCSSAForInstructions (used in fixupLCSSAFormFor) - // would accept a insertion point and return an LCSSA phi for that - // insertion point, so there is no need to insert & remove the temporary - // instruction. - Type *ToTy; - if (Inst->getType()->isIntegerTy()) - ToTy = Inst->getType()->getPointerTo(); - else - ToTy = Type::getInt32Ty(Inst->getContext()); - Instruction *Tmp = CastInst::CreateBitOrPointerCast( - Inst, ToTy, "tmp.lcssa.user", &*Builder.GetInsertPoint()); - V = fixupLCSSAFormFor(Tmp, 0); - - // Clean up temporary instruction. - Tmp->eraseFromParent(); - } - } - - InsertedExpressions[std::make_pair(SH, &*Builder.GetInsertPoint())] = V; if (Ty) { assert(SE.getTypeSizeInBits(Ty) == SE.getTypeSizeInBits(SH->getType()) && "non-trivial casts should be done with the SCEVs directly!"); @@ -1870,9 +1847,10 @@ // Expand the expression into instructions. Value *V = FindValueInExprValueMap(S, InsertPt); - if (!V) + if (!V) { V = visit(S); - else { + V = fixupLCSSAFormFor(V); + } else { // If we're reusing an existing instruction, we are effectively CSEing two // copies of the instruction (with potentially different flags). As such, // we need to drop any poison generating flags unless we can prove that @@ -1899,18 +1877,6 @@ InsertedValues.insert(V); }; DoInsert(I); - - if (!PreserveLCSSA) - return; - - if (auto *Inst = dyn_cast(I)) { - // A new instruction has been added, which might introduce new uses outside - // a defining loop. Fix LCSSA from for each operand of the new instruction, - // if required. - for (unsigned OpIdx = 0, OpEnd = Inst->getNumOperands(); OpIdx != OpEnd; - OpIdx++) - fixupLCSSAFormFor(Inst, OpIdx); - } } /// replaceCongruentIVs - Check for congruent phis in this loop header and @@ -2541,21 +2507,36 @@ return Builder.CreateOr(Checks); } -Value *SCEVExpander::fixupLCSSAFormFor(Instruction *User, unsigned OpIdx) { - assert(PreserveLCSSA); - SmallVector ToUpdate; - - auto *OpV = User->getOperand(OpIdx); - auto *OpI = dyn_cast(OpV); - if (!OpI) - return OpV; +Value *SCEVExpander::fixupLCSSAFormFor(Value *V) { + auto *DefI = dyn_cast(V); + if (!PreserveLCSSA || !DefI) + return V; - Loop *DefLoop = SE.LI.getLoopFor(OpI->getParent()); - Loop *UseLoop = SE.LI.getLoopFor(User->getParent()); + Instruction *InsertPt = &*Builder.GetInsertPoint(); + Loop *DefLoop = SE.LI.getLoopFor(DefI->getParent()); + Loop *UseLoop = SE.LI.getLoopFor(InsertPt->getParent()); if (!DefLoop || UseLoop == DefLoop || DefLoop->contains(UseLoop)) - return OpV; + return V; - ToUpdate.push_back(OpI); + // Create a temporary instruction to at the current insertion point, so we + // can hand it off to the helper to create LCSSA PHIs if required for the + // new use. + // FIXME: Ideally formLCSSAForInstructions (used in fixupLCSSAFormFor) + // would accept a insertion point and return an LCSSA phi for that + // insertion point, so there is no need to insert & remove the temporary + // instruction. + Type *ToTy; + if (DefI->getType()->isIntegerTy()) + ToTy = DefI->getType()->getPointerTo(); + else + ToTy = Type::getInt32Ty(DefI->getContext()); + Instruction *User = + CastInst::CreateBitOrPointerCast(DefI, ToTy, "tmp.lcssa.user", InsertPt); + auto RemoveUserOnExit = + make_scope_exit([User]() { User->eraseFromParent(); }); + + SmallVector ToUpdate; + ToUpdate.push_back(DefI); SmallVector PHIsToRemove; formLCSSAForInstructions(ToUpdate, SE.DT, SE.LI, &SE, Builder, &PHIsToRemove); for (PHINode *PN : PHIsToRemove) { @@ -2566,7 +2547,7 @@ PN->eraseFromParent(); } - return User->getOperand(OpIdx); + return User->getOperand(0); } namespace { diff --git a/llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll b/llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll --- a/llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll +++ b/llvm/test/Transforms/IndVarSimplify/lcssa-preservation.ll @@ -86,3 +86,59 @@ %res.lcssa2 = phi i64 [ %res.lcssa, %loop1.latch ] ret i64 %res.lcssa2 } + +; Check that it does not crash because the incoming values of an LCSSA phi are +; equal. +define void @pr57000(i64 %a) { +; CHECK-LABEL: @pr57000( +; CHECK-NEXT: entry: +; CHECK-NEXT: br label [[LOOP_1_OUTER:%.*]] +; CHECK: loop.1.loopexit: +; CHECK-NEXT: br label [[LOOP_1_OUTER]] +; CHECK: loop.1.outer: +; CHECK-NEXT: br label [[LOOP_1:%.*]] +; CHECK: loop.1: +; CHECK-NEXT: [[CMP:%.*]] = icmp sle i64 [[A:%.*]], 2546175499358690212 +; CHECK-NEXT: [[CMP_EXT:%.*]] = zext i1 [[CMP]] to i8 +; CHECK-NEXT: br i1 [[CMP]], label [[LOOP_1]], label [[LOOP_2_HEADER_PREHEADER:%.*]] +; CHECK: loop.2.header.preheader: +; CHECK-NEXT: [[CMP_LCSSA:%.*]] = phi i1 [ [[CMP]], [[LOOP_1]] ] +; CHECK-NEXT: [[CMP_EXT_LCSSA:%.*]] = phi i8 [ [[CMP_EXT]], [[LOOP_1]] ] +; CHECK-NEXT: br label [[LOOP_2_HEADER_OUTER:%.*]] +; CHECK: loop.2.header.outer: +; CHECK-NEXT: br label [[LOOP_2_HEADER:%.*]] +; CHECK: loop.2.header: +; CHECK-NEXT: switch i8 [[CMP_EXT_LCSSA]], label [[LOOP_1_LOOPEXIT:%.*]] [ +; CHECK-NEXT: i8 -1, label [[LOOP_2_LATCH:%.*]] +; CHECK-NEXT: i8 1, label [[LOOP_2_LATCH]] +; CHECK-NEXT: i8 4, label [[LOOP_2_HEADER]] +; CHECK-NEXT: ] +; CHECK: loop.2.latch: +; CHECK-NEXT: [[CMP_TRUNC_LCSSA1:%.*]] = phi i1 [ [[CMP_LCSSA]], [[LOOP_2_HEADER]] ], [ [[CMP_LCSSA]], [[LOOP_2_HEADER]] ] +; CHECK-NEXT: call void @use(i1 [[CMP_TRUNC_LCSSA1]]) +; CHECK-NEXT: br label [[LOOP_2_HEADER_OUTER]] +; +entry: + br label %loop.1 + +loop.1: + %p.1 = phi i1 [ 0 , %entry ], [ %p.1, %loop.1 ], [ %p.2, %loop.2.header ] + %cmp = icmp sle i64 %a, 2546175499358690212 + %cmp.ext = zext i1 %cmp to i8 + br i1 %cmp, label %loop.1, label %loop.2.header + +loop.2.header: + %p.2 = phi i1 [ %p.1, %loop.1 ], [ %p.2, %loop.2.header ], [ %cmp, %loop.2.latch ] + %cmp.trunc = trunc i8 %cmp.ext to i1 + switch i8 %cmp.ext, label %loop.1 [ + i8 -1, label %loop.2.latch + i8 1, label %loop.2.latch + i8 4, label %loop.2.header + ] + +loop.2.latch: + call void @use(i1 %cmp.trunc) + br label %loop.2.header +} + +declare void @use(i1) diff --git a/llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll b/llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll --- a/llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll +++ b/llvm/test/Transforms/LoopVectorize/skeleton-lcssa-crash.ll @@ -23,7 +23,6 @@ ; CHECK-NEXT: [[C_3:%.*]] = call i1 @cond() ; CHECK-NEXT: br i1 [[C_3]], label [[LOOP_3_PREHEADER:%.*]], label [[INNER_LATCH:%.*]] ; CHECK: loop.3.preheader: -; CHECK-NEXT: [[L_1_LCSSA7:%.*]] = phi ptr [ [[L_1]], [[INNER_BB]] ] ; CHECK-NEXT: [[L_1_LCSSA:%.*]] = phi ptr [ [[L_1]], [[INNER_BB]] ] ; CHECK-NEXT: [[L_2_LCSSA:%.*]] = phi ptr [ [[L_2]], [[INNER_BB]] ] ; CHECK-NEXT: [[TMP0:%.*]] = add i64 [[N:%.*]], 1 @@ -31,12 +30,12 @@ ; CHECK-NEXT: br i1 [[MIN_ITERS_CHECK]], label [[SCALAR_PH:%.*]], label [[VECTOR_MEMCHECK:%.*]] ; CHECK: vector.memcheck: ; CHECK-NEXT: [[UGLYGEP:%.*]] = getelementptr i8, ptr [[L_2_LCSSA]], i64 2 -; CHECK-NEXT: [[UGLYGEP3:%.*]] = getelementptr i8, ptr [[L_1_LCSSA]], i64 2 +; CHECK-NEXT: [[UGLYGEP5:%.*]] = getelementptr i8, ptr [[L_1_LCSSA]], i64 2 ; CHECK-NEXT: [[TMP1:%.*]] = shl i64 [[N]], 1 ; CHECK-NEXT: [[TMP2:%.*]] = add i64 [[TMP1]], 4 -; CHECK-NEXT: [[UGLYGEP6:%.*]] = getelementptr i8, ptr [[L_1_LCSSA7]], i64 [[TMP2]] +; CHECK-NEXT: [[UGLYGEP6:%.*]] = getelementptr i8, ptr [[L_1_LCSSA]], i64 [[TMP2]] ; CHECK-NEXT: [[BOUND0:%.*]] = icmp ult ptr [[L_2_LCSSA]], [[UGLYGEP6]] -; CHECK-NEXT: [[BOUND1:%.*]] = icmp ult ptr [[UGLYGEP3]], [[UGLYGEP]] +; CHECK-NEXT: [[BOUND1:%.*]] = icmp ult ptr [[UGLYGEP5]], [[UGLYGEP]] ; CHECK-NEXT: [[FOUND_CONFLICT:%.*]] = and i1 [[BOUND0]], [[BOUND1]] ; CHECK-NEXT: br i1 [[FOUND_CONFLICT]], label [[SCALAR_PH]], label [[VECTOR_PH:%.*]] ; CHECK: vector.ph: @@ -81,11 +80,11 @@ ; CHECK: exit.loopexit: ; CHECK-NEXT: br label [[EXIT:%.*]] ; CHECK: exit.loopexit1: -; CHECK-NEXT: [[L_1_LCSSA4:%.*]] = phi ptr [ [[L_1]], [[INNER_LATCH]] ] +; CHECK-NEXT: [[L_1_LCSSA3:%.*]] = phi ptr [ [[L_1]], [[INNER_LATCH]] ] ; CHECK-NEXT: br label [[EXIT]] ; CHECK: exit: -; CHECK-NEXT: [[L_15:%.*]] = phi ptr [ [[L_1_LCSSA4]], [[EXIT_LOOPEXIT1]] ], [ [[L_1_LCSSA]], [[EXIT_LOOPEXIT]] ] -; CHECK-NEXT: [[L_3:%.*]] = load i16, ptr [[L_15]], align 2 +; CHECK-NEXT: [[L_14:%.*]] = phi ptr [ [[L_1_LCSSA3]], [[EXIT_LOOPEXIT1]] ], [ [[L_1_LCSSA]], [[EXIT_LOOPEXIT]] ] +; CHECK-NEXT: [[L_3:%.*]] = load i16, ptr [[L_14]], align 2 ; CHECK-NEXT: ret i16 [[L_3]] ; entry: