llvm-reduce: Don't turn switches into returns

[arsenm]

Summary

Re-use one of the existing successors as the new default.
This helps with a future patch to fix handling of unreachable
blocks.

[aeubanks]

llvm/tools/llvm-reduce/deltas/ReduceBasicBlocks.cpp

98–104

do we have a test for this part?

114

if NewDefault was already a successor, doesn't its phis already have values from this block?

[arsenm]

llvm/tools/llvm-reduce/deltas/ReduceBasicBlocks.cpp

98–104

It gets hit, but not sure this is reflected in the direct output of any. Added another case

114

Yes that's where it's getting the value. Phis have to have an incoming operand for each instance

[aeubanks]

llvm/tools/llvm-reduce/deltas/ReduceBasicBlocks.cpp

114

TIL you need redundant entries

[arsenm]

bc405e37ee96092b2b6a0968e777a17da6c9000a

llvm/test/tools/llvm-reduce/remove-bb-switch-default.ll

+; RUN: llvm-reduce -abort-on-invalid-reduction --delta-passes=basic-blocks --test FileCheck --test-arg --check-prefix=CHECK-INTERESTINGNESS0 --test-arg %s --test-arg --input-file %s -o %t.0
+; RUN: FileCheck -check-prefix=RESULT0 %s < %t.0
+
+; RUN: llvm-reduce -abort-on-invalid-reduction --delta-passes=basic-blocks --test FileCheck --test-arg --check-prefix=CHECK-INTERESTINGNESS1 --test-arg %s --test-arg --input-file %s -o %t.1
+; RUN: FileCheck -check-prefix=RESULT1 %s < %t.1
+
+; CHECK-INTERESTINGNESS0: store i32 1,
+; CHECK-INTERESTINGNESS0: store i32 2,
+
+; CHECK-INTERESTINGNESS1: store i32 2,
+
+
+; RESULT0: bb:
+; RESULT0-NEXT: %bb.load = load i32, ptr null, align 4
+; RESULT0-NEXT: store i32 0, ptr null, align 4
+; RESULT0-NEXT: br i1 %arg0, label %bb1, label %bb2
+
+; RESULT0: bb1:
+; RESULT0-NEXT: %bb1.phi = phi i32 [ %bb.load, %bb ], [ %bb2.phi, %bb2 ], [ %bb2.phi, %bb2 ]
+; RESULT0-NEXT: store i32 1, ptr null, align 4
+; RESULT0-NEXT: ret void
+
+; RESULT0: bb2: ; preds = %bb
+; RESULT0-NEXT: %bb2.phi = phi i32 [ %bb.load, %bb ]
+; RESULT0-NEXT: store i32 2, ptr null, align 4
+; RESULT0-NEXT: switch i32 %bb2.phi, label %bb1 [
+; RESULT0-NEXT: i32 0, label %bb1
+; RESULT0-NEXT: ]
+
+
+; RESULT1: bb:
+; RESULT1-NEXT: %bb.load = load i32, ptr null, align 4
+; RESULT1-NEXT: store i32 0, ptr null, align 4
+; RESULT1-NEXT: br label %bb2
+
+; RESULT1: bb2:
+; RESULT1-NEXT: %bb2.phi = phi i32 [ %bb.load, %bb ]
+; RESULT1-NEXT: store i32 2, ptr null, align 4
+; RESULT1-NEXT: ret void
+define void @main(i1 %arg0) {
+bb:
+  %bb.load = load i32, ptr null
+  store i32 0, ptr null
+  br i1 %arg0, label %bb1, label %bb2
+
+bb1:
+  %bb1.phi = phi i32 [%bb.load, %bb], [9, %bb3], [%bb2.phi, %bb2]
+  store i32 1, ptr null
+  ret void
+
+bb2:
+  %bb2.phi = phi i32 [%bb.load, %bb], [%bb3.load, %bb3]
+  store i32 2, ptr null
+  switch i32 %bb2.phi, label %bb3 [
+    i32 0, label %bb1
+    i32 1, label %bb4
+  ]
+
+bb3:
+  %bb3.load = load i32, ptr null
+  store i32 3, ptr null
+  br i1 true, label %bb2, label %bb1
+
+bb4:
+  store i32 4, ptr null
+  ret void
+}

