[PATCH] D113497: [IPSCCP] Support unfeasible default dests for switch.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: nikic, efriedma.
Herald added a subscriber: hiraditya.
fhahn requested review of this revision.
Herald added a project: LLVM.

At the moment, unfeasible default destinations are not handled properly
in removeNonFeasibleEdges. So far, only unfeasible cases are removed,
but later code expects unreachable blocks to have no predecessors.

This is causing the crash reported in PR49573.

If the default destination is unfeasible it won't be executed. So it
should be safe to replace it with any feasible successor.

Note that at the moment this only is relevant for cases where
resolvedUndefsIn marks the first case as executable. Regular switch
handling has a FIXME/TODO to support determining whether the default
case is feasible or not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113497

Files:
  llvm/lib/Transforms/Scalar/SCCP.cpp
  llvm/test/Transforms/SCCP/switch-constantfold-crash.ll


Index: llvm/test/Transforms/SCCP/switch-constantfold-crash.ll
===================================================================
--- llvm/test/Transforms/SCCP/switch-constantfold-crash.ll
+++ llvm/test/Transforms/SCCP/switch-constantfold-crash.ll
@@ -90,3 +90,38 @@
 bb4:                                              ; preds = %bb2, %bb2, %bb2
   unreachable
 }
+
+; Test case from PR49573. %default.bb is unfeasible. Make sure it gets replaced
+; by a different feasible successor
+define void @pr49573_main() {
+  %tgt = call i16 @pr49573_fn()
+  switch i16 %tgt, label %default.bb [
+    i16 0, label %case.0
+    i16 1, label %case.1
+    i16 2, label %case.2
+  ]
+
+case.0:
+  unreachable
+
+default.bb:
+  ret void
+
+case.1:
+  ret void
+
+case.2:
+  ret void
+
+}
+
+define internal i16 @pr49573_fn() {
+entry:
+  br i1 undef, label %then, label %else
+
+then:
+  ret i16 0
+
+else:
+  ret i16 2
+}
Index: llvm/lib/Transforms/Scalar/SCCP.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/SCCP.cpp
+++ llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -386,6 +386,24 @@
   } else if (FeasibleSuccessors.size() > 1) {
     SwitchInstProfUpdateWrapper SI(*cast<SwitchInst>(TI));
     SmallVector<DominatorTree::UpdateType, 8> Updates;
+
+    // If the default destination is unfeasible it will never be taken. Replace
+    // it with a feasible successor to remove the edge to the original default
+    // destination.
+    BasicBlock *DefaultDest = SI->getDefaultDest();
+    if (!FeasibleSuccessors.contains(DefaultDest)) {
+      BasicBlock *FirstFeasible = nullptr;
+      for (BasicBlock *Succ : successors(BB)) {
+        if (Solver.isEdgeFeasible(BB, Succ)) {
+          FirstFeasible = Succ;
+          break;
+        }
+      }
+      assert(FirstFeasible && "at least one successor must be feasible");
+      SI->setDefaultDest(FirstFeasible);
+      Updates.push_back({DominatorTree::Delete, BB, DefaultDest});
+    }
+
     for (auto CI = SI->case_begin(); CI != SI->case_end();) {
       if (FeasibleSuccessors.contains(CI->getCaseSuccessor())) {
         ++CI;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113497.385889.patch
Type: text/x-patch
Size: 2125 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211109/b1cf22f4/attachment.bin>