[PATCH] D136289: llvm-reduce: Don't turn switches into returns

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceBasicBlocks.cpp:98
   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 (SwInst.getNumCases() == 0) {
+      auto *FnRetTy = SwInst.getParent()->getParent()->getReturnType();
----------------
aeubanks wrote:
> do we have a test for this part?
It gets hit, but not sure this is reflected in the direct output of any. Added another case


================
Comment at: llvm/tools/llvm-reduce/deltas/ReduceBasicBlocks.cpp:114
+    for (PHINode &SuccPHI : NewDefault->phis()) {
+      SuccPHI.addIncoming(SuccPHI.getIncomingValueForBlock(SwInst.getParent()),
+                          SwInst.getParent());
----------------
aeubanks wrote:
> if `NewDefault` was already a successor, doesn't its phis already have values from this block?
Yes that's where it's getting the value. Phis have to have an incoming operand for each instance


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136289/new/

https://reviews.llvm.org/D136289