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

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

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-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t
+; RUN: FileCheck %s < %t
+
+; CHECK-INTERESTINGNESS: store i32 1,
+; CHECK-INTERESTINGNESS: store i32 2,
+
+
+; CHECK: bb:
+; CHECK-NEXT: %bb.load = load i32, ptr null, align 4
+; CHECK-NEXT: store i32 0, ptr null, align 4
+; CHECK-NEXT: br i1 %arg0, label %bb1, label %bb2
+
+; CHECK: bb1:
+; CHECK-NEXT: %bb1.phi = phi i32 [ %bb.load, %bb ], [ %bb2.phi, %bb2 ], [ %bb2.phi, %bb2 ]
+; CHECK-NEXT: store i32 1, ptr null, align 4
+; CHECK-NEXT: ret void
+
+; CHECK: bb2: ; preds = %bb
+; CHECK-NEXT: %bb2.phi = phi i32 [ %bb.load, %bb ]
+; CHECK-NEXT: store i32 2, ptr null, align 4
+; CHECK-NEXT: switch i32 %bb2.phi, label %bb1 [
+; CHECK-NEXT: i32 0, label %bb1
+; CHECK-NEXT: ]
+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());
+    }
+  }
 }
 
 /// It's OK to add a block to the set of removed blocks if the first
 /// basic block in the function that survives all of the deletions is
 /// a legal entry block. Keep at least one block in a function.
 static bool okToRemove(BasicBlock &Candidate, Function &F,
                        const DenseSet<BasicBlock *> &BBsToDelete) {
   size_t NumBlocksDeleted = 0;
for (auto &F : Program) {
     for (auto &BB : F) {
       if (!okToRemove(BB, F, BBsToDelete) || 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");
 }