llvm/tools/llvm-reduce/deltas/ReduceBasicBlocks.cpp

 }
 
 /// Removes uninteresting BBs from switch, if the default case ends up being
 /// uninteresting, the switch is replaced with a void return (since it has to be
 /// replace with something)
 static void
 removeUninterestingBBsFromSwitch(SwitchInst &SwInst,
                                  const DenseSet<BasicBlock *> &BBsToKeep) {
-  if (!BBsToKeep.count(SwInst.getDefaultDest())) {
-    auto *FnRetTy = SwInst.getParent()->getParent()->getReturnType();
-    ReturnInst::Create(SwInst.getContext(),
-                       FnRetTy->isVoidTy() ? nullptr : getDefaultValue(FnRetTy),
-                       SwInst.getParent());
-    SwInst.eraseFromParent();
-  } else
-    for (int I = 0, E = SwInst.getNumCases(); I != E; ++I) {
-      auto Case = SwInst.case_begin() + I;
-      if (!BBsToKeep.count(Case->getCaseSuccessor())) {
-        SwInst.removeCase(Case);
-        --I;
-        --E;
-      }
-    }
+
+  if (!BBsToKeep.count(SwInst.getDefaultDest())) {
+    if (SwInst.getNumCases() == 0) {
+      auto *FnRetTy = SwInst.getParent()->getParent()->getReturnType();
+      Value *RetValue =
+          FnRetTy->isVoidTy() ? nullptr : getDefaultValue(FnRetTy);
+      ReturnInst::Create(SwInst.getContext(), RetValue, SwInst.getParent());
+      SwInst.eraseFromParent();
+      return;
+    }
+
+    // Replace the default dest with one of the other cases
+    auto Case = SwInst.case_begin();
+
+    BasicBlock *NewDefault = Case->getCaseSuccessor();
+    SwInst.setDefaultDest(NewDefault);
+
+    for (PHINode &SuccPHI : NewDefault->phis()) {
+      SuccPHI.addIncoming(SuccPHI.getIncomingValueForBlock(SwInst.getParent()),
+                          SwInst.getParent());
+    }
+  }
 }
 
 /// Removes out-of-chunk arguments from functions, and modifies their calls
 /// accordingly. It also removes allocations of out-of-chunk arguments.
 static void extractBasicBlocksFromModule(Oracle &O, Module &Program) {
   DenseSet<BasicBlock *> BBsToKeep, BBsToDelete;
 
   for (auto &F : Program) {
     if (F.empty())
       continue;
 
     // Never try to delete the entry block.
     for (auto &BB : make_range(++F.begin(), F.end())) {
       if (O.shouldKeep())
         BBsToKeep.insert(&BB);
       else
         BBsToDelete.insert(&BB);
     }
   }
 
-  // Remove out-of-chunk BB from successor phi nodes
-  for (auto &BB : BBsToDelete) {
-    for (auto *Succ : successors(BB))
-      Succ->removePredecessor(BB, /*KeepOneInputPHIs=*/true);
-  }
-
   // Replace terminators that reference out-of-chunk BBs
   for (auto &F : Program)
     for (auto &BB : F) {
       if (auto *SwInst = dyn_cast<SwitchInst>(BB.getTerminator()))
         removeUninterestingBBsFromSwitch(*SwInst, BBsToKeep);
       else
         replaceBranchTerminator(BB, BBsToKeep);
     }
 
+  // Remove out-of-chunk BB from successor phi nodes
+  for (auto &BB : BBsToDelete) {
+    for (auto *Succ : successors(BB))
+      Succ->removePredecessor(BB, /*KeepOneInputPHIs=*/true);
+  }
+
   // Replace out-of-chunk switch uses
   for (auto &BB : BBsToDelete) {
     // Instructions might be referenced in other BBs
     for (auto &I : *BB)
       I.replaceAllUsesWith(getDefaultValue(I.getType()));
     // Should not be completely removing the body of a function.
     assert(BB->getParent()->size() > 1);
     BB->eraseFromParent();
   }
 }
 
 void llvm::reduceBasicBlocksDeltaPass(TestRunner &Test) {
   runDeltaPass(Test, extractBasicBlocksFromModule, "Reducing Basic Blocks");
 }