diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -3074,6 +3074,47 @@ LLVM_DEBUG(dbgs() << "FOLDING BRANCH TO COMMON DEST:\n" << *PBI << *BB); + // In certain cases, while the IR is in SSA form, it's implicit, + // and that makes miscompiles easy. Let's make it explicit instead, + // by forming sudo-LCSSA. + for (Instruction &BonusInst : + reverse(iterator_range(*BB))) { + auto isOffendingExternalUse = [BB, &BonusInst](Use &U) { + auto *UI = cast(U.getUser()); + if (auto *PN = dyn_cast(UI)) + if (PN->getIncomingBlock(U) == BB) + return false; // Uses by incoming values when coming from BB are fine. + return UI->getParent() != BB || !BonusInst.comesBefore(UI); + }; + + // Does this instruction require rewriting of uses? + if (none_of(BonusInst.uses(), isOffendingExternalUse)) + continue; + + SSAUpdater SSAUpdate; + Type *Ty = BonusInst.getType(); + SSAUpdate.Initialize(Ty, BonusInst.getName()); + + // Into each successor block of BB, insert a PHI node, that recieves + // the BonusInst when coming from it's basic block, or poison otherwise. + for (BasicBlock *Succ : successors(BB)) { + // The block may have the same successor multiple times. Do it only once. + if (SSAUpdate.HasValueForBlock(Succ)) + continue; + PHINode *PN = PHINode::Create(Ty, 0, BonusInst.getName(), &Succ->front()); + for (BasicBlock *PredOfSucc : predecessors(Succ)) + PN->addIncoming(PredOfSucc == BB ? (Value *)&BonusInst + : PoisonValue::get(Ty), + PredOfSucc); + SSAUpdate.AddAvailableValue(Succ, PN); + } + + // And rewrite all the external uses of BonusInst to use these PHI nodes. + for (Use &U : make_early_inc_range(BonusInst.uses())) + if (isOffendingExternalUse(U)) + SSAUpdate.RewriteUseAfterInsertions(U); + } + IRBuilder<> Builder(PBI); // The builder is used to create instructions to eliminate the branch in BB. // If BB's terminator has !annotation metadata, add it to the new diff --git a/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll b/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll --- a/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll +++ b/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll @@ -801,7 +801,7 @@ ; CHECK-NEXT: entry: ; CHECK-NEXT: br label [[FOR_BODY:%.*]] ; CHECK: for.body: -; CHECK-NEXT: [[COUNTDOWN:%.*]] = phi i8 [ 8, [[ENTRY:%.*]] ], [ [[DEC_MERGE:%.*]], [[FOR_BODYTHREAD_PRE_SPLIT:%.*]] ] +; CHECK-NEXT: [[COUNTDOWN:%.*]] = phi i8 [ 8, [[ENTRY:%.*]] ], [ [[DEC2:%.*]], [[FOR_BODYTHREAD_PRE_SPLIT:%.*]] ] ; CHECK-NEXT: [[C:%.*]] = call i1 @gen1() ; CHECK-NEXT: br i1 [[C]], label [[FOR_INC:%.*]], label [[IF_THEN:%.*]] ; CHECK: for.inc: @@ -816,10 +816,11 @@ ; CHECK-NEXT: [[OR_COND:%.*]] = select i1 [[C2_NOT]], i1 true, i1 [[CMP_NOT]] ; CHECK-NEXT: br i1 [[OR_COND]], label [[IF_END_LOOPEXIT]], label [[FOR_BODYTHREAD_PRE_SPLIT]] ; CHECK: for.bodythread-pre-split: -; CHECK-NEXT: [[DEC_MERGE]] = phi i8 [ [[DEC]], [[IF_THEN]] ], [ [[DEC_OLD]], [[FOR_INC]] ] +; CHECK-NEXT: [[DEC2]] = phi i8 [ [[DEC_OLD]], [[FOR_INC]] ], [ [[DEC]], [[IF_THEN]] ] ; CHECK-NEXT: call void @sideeffect0() ; CHECK-NEXT: br label [[FOR_BODY]] ; CHECK: if.end.loopexit: +; CHECK-NEXT: [[DEC1:%.*]] = phi i8 [ poison, [[IF_THEN]] ], [ [[DEC_OLD]], [[FOR_INC]] ] ; CHECK-NEXT: ret void ; entry: @@ -852,7 +853,7 @@ ; CHECK-NEXT: entry: ; CHECK-NEXT: br label [[FOR_BODY:%.*]] ; CHECK: for.body: -; CHECK-NEXT: [[COUNTDOWN:%.*]] = phi i8 [ 8, [[ENTRY:%.*]] ], [ [[DEC_MERGE:%.*]], [[FOR_BODYTHREAD_PRE_SPLIT:%.*]] ] +; CHECK-NEXT: [[COUNTDOWN:%.*]] = phi i8 [ 8, [[ENTRY:%.*]] ], [ [[DEC2:%.*]], [[FOR_BODYTHREAD_PRE_SPLIT:%.*]] ] ; CHECK-NEXT: [[C:%.*]] = call i1 @gen1() ; CHECK-NEXT: br i1 [[C]], label [[FOR_INC:%.*]], label [[IF_THEN:%.*]] ; CHECK: for.inc: @@ -867,7 +868,7 @@ ; CHECK-NEXT: [[OR_COND:%.*]] = select i1 [[C2_NOT]], i1 true, i1 [[CMP_NOT]] ; CHECK-NEXT: br i1 [[OR_COND]], label [[IF_END_LOOPEXIT]], label [[FOR_BODYTHREAD_PRE_SPLIT]] ; CHECK: for.bodythread-pre-split: -; CHECK-NEXT: [[DEC_MERGE]] = phi i8 [ [[DEC_OLD]], [[FOR_INC]] ], [ [[DEC_MERGE]], [[FOR_BODYTHREAD_PRE_SPLIT_LOOPBACK:%.*]] ], [ [[DEC]], [[IF_THEN]] ] +; CHECK-NEXT: [[DEC2]] = phi i8 [ poison, [[FOR_BODYTHREAD_PRE_SPLIT_LOOPBACK:%.*]] ], [ [[DEC_OLD]], [[FOR_INC]] ], [ [[DEC]], [[IF_THEN]] ] ; CHECK-NEXT: [[SHOULD_LOOPBACK:%.*]] = phi i1 [ true, [[FOR_INC]] ], [ false, [[FOR_BODYTHREAD_PRE_SPLIT_LOOPBACK]] ], [ true, [[IF_THEN]] ] ; CHECK-NEXT: [[DO_LOOPBACK:%.*]] = and i1 [[SHOULD_LOOPBACK]], [[ENABLE_LOOPBACK:%.*]] ; CHECK-NEXT: call void @sideeffect0() @@ -876,6 +877,7 @@ ; CHECK-NEXT: call void @sideeffect0() ; CHECK-NEXT: br label [[FOR_BODYTHREAD_PRE_SPLIT]] ; CHECK: if.end.loopexit: +; CHECK-NEXT: [[DEC1:%.*]] = phi i8 [ poison, [[IF_THEN]] ], [ [[DEC_OLD]], [[FOR_INC]] ] ; CHECK-NEXT: ret void ; entry: @@ -972,13 +974,14 @@ ; CHECK-NEXT: [[TOBOOL_OLD:%.*]] = icmp ne i16 [[DOTOLD]], 0 ; CHECK-NEXT: br i1 [[TOBOOL_OLD]], label [[LAND_RHS:%.*]], label [[FOR_END:%.*]] ; CHECK: land.rhs: -; CHECK-NEXT: [[DOTMERGE:%.*]] = phi i16 [ [[TMP0:%.*]], [[LAND_RHS]] ], [ [[DOTOLD]], [[ENTRY:%.*]] ] -; CHECK-NEXT: [[CMP:%.*]] = icmp slt i16 [[DOTMERGE]], 0 -; CHECK-NEXT: [[TMP0]] = load i16, i16* @global_pr49510, align 1 -; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i16 [[TMP0]], 0 +; CHECK-NEXT: [[TMP0:%.*]] = phi i16 [ [[DOTOLD]], [[ENTRY:%.*]] ], [ [[TMP1:%.*]], [[LAND_RHS]] ] +; CHECK-NEXT: [[CMP:%.*]] = icmp slt i16 [[TMP0]], 0 +; CHECK-NEXT: [[TMP1]] = load i16, i16* @global_pr49510, align 1 +; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i16 [[TMP1]], 0 ; CHECK-NEXT: [[OR_COND:%.*]] = select i1 [[CMP]], i1 [[TOBOOL]], i1 false ; CHECK-NEXT: br i1 [[OR_COND]], label [[LAND_RHS]], label [[FOR_END]] ; CHECK: for.end: +; CHECK-NEXT: [[TMP2:%.*]] = phi i16 [ poison, [[LAND_RHS]] ], [ [[DOTOLD]], [[ENTRY]] ] ; CHECK-NEXT: ret void ; entry: @@ -997,10 +1000,6 @@ ret void } -; FIXME: -; This is a miscompile if we replace a phi incoming value -; with an updated loaded value *after* it was stored. - @global_pr51125 = global i32 1, align 4 define i32 @pr51125() { @@ -1010,15 +1009,16 @@ ; CHECK-NEXT: [[ISZERO_OLD:%.*]] = icmp eq i32 [[LD_OLD]], 0 ; CHECK-NEXT: br i1 [[ISZERO_OLD]], label [[EXIT:%.*]], label [[L2:%.*]] ; CHECK: L2: -; CHECK-NEXT: [[LD_MERGE:%.*]] = phi i32 [ [[LD:%.*]], [[L2]] ], [ [[LD_OLD]], [[ENTRY:%.*]] ] +; CHECK-NEXT: [[LD2:%.*]] = phi i32 [ [[LD_OLD]], [[ENTRY:%.*]] ], [ [[LD:%.*]], [[L2]] ] ; CHECK-NEXT: store i32 -1, i32* @global_pr51125, align 4 -; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[LD_MERGE]], -1 +; CHECK-NEXT: [[CMP:%.*]] = icmp ne i32 [[LD2]], -1 ; CHECK-NEXT: [[LD]] = load i32, i32* @global_pr51125, align 4 ; CHECK-NEXT: [[ISZERO:%.*]] = icmp eq i32 [[LD]], 0 ; CHECK-NEXT: [[OR_COND:%.*]] = select i1 [[CMP]], i1 true, i1 [[ISZERO]] ; CHECK-NEXT: br i1 [[OR_COND]], label [[EXIT]], label [[L2]] ; CHECK: exit: -; CHECK-NEXT: [[R:%.*]] = phi i32 [ [[LD]], [[L2]] ], [ [[LD_OLD]], [[ENTRY]] ] +; CHECK-NEXT: [[LD1:%.*]] = phi i32 [ poison, [[L2]] ], [ [[LD_OLD]], [[ENTRY]] ] +; CHECK-NEXT: [[R:%.*]] = phi i32 [ [[LD2]], [[L2]] ], [ [[LD_OLD]], [[ENTRY]] ] ; CHECK-NEXT: ret i32 [[R]] ; entry: