Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LowerSwitch should remove all dead blocks introduced by switch processing #51725

Closed
Nuullll opened this issue Nov 3, 2021 · 6 comments
Closed
Assignees
Labels
bugzilla Issues migrated from bugzilla wontfix Issue is real, but we can't or won't fix it. Not invalid

Comments

@Nuullll
Copy link
Contributor

Nuullll commented Nov 3, 2021

Bugzilla Link 52383
Resolution WONTFIX
Resolved on Nov 16, 2021 00:00
Version trunk
OS All

Extended Description

Consider the following ll:

define i32 @​f(i32 %c) {
chk65:
  %cmp = icmp sgt i32 %c, 65
  br i1 %cmp, label %return, label %chk0

chk0:
  %cmp1 = icmp slt i32 %c, 0
  br i1 %cmp, label %return, label %bb_if

bb_if:
  %ashr.val = ashr exact i32 %c, 2
  %cmp2 = icmp sgt i32 %ashr.val, 14
  br i1 %cmp2, label %bb_switch, label %return

bb_switch:
; %ashr.val must be 15 or 16 here (analyzed by LazyValueInfo)
  switch i32 %ashr.val, label %out_of_switch_range [
    i32 15, label %return
    i32 16, label %another_ret
  ]

; unreachable default case
out_of_switch_range:
  br label %successor_of_unreachable

; unreachable successors
successor_of_unreachable:
  br label %return

another_ret:
  br label %return

return:
  %retval = phi i32 [-1, %chk0], [-1, %chk65], [-1, %bb_if], [42, %bb_switch], [42, %another_ret], [-42, %successor_of_unreachable]
  ret i32 %retval
}

Both "out_of_switch_range" and "successor_of_unreachable" should be deleted as they become dead after switch lowering.

But LowerSwitch only deletes the "out_of_switch_range" -- it didn't delete dead block's dominatees which also become unreachable.

@Nuullll
Copy link
Contributor Author

Nuullll commented Nov 3, 2021

assigned to @Nuullll

@Nuullll
Copy link
Contributor Author

Nuullll commented Nov 3, 2021

opt -S -lowerswitch t.ll

define i32 @​f(i32 %c) {
chk65:
%cmp = icmp sgt i32 %c, 65
br i1 %cmp, label %return, label %chk0

chk0: ; preds = %chk65
%cmp1 = icmp slt i32 %c, 0
br i1 %cmp, label %return, label %bb_if

bb_if: ; preds = %chk0
%ashr.val = ashr exact i32 %c, 2
%cmp2 = icmp sgt i32 %ashr.val, 14
br i1 %cmp2, label %bb_switch, label %return

bb_switch: ; preds = %bb_if
br label %LeafBlock

LeafBlock: ; preds = %bb_switch
%SwitchLeaf = icmp eq i32 %ashr.val, 16
br i1 %SwitchLeaf, label %another_ret, label %NewDefault

successor_of_unreachable: ; No predecessors!
br label %return

another_ret: ; preds = %LeafBlock
br label %return

NewDefault: ; preds = %LeafBlock
br label %return

return: ; preds = %NewDefault, %another_ret, %successor_of_unreachable, %bb_if, %chk0, %chk65
%retval = phi i32 [ -1, %chk0 ], [ -1, %chk65 ], [ -1, %bb_if ], [ 42, %NewDefault ], [ 42, %another_ret ], [ -42, %successor_of_unreachable ]
ret i32 %retval
}

@Nuullll
Copy link
Contributor Author

Nuullll commented Nov 3, 2021

Another simpler example: there's only one non-default case of the switch inst.

define i32 @​g(i32 %c) {
bb_switch:
  switch i32 %c, label %out_of_switch_range [
    i32 16, label %return
  ]

out_of_switch_range:
  unreachable

return:
  ret i32 %c
}

opt -S -lowerswitch t.ll

define i32 @​g(i32 %c) {
bb_switch:
  br label %return

out_of_switch_range:                              ; No predecessors!
  unreachable

return:                                           ; preds = %bb_switch
  ret i32 %c
}

The default block isn't deleted.

@Nuullll
Copy link
Contributor Author

Nuullll commented Nov 3, 2021

I think we need to use Dominator Tree to be able to delete new dead blocks accurately.

@Nuullll
Copy link
Contributor Author

Nuullll commented Nov 3, 2021

@Nuullll
Copy link
Contributor Author

Nuullll commented Nov 16, 2021

We should delegate the dead block elimination to subsequent passes (such as DCE), to avoid unnecessary expense on compilation time.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
@Quuxplusone Quuxplusone added the wontfix Issue is real, but we can't or won't fix it. Not invalid label Jan 20, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla wontfix Issue is real, but we can't or won't fix it. Not invalid
Projects
None yet
Development

No branches or pull requests

2 participants