[llvm] 5d4f37e - [NFCI][SimplifyCFG] Rewrite `createUnreachableSwitchDefault()`

[Roman Lebedev via llvm-commits]

On Fri, Aug 20, 2021 at 7:51 PM Roman Lebedev <lebedev.ri at gmail.com> wrote:
>
> Yep, i think it is important to not even breathe in the direction of
> other successors, especially if some other case happened to
> share the destination with the one from switch's default.
Err, i obviously meant predecessors, not successors.

Roman

> On Fri, Aug 20, 2021 at 7:45 PM Craig Topper <craig.topper at gmail.com> wrote:
> >
> > Nevermind, you're not deleting the original block just disconnecting it.
> >
> > ~Craig
> >
> >
> > On Fri, Aug 20, 2021 at 9:40 AM Craig Topper <craig.topper at gmail.com> wrote:
> >>
> >> Is it possible the unreachable block has instructions that are used by its original successors?
> >>
> >> ~Craig
> >>
> >>
> >> On Fri, Aug 20, 2021 at 3:28 AM Roman Lebedev via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >>>
> >>>
> >>> Author: Roman Lebedev
> >>> Date: 2021-08-20T13:28:08+03:00
> >>> New Revision: 5d4f37e895487408e61498ec42e545ec5f778b5b
> >>>
> >>> URL: https://github.com/llvm/llvm-project/commit/5d4f37e895487408e61498ec42e545ec5f778b5b
> >>> DIFF: https://github.com/llvm/llvm-project/commit/5d4f37e895487408e61498ec42e545ec5f778b5b.diff
> >>>
> >>> LOG: [NFCI][SimplifyCFG] Rewrite `createUnreachableSwitchDefault()`
> >>>
> >>> The only thing that function should do as per it's semantic,
> >>> is to ensure that the switch's default is a block consisting only of
> >>> an `unreachable` terminator.
> >>>
> >>> So let's just create such a block and update switch's default
> >>> to point to it. There should be no need for all this weird dance
> >>> around predecessors/successors.
> >>>
> >>> Added:
> >>>
> >>>
> >>> Modified:
> >>>     llvm/lib/Transforms/Utils/SimplifyCFG.cpp
> >>>     llvm/test/Transforms/SimplifyCFG/switch-dead-default.ll
> >>>
> >>> Removed:
> >>>
> >>>
> >>>
> >>> ################################################################################
> >>> diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
> >>> index 847fdd760d2fe..ba3129f5581fe 100644
> >>> --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
> >>> +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
> >>> @@ -4747,23 +4747,20 @@ static void createUnreachableSwitchDefault(SwitchInst *Switch,
> >>>                                             DomTreeUpdater *DTU) {
> >>>    LLVM_DEBUG(dbgs() << "SimplifyCFG: switch default is dead.\n");
> >>>    auto *BB = Switch->getParent();
> >>> -  BasicBlock *NewDefaultBlock = SplitBlockPredecessors(
> >>> -      Switch->getDefaultDest(), Switch->getParent(), "", DTU);
> >>>    auto *OrigDefaultBlock = Switch->getDefaultDest();
> >>> +  OrigDefaultBlock->removePredecessor(BB);
> >>> +  BasicBlock *NewDefaultBlock = BasicBlock::Create(
> >>> +      BB->getContext(), BB->getName() + ".unreachabledefault", BB->getParent(),
> >>> +      OrigDefaultBlock);
> >>> +  new UnreachableInst(Switch->getContext(), NewDefaultBlock);
> >>>    Switch->setDefaultDest(&*NewDefaultBlock);
> >>> -  if (DTU)
> >>> -    DTU->applyUpdates({{DominatorTree::Insert, BB, &*NewDefaultBlock},
> >>> -                       {DominatorTree::Delete, BB, OrigDefaultBlock}});
> >>> -  SplitBlock(&*NewDefaultBlock, &NewDefaultBlock->front(), DTU);
> >>> -  SmallVector<DominatorTree::UpdateType, 2> Updates;
> >>> -  if (DTU)
> >>> -    for (auto *Successor : successors(NewDefaultBlock))
> >>> -      Updates.push_back({DominatorTree::Delete, NewDefaultBlock, Successor});
> >>> -  auto *NewTerminator = NewDefaultBlock->getTerminator();
> >>> -  new UnreachableInst(Switch->getContext(), NewTerminator);
> >>> -  EraseTerminatorAndDCECond(NewTerminator);
> >>> -  if (DTU)
> >>> +  if (DTU) {
> >>> +    SmallVector<DominatorTree::UpdateType, 2> Updates;
> >>> +    Updates.push_back({DominatorTree::Insert, BB, &*NewDefaultBlock});
> >>> +    if (!is_contained(successors(BB), OrigDefaultBlock))
> >>> +      Updates.push_back({DominatorTree::Delete, BB, &*OrigDefaultBlock});
> >>>      DTU->applyUpdates(Updates);
> >>> +  }
> >>>  }
> >>>
> >>>  /// Turn a switch with two reachable destinations into an integer range
> >>>
> >>> diff  --git a/llvm/test/Transforms/SimplifyCFG/switch-dead-default.ll b/llvm/test/Transforms/SimplifyCFG/switch-dead-default.ll
> >>> index b42024ee48995..154ecb2166310 100644
> >>> --- a/llvm/test/Transforms/SimplifyCFG/switch-dead-default.ll
> >>> +++ b/llvm/test/Transforms/SimplifyCFG/switch-dead-default.ll
> >>> @@ -33,7 +33,7 @@ default:
> >>>
> >>>  define void @test2(i2 %a) {
> >>>  ; CHECK-LABEL: @test2(
> >>> -; CHECK-NEXT:    switch i2 [[A:%.*]], label [[DEFAULT1:%.*]] [
> >>> +; CHECK-NEXT:    switch i2 [[A:%.*]], label [[DOTUNREACHABLEDEFAULT:%.*]] [
> >>>  ; CHECK-NEXT:    i2 0, label [[CASE0:%.*]]
> >>>  ; CHECK-NEXT:    i2 1, label [[CASE1:%.*]]
> >>>  ; CHECK-NEXT:    i2 -2, label [[CASE2:%.*]]
> >>> @@ -53,7 +53,7 @@ define void @test2(i2 %a) {
> >>>  ; CHECK:       case3:
> >>>  ; CHECK-NEXT:    call void @foo(i32 3)
> >>>  ; CHECK-NEXT:    br label [[COMMON_RET]]
> >>> -; CHECK:       default1:
> >>> +; CHECK:       .unreachabledefault:
> >>>  ; CHECK-NEXT:    unreachable
> >>>  ;
> >>>    switch i2 %a, label %default [i2 0, label %case0
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> llvm-commits mailing list
> >>> llvm-commits at lists.llvm.org
> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits