llvm-reduce: Support exotic terminators in instructions-to-return

Use splitBasicBlock and avoid directly dealing with the specific of
how to trim the existing terminators. We just need to deal with
unconditional branch to return.

Author: arsenm

llvm/test/tools/llvm-reduce/reduce-values-to-return-callbr.ll

@@ -0,0 +1,54 @@
+; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=instructions-to-return --test FileCheck --test-arg --check-prefix=INTERESTING --test-arg %s --test-arg --input-file %s -o %t
+; RUN: FileCheck --check-prefix=RESULT %s < %t
+
+@gv = global i32 0, align 4
+
+; INTERESTING-LABEL: @callbr0(
+; INTERESTING: %load0 = load i32, ptr %arg0
+; INTERESTING: store i32 %load0, ptr @gv
+
+; RESULT-LABEL: define void @callbr0(ptr %arg0) {
+; RESULT: %load0 = load i32, ptr %arg0, align 4
+; RESULT-NEXT: %callbr = callbr i32 asm
+define void @callbr0(ptr %arg0) {
+entry:
+  %load0 = load i32, ptr %arg0
+  %callbr = callbr i32 asm "", "=r,r,!i,!i"(i32 %load0)
+              to label %one [label %two, label %three]
+one:
+  store i32 %load0, ptr @gv
+  ret void
+
+two:
+  store i32 %load0, ptr @gv
+  ret void
+
+three:
+  store i32 %load0, ptr @gv
+  ret void
+}
+
+; INTERESTING-LABEL: @callbr1(
+; INTERESTING: %load0 = load i32, ptr %arg0
+
+; RESULT-LABEL: define i32 @callbr1(ptr %arg0) {
+; RESULT-NEXT: entry:
+; RESULT-NEXT: %load0 = load i32, ptr %arg0
+; RESULT-NEXT: ret i32 %load0
+define void @callbr1(ptr %arg0) {
+entry:
+  %load0 = load i32, ptr %arg0
+  %callbr = callbr i32 asm "", "=r,r,!i,!i"(i32 %load0)
+              to label %one [label %two, label %three]
+one:
+  store i32 %load0, ptr @gv
+  ret void
+
+two:
+  store i32 %load0, ptr @gv
+  ret void
+
+three:
+  store i32 %load0, ptr @gv
+  ret void
+}

llvm/test/tools/llvm-reduce/reduce-values-to-return-invoke.ll

@@ -0,0 +1,56 @@
+; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=instructions-to-return --test FileCheck --test-arg --check-prefix=INTERESTING --test-arg %s --test-arg --input-file %s -o %t
+; RUN: FileCheck --check-prefix=RESULT %s < %t
+
+@gv = global i32 0, align 4
+
+
+define i32 @has_invoke_user(ptr %arg) {
+  %load = load i32, ptr %arg
+  store i32 %load, ptr @gv
+  ret i32 9
+}
+
+declare i32 @__gxx_personality_v0(...)
+
+; INTERESTING-LABEL: @invoker_keep_invoke(
+; INTERESTING: %invoke
+; RESULT:   %invoke = invoke i32 @has_invoke_user(ptr %arg)
+define void @invoker_keep_invoke(ptr %arg) personality ptr @__gxx_personality_v0 {
+bb:
+  %invoke = invoke i32 @has_invoke_user(ptr %arg)
+    to label %bb3 unwind label %bb1
+
+bb1:
+  landingpad { ptr, i32 }
+  catch ptr null
+  ret void
+
+bb3:
+  store i32 %invoke, ptr null
+  ret void
+}
+
+; INTERESTING-LABEL: @invoker_drop_invoke(
+; INTERESTING: %add = add i32
+
+; RESULT-LABEL: define i32 @invoker_drop_invoke(i32 %arg0, ptr %arg1) personality ptr @__gxx_personality_v0 {
+; RESULT-NEXT: bb:
+; RESULT-NEXT: %add = add i32 %arg0, 9
+; RESULT-NEXT: ret i32 %add
+; RESULT-NEXT: }
+define void @invoker_drop_invoke(i32 %arg0, ptr %arg1) personality ptr @__gxx_personality_v0 {
+bb:
+  %add = add i32 %arg0, 9
+  %invoke = invoke i32 @has_invoke_user(ptr %arg1)
+    to label %bb3 unwind label %bb1
+
+bb1:
+  landingpad { ptr, i32 }
+  catch ptr null
+  br label %bb3
+
+bb3:
+  %phi = phi i32 [ %invoke, %bb ], [ %add, %bb1 ]
+  store i32 %phi, ptr null
+  ret void
+}

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

@@ -22,7 +22,6 @@
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Instructions.h"
-#include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 
 using namespace llvm;
@@ -55,7 +54,7 @@ static void rewriteFuncWithReturnType(Function &OldF, Value *NewRetValue) {
   BasicBlock *NewRetBlock = NewRetI ? NewRetI->getParent() : &EntryBB;
 
   BasicBlock::iterator NewValIt =
-      NewRetI ? NewRetI->getIterator() : EntryBB.end();
+      NewRetI ? std::next(NewRetI->getIterator()) : EntryBB.begin();
 
   Type *OldRetTy = OldFuncTy->getReturnType();
 
@@ -73,28 +72,16 @@ static void rewriteFuncWithReturnType(Function &OldF, Value *NewRetValue) {
     }
   }
 
-  // Now prune any CFG edges we have to deal with.
-  //
-  // Use KeepOneInputPHIs in case the instruction we are using for the return is
-  // that phi.
-  // TODO: Could avoid this with fancier iterator management.
-  for (BasicBlock *Succ : successors(NewRetBlock))
-    Succ->removePredecessor(NewRetBlock, /*KeepOneInputPHIs=*/true);
-
-  // Now delete the tail of this block, in reverse to delete uses before defs.
-  for (Instruction &I : make_early_inc_range(
-           make_range(NewRetBlock->rbegin(), NewValIt.getReverse()))) {
-    Value *Replacement = getDefaultValue(I.getType());
-    I.replaceAllUsesWith(Replacement);
-    I.eraseFromParent();
-  }
+  // If we're returning an instruction, split the basic block so we can let
+  // EliminateUnreachableBlocks cleanup the successors.
+  BasicBlock *TailBB = NewRetBlock->splitBasicBlock(NewValIt);
 
+  // Replace the unconditional branch splitBasicBlock created
+  NewRetBlock->getTerminator()->eraseFromParent();
   ReturnInst::Create(Ctx, NewRetValue, NewRetBlock);
 
-  // TODO: We may be eliminating blocks that were originally unreachable. We
-  // probably ought to only be pruning blocks that became dead directly as a
-  // result of our pruning here.
-  EliminateUnreachableBlocks(OldF);
+  // Now prune any CFG edges we have to deal with.
+  simpleSimplifyCFG(OldF, {TailBB}, /*FoldBlockIntoPredecessor=*/false);
 
   // Drop the incompatible attributes before we copy over to the new function.
   if (OldRetTy != NewRetTy) {
@@ -199,20 +186,6 @@ static bool shouldReplaceNonVoidReturnValue(const BasicBlock &BB,
   return true;
 }
 
-static bool canHandleSuccessors(const BasicBlock &BB) {
-  // TODO: Handle invoke and other exotic terminators
-  if (!isa<ReturnInst, UnreachableInst, BranchInst, SwitchInst>(
-          BB.getTerminator()))
-    return false;
-
-  for (const BasicBlock *Succ : successors(&BB)) {
-    if (!Succ->canSplitPredecessors())
-      return false;
-  }
-
-  return true;
-}
-
 static bool shouldForwardValueToReturn(const BasicBlock &BB, const Value *V,
                                        Type *RetTy) {
   if (!isReallyValidReturnType(V->getType()))
@@ -231,10 +204,9 @@ static bool tryForwardingInstructionsToReturn(
   Type *RetTy = F.getReturnType();
 
   for (BasicBlock &BB : F) {
-    if (!canHandleSuccessors(BB))
-      continue;
-
-    for (Instruction &I : BB) {
+    // Skip the terminator, we can't insert a second terminator to return its
+    // value.
+    for (Instruction &I : make_range(BB.begin(), std::prev(BB.end()))) {
       if (shouldForwardValueToReturn(BB, &I, RetTy) && !O.shouldKeep()) {
         FuncsToReplace.emplace_back(&F, &I);
         return true;

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

@@ -79,7 +79,7 @@ void llvm::simpleSimplifyCFG(Function &F, ArrayRef<BasicBlock *> BBs,
   for (BasicBlock *BB : Unreachable) {
     for (BasicBlock *Successor : successors(&*BB))
       if (Visited.count(Successor))
-        Successor->removePredecessor(&*BB);
+        Successor->removePredecessor(&*BB, /*KeepOneInputPHIs=*/true);
     BB->dropAllReferences();
   }