Skip to content

[SCCP] Check whether the default case is reachable #76295

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

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Dec 23, 2023

This patch eliminates unreachable default cases using range information.
Fixes #76085.

@llvmbot
Copy link
Member

llvmbot commented Dec 23, 2023

@llvm/pr-subscribers-function-specialization

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch eliminates unreachable default cases using range information.
Fixes #76085.


Full diff: https://github.com/llvm/llvm-project/pull/76295.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/SCCPSolver.cpp (+7-3)
  • (modified) llvm/test/Transforms/SCCP/switch.ll (+79-12)
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index ab95698abc4399..3dc6016a0a373f 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -310,6 +310,7 @@ bool SCCPSolver::removeNonFeasibleEdges(BasicBlock *BB, DomTreeUpdater &DTU,
         new UnreachableInst(DefaultDest->getContext(), NewUnreachableBB);
       }
 
+      DefaultDest->removePredecessor(BB);
       SI->setDefaultDest(NewUnreachableBB);
       Updates.push_back({DominatorTree::Delete, BB, DefaultDest});
       Updates.push_back({DominatorTree::Insert, BB, NewUnreachableBB});
@@ -1063,14 +1064,17 @@ void SCCPInstVisitor::getFeasibleSuccessors(Instruction &TI,
     // is ready.
     if (SCValue.isConstantRange(/*UndefAllowed=*/false)) {
       const ConstantRange &Range = SCValue.getConstantRange();
+      unsigned ReachableCaseCount = 0;
       for (const auto &Case : SI->cases()) {
         const APInt &CaseValue = Case.getCaseValue()->getValue();
-        if (Range.contains(CaseValue))
+        if (Range.contains(CaseValue)) {
           Succs[Case.getSuccessorIndex()] = true;
+          ++ReachableCaseCount;
+        }
       }
 
-      // TODO: Determine whether default case is reachable.
-      Succs[SI->case_default()->getSuccessorIndex()] = true;
+      Succs[SI->case_default()->getSuccessorIndex()] =
+          Range.isSizeLargerThan(ReachableCaseCount);
       return;
     }
 
diff --git a/llvm/test/Transforms/SCCP/switch.ll b/llvm/test/Transforms/SCCP/switch.ll
index 19e72217fd03bf..306f0eebf2b408 100644
--- a/llvm/test/Transforms/SCCP/switch.ll
+++ b/llvm/test/Transforms/SCCP/switch.ll
@@ -4,6 +4,8 @@
 ; Make sure we always consider the default edge executable for a switch
 ; with no cases.
 declare void @foo()
+declare i32 @g(i32)
+
 define void @test1() {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:    switch i32 undef, label [[D:%.*]] [
@@ -115,17 +117,16 @@ switch.1:
   ret i32 %phi
 }
 
-; TODO: Determine that the default destination is dead.
 define i32 @test_local_range(ptr %p) {
 ; CHECK-LABEL: @test_local_range(
 ; CHECK-NEXT:    [[X:%.*]] = load i32, ptr [[P:%.*]], align 4, !range [[RNG0]]
-; CHECK-NEXT:    switch i32 [[X]], label [[SWITCH_DEFAULT:%.*]] [
+; CHECK-NEXT:    switch i32 [[X]], label [[DEFAULT_UNREACHABLE:%.*]] [
 ; CHECK-NEXT:    i32 0, label [[SWITCH_0:%.*]]
 ; CHECK-NEXT:    i32 1, label [[SWITCH_1:%.*]]
 ; CHECK-NEXT:    i32 2, label [[SWITCH_2:%.*]]
 ; CHECK-NEXT:    ]
-; CHECK:       switch.default:
-; CHECK-NEXT:    ret i32 -1
+; CHECK:       default.unreachable:
+; CHECK-NEXT:    unreachable
 ; CHECK:       switch.0:
 ; CHECK-NEXT:    ret i32 0
 ; CHECK:       switch.1:
@@ -161,14 +162,14 @@ switch.3:
 define i32 @test_duplicate_successors(ptr %p) {
 ; CHECK-LABEL: @test_duplicate_successors(
 ; CHECK-NEXT:    [[X:%.*]] = load i32, ptr [[P:%.*]], align 4, !range [[RNG0]]
-; CHECK-NEXT:    switch i32 [[X]], label [[SWITCH_DEFAULT:%.*]] [
+; CHECK-NEXT:    switch i32 [[X]], label [[DEFAULT_UNREACHABLE:%.*]] [
 ; CHECK-NEXT:    i32 0, label [[SWITCH_0:%.*]]
 ; CHECK-NEXT:    i32 1, label [[SWITCH_0]]
 ; CHECK-NEXT:    i32 2, label [[SWITCH_1:%.*]]
 ; CHECK-NEXT:    i32 3, label [[SWITCH_1]]
 ; CHECK-NEXT:    ]
-; CHECK:       switch.default:
-; CHECK-NEXT:    ret i32 -1
+; CHECK:       default.unreachable:
+; CHECK-NEXT:    unreachable
 ; CHECK:       switch.0:
 ; CHECK-NEXT:    ret i32 0
 ; CHECK:       switch.1:
@@ -201,13 +202,13 @@ switch.2:
 ; range information.
 define internal i32 @test_ip_range(i32 %x) {
 ; CHECK-LABEL: @test_ip_range(
-; CHECK-NEXT:    switch i32 [[X:%.*]], label [[SWITCH_DEFAULT:%.*]] [
+; CHECK-NEXT:    switch i32 [[X:%.*]], label [[DEFAULT_UNREACHABLE:%.*]] [
 ; CHECK-NEXT:    i32 3, label [[SWITCH_3:%.*]]
 ; CHECK-NEXT:    i32 1, label [[SWITCH_1:%.*]]
 ; CHECK-NEXT:    i32 2, label [[SWITCH_2:%.*]]
 ; CHECK-NEXT:    ], !prof [[PROF1:![0-9]+]]
-; CHECK:       switch.default:
-; CHECK-NEXT:    ret i32 -1
+; CHECK:       default.unreachable:
+; CHECK-NEXT:    unreachable
 ; CHECK:       switch.1:
 ; CHECK-NEXT:    ret i32 1
 ; CHECK:       switch.2:
@@ -240,8 +241,8 @@ switch.3:
 
 define void @call_test_ip_range() {
 ; CHECK-LABEL: @call_test_ip_range(
-; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @test_ip_range(i32 1)
-; CHECK-NEXT:    [[TMP2:%.*]] = call i32 @test_ip_range(i32 3)
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @test_ip_range(i32 1), !range [[RNG2:![0-9]+]]
+; CHECK-NEXT:    [[TMP2:%.*]] = call i32 @test_ip_range(i32 3), !range [[RNG2]]
 ; CHECK-NEXT:    ret void
 ;
   call i32 @test_ip_range(i32 1)
@@ -301,6 +302,72 @@ end.2:
   ret i32 20
 }
 
+define i32 @test_default_unreachable_by_dom_cond(i32 %x) {
+; CHECK-LABEL: @test_default_unreachable_by_dom_cond(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[OR_COND:%.*]] = icmp ult i32 [[X:%.*]], 4
+; CHECK-NEXT:    br i1 [[OR_COND]], label [[IF_THEN:%.*]], label [[RETURN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    switch i32 [[X]], label [[DEFAULT_UNREACHABLE:%.*]] [
+; CHECK-NEXT:      i32 0, label [[SW_BB:%.*]]
+; CHECK-NEXT:      i32 1, label [[SW_BB2:%.*]]
+; CHECK-NEXT:      i32 2, label [[SW_BB4:%.*]]
+; CHECK-NEXT:      i32 3, label [[SW_BB6:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       sw.bb:
+; CHECK-NEXT:    [[CALL:%.*]] = tail call i32 @g(i32 2)
+; CHECK-NEXT:    br label [[RETURN]]
+; CHECK:       sw.bb2:
+; CHECK-NEXT:    [[CALL3:%.*]] = tail call i32 @g(i32 3)
+; CHECK-NEXT:    br label [[RETURN]]
+; CHECK:       sw.bb4:
+; CHECK-NEXT:    [[CALL5:%.*]] = tail call i32 @g(i32 4)
+; CHECK-NEXT:    br label [[RETURN]]
+; CHECK:       sw.bb6:
+; CHECK-NEXT:    [[CALL7:%.*]] = tail call i32 @g(i32 5)
+; CHECK-NEXT:    br label [[RETURN]]
+; CHECK:       default.unreachable:
+; CHECK-NEXT:    unreachable
+; CHECK:       return:
+; CHECK-NEXT:    [[RETVAL_0:%.*]] = phi i32 [ [[CALL7]], [[SW_BB6]] ], [ [[CALL5]], [[SW_BB4]] ], [ [[CALL3]], [[SW_BB2]] ], [ [[CALL]], [[SW_BB]] ], [ -23, [[ENTRY:%.*]] ]
+; CHECK-NEXT:    ret i32 [[RETVAL_0]]
+;
+entry:
+  %or.cond = icmp ult i32 %x, 4
+  br i1 %or.cond, label %if.then, label %return
+
+if.then:
+  switch i32 %x, label %sw.epilog [
+  i32 0, label %sw.bb
+  i32 1, label %sw.bb2
+  i32 2, label %sw.bb4
+  i32 3, label %sw.bb6
+  ]
+
+sw.bb:
+  %call = tail call i32 @g(i32 2)
+  br label %return
+
+sw.bb2:
+  %call3 = tail call i32 @g(i32 3)
+  br label %return
+
+sw.bb4:
+  %call5 = tail call i32 @g(i32 4)
+  br label %return
+
+sw.bb6:
+  %call7 = tail call i32 @g(i32 5)
+  br label %return
+
+sw.epilog:
+  call void @foo()
+  br label %return
+
+return:
+  %retval.0 = phi i32 [ %call7, %sw.bb6 ], [ %call5, %sw.bb4 ], [ %call3, %sw.bb2 ], [ %call, %sw.bb ], [ -23, %sw.epilog ], [ -23, %entry ]
+  ret i32 %retval.0
+}
 
 declare void @llvm.assume(i1)
 

@llvmbot
Copy link
Member

llvmbot commented Dec 23, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch eliminates unreachable default cases using range information.
Fixes #76085.


Full diff: https://github.com/llvm/llvm-project/pull/76295.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/SCCPSolver.cpp (+7-3)
  • (modified) llvm/test/Transforms/SCCP/switch.ll (+79-12)
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index ab95698abc4399..3dc6016a0a373f 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -310,6 +310,7 @@ bool SCCPSolver::removeNonFeasibleEdges(BasicBlock *BB, DomTreeUpdater &DTU,
         new UnreachableInst(DefaultDest->getContext(), NewUnreachableBB);
       }
 
+      DefaultDest->removePredecessor(BB);
       SI->setDefaultDest(NewUnreachableBB);
       Updates.push_back({DominatorTree::Delete, BB, DefaultDest});
       Updates.push_back({DominatorTree::Insert, BB, NewUnreachableBB});
@@ -1063,14 +1064,17 @@ void SCCPInstVisitor::getFeasibleSuccessors(Instruction &TI,
     // is ready.
     if (SCValue.isConstantRange(/*UndefAllowed=*/false)) {
       const ConstantRange &Range = SCValue.getConstantRange();
+      unsigned ReachableCaseCount = 0;
       for (const auto &Case : SI->cases()) {
         const APInt &CaseValue = Case.getCaseValue()->getValue();
-        if (Range.contains(CaseValue))
+        if (Range.contains(CaseValue)) {
           Succs[Case.getSuccessorIndex()] = true;
+          ++ReachableCaseCount;
+        }
       }
 
-      // TODO: Determine whether default case is reachable.
-      Succs[SI->case_default()->getSuccessorIndex()] = true;
+      Succs[SI->case_default()->getSuccessorIndex()] =
+          Range.isSizeLargerThan(ReachableCaseCount);
       return;
     }
 
diff --git a/llvm/test/Transforms/SCCP/switch.ll b/llvm/test/Transforms/SCCP/switch.ll
index 19e72217fd03bf..306f0eebf2b408 100644
--- a/llvm/test/Transforms/SCCP/switch.ll
+++ b/llvm/test/Transforms/SCCP/switch.ll
@@ -4,6 +4,8 @@
 ; Make sure we always consider the default edge executable for a switch
 ; with no cases.
 declare void @foo()
+declare i32 @g(i32)
+
 define void @test1() {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:    switch i32 undef, label [[D:%.*]] [
@@ -115,17 +117,16 @@ switch.1:
   ret i32 %phi
 }
 
-; TODO: Determine that the default destination is dead.
 define i32 @test_local_range(ptr %p) {
 ; CHECK-LABEL: @test_local_range(
 ; CHECK-NEXT:    [[X:%.*]] = load i32, ptr [[P:%.*]], align 4, !range [[RNG0]]
-; CHECK-NEXT:    switch i32 [[X]], label [[SWITCH_DEFAULT:%.*]] [
+; CHECK-NEXT:    switch i32 [[X]], label [[DEFAULT_UNREACHABLE:%.*]] [
 ; CHECK-NEXT:    i32 0, label [[SWITCH_0:%.*]]
 ; CHECK-NEXT:    i32 1, label [[SWITCH_1:%.*]]
 ; CHECK-NEXT:    i32 2, label [[SWITCH_2:%.*]]
 ; CHECK-NEXT:    ]
-; CHECK:       switch.default:
-; CHECK-NEXT:    ret i32 -1
+; CHECK:       default.unreachable:
+; CHECK-NEXT:    unreachable
 ; CHECK:       switch.0:
 ; CHECK-NEXT:    ret i32 0
 ; CHECK:       switch.1:
@@ -161,14 +162,14 @@ switch.3:
 define i32 @test_duplicate_successors(ptr %p) {
 ; CHECK-LABEL: @test_duplicate_successors(
 ; CHECK-NEXT:    [[X:%.*]] = load i32, ptr [[P:%.*]], align 4, !range [[RNG0]]
-; CHECK-NEXT:    switch i32 [[X]], label [[SWITCH_DEFAULT:%.*]] [
+; CHECK-NEXT:    switch i32 [[X]], label [[DEFAULT_UNREACHABLE:%.*]] [
 ; CHECK-NEXT:    i32 0, label [[SWITCH_0:%.*]]
 ; CHECK-NEXT:    i32 1, label [[SWITCH_0]]
 ; CHECK-NEXT:    i32 2, label [[SWITCH_1:%.*]]
 ; CHECK-NEXT:    i32 3, label [[SWITCH_1]]
 ; CHECK-NEXT:    ]
-; CHECK:       switch.default:
-; CHECK-NEXT:    ret i32 -1
+; CHECK:       default.unreachable:
+; CHECK-NEXT:    unreachable
 ; CHECK:       switch.0:
 ; CHECK-NEXT:    ret i32 0
 ; CHECK:       switch.1:
@@ -201,13 +202,13 @@ switch.2:
 ; range information.
 define internal i32 @test_ip_range(i32 %x) {
 ; CHECK-LABEL: @test_ip_range(
-; CHECK-NEXT:    switch i32 [[X:%.*]], label [[SWITCH_DEFAULT:%.*]] [
+; CHECK-NEXT:    switch i32 [[X:%.*]], label [[DEFAULT_UNREACHABLE:%.*]] [
 ; CHECK-NEXT:    i32 3, label [[SWITCH_3:%.*]]
 ; CHECK-NEXT:    i32 1, label [[SWITCH_1:%.*]]
 ; CHECK-NEXT:    i32 2, label [[SWITCH_2:%.*]]
 ; CHECK-NEXT:    ], !prof [[PROF1:![0-9]+]]
-; CHECK:       switch.default:
-; CHECK-NEXT:    ret i32 -1
+; CHECK:       default.unreachable:
+; CHECK-NEXT:    unreachable
 ; CHECK:       switch.1:
 ; CHECK-NEXT:    ret i32 1
 ; CHECK:       switch.2:
@@ -240,8 +241,8 @@ switch.3:
 
 define void @call_test_ip_range() {
 ; CHECK-LABEL: @call_test_ip_range(
-; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @test_ip_range(i32 1)
-; CHECK-NEXT:    [[TMP2:%.*]] = call i32 @test_ip_range(i32 3)
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @test_ip_range(i32 1), !range [[RNG2:![0-9]+]]
+; CHECK-NEXT:    [[TMP2:%.*]] = call i32 @test_ip_range(i32 3), !range [[RNG2]]
 ; CHECK-NEXT:    ret void
 ;
   call i32 @test_ip_range(i32 1)
@@ -301,6 +302,72 @@ end.2:
   ret i32 20
 }
 
+define i32 @test_default_unreachable_by_dom_cond(i32 %x) {
+; CHECK-LABEL: @test_default_unreachable_by_dom_cond(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[OR_COND:%.*]] = icmp ult i32 [[X:%.*]], 4
+; CHECK-NEXT:    br i1 [[OR_COND]], label [[IF_THEN:%.*]], label [[RETURN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    switch i32 [[X]], label [[DEFAULT_UNREACHABLE:%.*]] [
+; CHECK-NEXT:      i32 0, label [[SW_BB:%.*]]
+; CHECK-NEXT:      i32 1, label [[SW_BB2:%.*]]
+; CHECK-NEXT:      i32 2, label [[SW_BB4:%.*]]
+; CHECK-NEXT:      i32 3, label [[SW_BB6:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       sw.bb:
+; CHECK-NEXT:    [[CALL:%.*]] = tail call i32 @g(i32 2)
+; CHECK-NEXT:    br label [[RETURN]]
+; CHECK:       sw.bb2:
+; CHECK-NEXT:    [[CALL3:%.*]] = tail call i32 @g(i32 3)
+; CHECK-NEXT:    br label [[RETURN]]
+; CHECK:       sw.bb4:
+; CHECK-NEXT:    [[CALL5:%.*]] = tail call i32 @g(i32 4)
+; CHECK-NEXT:    br label [[RETURN]]
+; CHECK:       sw.bb6:
+; CHECK-NEXT:    [[CALL7:%.*]] = tail call i32 @g(i32 5)
+; CHECK-NEXT:    br label [[RETURN]]
+; CHECK:       default.unreachable:
+; CHECK-NEXT:    unreachable
+; CHECK:       return:
+; CHECK-NEXT:    [[RETVAL_0:%.*]] = phi i32 [ [[CALL7]], [[SW_BB6]] ], [ [[CALL5]], [[SW_BB4]] ], [ [[CALL3]], [[SW_BB2]] ], [ [[CALL]], [[SW_BB]] ], [ -23, [[ENTRY:%.*]] ]
+; CHECK-NEXT:    ret i32 [[RETVAL_0]]
+;
+entry:
+  %or.cond = icmp ult i32 %x, 4
+  br i1 %or.cond, label %if.then, label %return
+
+if.then:
+  switch i32 %x, label %sw.epilog [
+  i32 0, label %sw.bb
+  i32 1, label %sw.bb2
+  i32 2, label %sw.bb4
+  i32 3, label %sw.bb6
+  ]
+
+sw.bb:
+  %call = tail call i32 @g(i32 2)
+  br label %return
+
+sw.bb2:
+  %call3 = tail call i32 @g(i32 3)
+  br label %return
+
+sw.bb4:
+  %call5 = tail call i32 @g(i32 4)
+  br label %return
+
+sw.bb6:
+  %call7 = tail call i32 @g(i32 5)
+  br label %return
+
+sw.epilog:
+  call void @foo()
+  br label %return
+
+return:
+  %retval.0 = phi i32 [ %call7, %sw.bb6 ], [ %call5, %sw.bb4 ], [ %call3, %sw.bb2 ], [ %call, %sw.bb ], [ -23, %sw.epilog ], [ -23, %entry ]
+  ret i32 %retval.0
+}
 
 declare void @llvm.assume(i1)
 

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Dec 23, 2023
@dtcxzyw
Copy link
Member Author

dtcxzyw commented Dec 30, 2023

Ping.

@nikic
Copy link
Contributor

nikic commented Dec 30, 2023

What happens if the default is already unreachable? Will we replace it with a new unreachable block? It would be good to skip the update in that case.

@nikic
Copy link
Contributor

nikic commented Dec 30, 2023

What happens if the default is already unreachable? Will we replace it with a new unreachable block? It would be good to skip the update in that case.

Hm, I guess this isn't so simple because we'll mark the block as dead and remove it.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikic
Copy link
Contributor

nikic commented Dec 30, 2023

What happens if the default is already unreachable? Will we replace it with a new unreachable block? It would be good to skip the update in that case.

Hm, I guess this isn't so simple because we'll mark the block as dead and remove it.

It may still be worth doing this to avoid spurious invalidation of CFG analyses, but it's not really related to this patch...

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Jan 7, 2024
@dtcxzyw dtcxzyw force-pushed the sccp-reachable-default branch from 6238a96 to c12b109 Compare January 8, 2024 11:54
@dtcxzyw dtcxzyw merged commit d218092 into llvm:main Jan 8, 2024
@dtcxzyw dtcxzyw deleted the sccp-reachable-default branch January 8, 2024 12:08
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
This patch eliminates unreachable default cases using range information.
Fixes llvm#76085.
hiraditya added a commit to hiraditya/llvm-project that referenced this pull request Aug 7, 2024
hiraditya added a commit to hiraditya/llvm-project that referenced this pull request Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch default destination not eliminated despite known value range
3 participants