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 @@ -2052,109 +2052,119 @@ if (ScanIdx == 0) return false; - // Okay, we *could* sink last ScanIdx instructions. But how many can we - // actually sink before encountering instruction that is unprofitable to sink? - auto ProfitableToSinkInstruction = [&](LockstepReverseIterator &LRI) { - unsigned NumPHIdValues = 0; - for (auto *I : *LRI) - for (auto *V : PHIOperands[I]) { - if (!InstructionsToSink.contains(V)) - ++NumPHIdValues; - // FIXME: this check is overly optimistic. We may end up not sinking - // said instruction, due to the very same profitability check. - // See @creating_too_many_phis in sink-common-code.ll. - } - LLVM_DEBUG(dbgs() << "SINK: #phid values: " << NumPHIdValues << "\n"); - unsigned NumPHIInsts = NumPHIdValues / UnconditionalPreds.size(); - if ((NumPHIdValues % UnconditionalPreds.size()) != 0) + bool followedByDeoptOrUnreachable = IsBlockFollowedByDeoptOrUnreachable(BB); + + if (!followedByDeoptOrUnreachable) { + // Okay, we *could* sink last ScanIdx instructions. But how many can we + // actually sink before encountering instruction that is unprofitable to + // sink? + auto ProfitableToSinkInstruction = [&](LockstepReverseIterator &LRI) { + unsigned NumPHIdValues = 0; + for (auto *I : *LRI) + for (auto *V : PHIOperands[I]) { + if (!InstructionsToSink.contains(V)) + ++NumPHIdValues; + // FIXME: this check is overly optimistic. We may end up not sinking + // said instruction, due to the very same profitability check. + // See @creating_too_many_phis in sink-common-code.ll. + } + LLVM_DEBUG(dbgs() << "SINK: #phid values: " << NumPHIdValues << "\n"); + unsigned NumPHIInsts = NumPHIdValues / UnconditionalPreds.size(); + if ((NumPHIdValues % UnconditionalPreds.size()) != 0) NumPHIInsts++; - return NumPHIInsts <= 1; - }; + return NumPHIInsts <= 1; + }; - // We've determined that we are going to sink last ScanIdx instructions, - // and recorded them in InstructionsToSink. Now, some instructions may be - // unprofitable to sink. But that determination depends on the instructions - // that we are going to sink. - - // First, forward scan: find the first instruction unprofitable to sink, - // recording all the ones that are profitable to sink. - // FIXME: would it be better, after we detect that not all are profitable. - // to either record the profitable ones, or erase the unprofitable ones? - // Maybe we need to choose (at runtime) the one that will touch least instrs? - LRI.reset(); - int Idx = 0; - SmallPtrSet InstructionsProfitableToSink; - while (Idx < ScanIdx) { - if (!ProfitableToSinkInstruction(LRI)) { - // Too many PHIs would be created. - LLVM_DEBUG( - dbgs() << "SINK: stopping here, too many PHIs would be created!\n"); - break; + // We've determined that we are going to sink last ScanIdx instructions, + // and recorded them in InstructionsToSink. Now, some instructions may be + // unprofitable to sink. But that determination depends on the instructions + // that we are going to sink. + + // First, forward scan: find the first instruction unprofitable to sink, + // recording all the ones that are profitable to sink. + // FIXME: would it be better, after we detect that not all are profitable. + // to either record the profitable ones, or erase the unprofitable ones? + // Maybe we need to choose (at runtime) the one that will touch least + // instrs? + LRI.reset(); + int Idx = 0; + SmallPtrSet InstructionsProfitableToSink; + while (Idx < ScanIdx) { + if (!ProfitableToSinkInstruction(LRI)) { + // Too many PHIs would be created. + LLVM_DEBUG( + dbgs() << "SINK: stopping here, too many PHIs would be created!\n"); + break; + } + InstructionsProfitableToSink.insert((*LRI).begin(), (*LRI).end()); + --LRI; + ++Idx; } - InstructionsProfitableToSink.insert((*LRI).begin(), (*LRI).end()); - --LRI; - ++Idx; - } - // If no instructions can be sunk, early-return. - if (Idx == 0) - return false; + // If no instructions can be sunk, early-return. + if (Idx == 0) + return false; - // Did we determine that (only) some instructions are unprofitable to sink? - if (Idx < ScanIdx) { - // Okay, some instructions are unprofitable. - ScanIdx = Idx; - InstructionsToSink = InstructionsProfitableToSink; - - // But, that may make other instructions unprofitable, too. - // So, do a backward scan, do any earlier instructions become unprofitable? - assert(!ProfitableToSinkInstruction(LRI) && - "We already know that the last instruction is unprofitable to sink"); - ++LRI; - --Idx; - while (Idx >= 0) { - // If we detect that an instruction becomes unprofitable to sink, - // all earlier instructions won't be sunk either, - // so preemptively keep InstructionsProfitableToSink in sync. - // FIXME: is this the most performant approach? - for (auto *I : *LRI) - InstructionsProfitableToSink.erase(I); - if (!ProfitableToSinkInstruction(LRI)) { - // Everything starting with this instruction won't be sunk. - ScanIdx = Idx; - InstructionsToSink = InstructionsProfitableToSink; - } + // Did we determine that (only) some instructions are unprofitable to sink? + if (Idx < ScanIdx) { + // Okay, some instructions are unprofitable. + ScanIdx = Idx; + InstructionsToSink = InstructionsProfitableToSink; + + // But, that may make other instructions unprofitable, too. + // So, do a backward scan, do any earlier instructions become + // unprofitable? + assert( + !ProfitableToSinkInstruction(LRI) && + "We already know that the last instruction is unprofitable to sink"); ++LRI; --Idx; + while (Idx >= 0) { + // If we detect that an instruction becomes unprofitable to sink, + // all earlier instructions won't be sunk either, + // so preemptively keep InstructionsProfitableToSink in sync. + // FIXME: is this the most performant approach? + for (auto *I : *LRI) + InstructionsProfitableToSink.erase(I); + if (!ProfitableToSinkInstruction(LRI)) { + // Everything starting with this instruction won't be sunk. + ScanIdx = Idx; + InstructionsToSink = InstructionsProfitableToSink; + } + ++LRI; + --Idx; + } } - } - // If no instructions can be sunk, early-return. - if (ScanIdx == 0) - return false; + // If no instructions can be sunk, early-return. + if (ScanIdx == 0) + return false; + } bool Changed = false; if (HaveNonUnconditionalPredecessors) { - // It is always legal to sink common instructions from unconditional - // predecessors. However, if not all predecessors are unconditional, - // this transformation might be pessimizing. So as a rule of thumb, - // don't do it unless we'd sink at least one non-speculatable instruction. - // See https://bugs.llvm.org/show_bug.cgi?id=30244 - LRI.reset(); - int Idx = 0; - bool Profitable = false; - while (Idx < ScanIdx) { - if (!isSafeToSpeculativelyExecute((*LRI)[0])) { - Profitable = true; - break; + if (!followedByDeoptOrUnreachable) { + // It is always legal to sink common instructions from unconditional + // predecessors. However, if not all predecessors are unconditional, + // this transformation might be pessimizing. So as a rule of thumb, + // don't do it unless we'd sink at least one non-speculatable instruction. + // See https://bugs.llvm.org/show_bug.cgi?id=30244 + LRI.reset(); + int Idx = 0; + bool Profitable = false; + while (Idx < ScanIdx) { + if (!isSafeToSpeculativelyExecute((*LRI)[0])) { + Profitable = true; + break; + } + --LRI; + ++Idx; } - --LRI; - ++Idx; + if (!Profitable) + return false; } - if (!Profitable) - return false; LLVM_DEBUG(dbgs() << "SINK: Splitting edge\n"); // We have a conditional edge and we're going to sink some instructions. diff --git a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code-into-unreachable.ll b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code-into-unreachable.ll --- a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code-into-unreachable.ll +++ b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code-into-unreachable.ll @@ -11,22 +11,19 @@ define void @t0(i4 %cond, i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f, i32 %g, i32 %h) { ; CHECK-LABEL: @t0( ; CHECK-NEXT: switch i4 [[COND:%.*]], label [[END:%.*]] [ -; CHECK-NEXT: i4 0, label [[BB0:%.*]] -; CHECK-NEXT: i4 -1, label [[BB0]] +; CHECK-NEXT: i4 0, label [[END_SINK_SPLIT:%.*]] +; CHECK-NEXT: i4 -1, label [[END_SINK_SPLIT]] ; CHECK-NEXT: i4 1, label [[BB1:%.*]] ; CHECK-NEXT: i4 -2, label [[BB1]] ; CHECK-NEXT: ] -; CHECK: bb0: -; CHECK-NEXT: [[V0:%.*]] = add i32 [[A:%.*]], [[B:%.*]] -; CHECK-NEXT: [[V1:%.*]] = add i32 [[V0]], [[C:%.*]] -; CHECK-NEXT: [[V2:%.*]] = add i32 [[D:%.*]], [[E:%.*]] -; CHECK-NEXT: [[R3:%.*]] = add i32 [[V1]], [[V2]] -; CHECK-NEXT: call void @use32(i32 [[R3]]) -; CHECK-NEXT: unreachable ; CHECK: bb1: -; CHECK-NEXT: [[V4:%.*]] = add i32 [[A]], [[B]] -; CHECK-NEXT: [[V5:%.*]] = add i32 [[V4]], [[C]] -; CHECK-NEXT: [[V6:%.*]] = add i32 [[G:%.*]], [[H:%.*]] +; CHECK-NEXT: br label [[END_SINK_SPLIT]] +; CHECK: end.sink.split: +; CHECK-NEXT: [[H_SINK:%.*]] = phi i32 [ [[H:%.*]], [[BB1]] ], [ [[E:%.*]], [[TMP0:%.*]] ], [ [[E]], [[TMP0]] ] +; CHECK-NEXT: [[G_SINK:%.*]] = phi i32 [ [[G:%.*]], [[BB1]] ], [ [[D:%.*]], [[TMP0]] ], [ [[D]], [[TMP0]] ] +; CHECK-NEXT: [[V4:%.*]] = add i32 [[A:%.*]], [[B:%.*]] +; CHECK-NEXT: [[V5:%.*]] = add i32 [[V4]], [[C:%.*]] +; CHECK-NEXT: [[V6:%.*]] = add i32 [[G_SINK]], [[H_SINK]] ; CHECK-NEXT: [[R7:%.*]] = add i32 [[V5]], [[V6]] ; CHECK-NEXT: call void @use32(i32 [[R7]]) ; CHECK-NEXT: unreachable