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

[Roman Lebedev via llvm-commits]

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.


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