[llvm] r248677 - [EH] Create removeUnwindEdge utility

Joseph Tremoulet via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 26 18:47:47 PDT 2015


Author: josepht
Date: Sat Sep 26 20:47:46 2015
New Revision: 248677

URL: http://llvm.org/viewvc/llvm-project?rev=248677&view=rev
Log:
[EH] Create removeUnwindEdge utility

Summary:
Factor the code that rewrites invokes to calls and rewrites WinEH
terminators to their "unwind to caller" equivalents into a helper in
Utils/Local, and use it in the three places I'm aware of that need to do
this.


Reviewers: andrew.w.kaylor, majnemer, rnk

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D13152

Added:
    llvm/trunk/test/Transforms/SimplifyCFG/wineh-unreachable.ll
Modified:
    llvm/trunk/include/llvm/Transforms/Utils/Local.h
    llvm/trunk/lib/CodeGen/WinEHPrepare.cpp
    llvm/trunk/lib/Transforms/Utils/Local.cpp
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/trunk/test/CodeGen/WinEH/wineh-cloning.ll

Modified: llvm/trunk/include/llvm/Transforms/Utils/Local.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/Local.h?rev=248677&r1=248676&r2=248677&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/Local.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/Local.h Sat Sep 26 20:47:46 2015
@@ -277,6 +277,14 @@ DbgDeclareInst *FindAllocaDbgDeclare(Val
 bool replaceDbgDeclareForAlloca(AllocaInst *AI, Value *NewAllocaAddress,
                                 DIBuilder &Builder, bool Deref);
 
+/// Replace 'BB's terminator with one that does not have an unwind successor
+/// block.  Rewrites `invoke` to `call`, `catchendpad unwind label %foo` to
+/// `catchendpad unwind to caller`, etc.  Updates any PHIs in unwind successor.
+///
+/// \param BB  Block whose terminator will be replaced.  Its terminator must
+///            have an unwind successor.
+void removeUnwindEdge(BasicBlock *BB);
+
 /// \brief Remove all blocks that can not be reached from the function's entry.
 ///
 /// Returns true if any basic block was removed.

Modified: llvm/trunk/lib/CodeGen/WinEHPrepare.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/WinEHPrepare.cpp?rev=248677&r1=248676&r2=248677&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/WinEHPrepare.cpp (original)
+++ llvm/trunk/lib/CodeGen/WinEHPrepare.cpp Sat Sep 26 20:47:46 2015
@@ -3184,9 +3184,23 @@ void WinEHPrepare::removeImplausibleTerm
         for (BasicBlock *SuccBB : TI->successors())
           SuccBB->removePredecessor(BB);
 
+        if (IsUnreachableCleanupendpad) {
+          // We can't simply replace a cleanupendpad with unreachable, because
+          // its predecessor edges are EH edges and unreachable is not an EH
+          // pad.  Change all predecessors to the "unwind to caller" form.
+          for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB);
+               PI != PE;) {
+            BasicBlock *Pred = *PI++;
+            removeUnwindEdge(Pred);
+          }
+        }
+
         new UnreachableInst(BB->getContext(), TI);
         TI->eraseFromParent();
       }
+      // FIXME: Check for invokes/cleanuprets/cleanupendpads which unwind to
+      // implausible catchendpads (i.e. catchendpad not in immediate parent
+      // funclet).
     }
   }
 }

Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=248677&r1=248676&r2=248677&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/Local.cpp Sat Sep 26 20:47:46 2015
@@ -1258,6 +1258,43 @@ static bool markAliveBlocks(Function &F,
   return Changed;
 }
 
+void llvm::removeUnwindEdge(BasicBlock *BB) {
+  TerminatorInst *TI = BB->getTerminator();
+
+  if (auto *II = dyn_cast<InvokeInst>(TI)) {
+    changeToCall(II);
+    return;
+  }
+
+  TerminatorInst *NewTI;
+  BasicBlock *UnwindDest;
+
+  if (auto *CRI = dyn_cast<CleanupReturnInst>(TI)) {
+    NewTI = CleanupReturnInst::Create(CRI->getCleanupPad(), nullptr, CRI);
+    UnwindDest = CRI->getUnwindDest();
+  } else if (auto *CEP = dyn_cast<CleanupEndPadInst>(TI)) {
+    NewTI = CleanupEndPadInst::Create(CEP->getCleanupPad(), nullptr, CEP);
+    UnwindDest = CEP->getUnwindDest();
+  } else if (auto *CEP = dyn_cast<CatchEndPadInst>(TI)) {
+    NewTI = CatchEndPadInst::Create(CEP->getContext(), nullptr, CEP);
+    UnwindDest = CEP->getUnwindDest();
+  } else if (auto *TPI = dyn_cast<TerminatePadInst>(TI)) {
+    SmallVector<Value *, 3> TerminatePadArgs;
+    for (Value *Operand : TPI->arg_operands())
+      TerminatePadArgs.push_back(Operand);
+    NewTI = TerminatePadInst::Create(TPI->getContext(), nullptr,
+                                     TerminatePadArgs, TPI);
+    UnwindDest = TPI->getUnwindDest();
+  } else {
+    llvm_unreachable("Could not find unwind successor");
+  }
+
+  NewTI->takeName(TI);
+  NewTI->setDebugLoc(TI->getDebugLoc());
+  UnwindDest->removePredecessor(BB);
+  TI->eraseFromParent();
+}
+
 /// removeUnreachableBlocksFromFn - Remove blocks that are not reachable, even
 /// if they are in a dead cycle.  Return true if a change was made, false
 /// otherwise.

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=248677&r1=248676&r2=248677&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Sat Sep 26 20:47:46 2015
@@ -2901,31 +2901,6 @@ static bool SimplifyBranchOnICmpChain(Br
   return true;
 }
 
-// FIXME: This seems like a pretty common thing to want to do.  Consider
-// whether there is a more accessible place to put this.
-static void convertInvokeToCall(InvokeInst *II) {
-  SmallVector<Value*, 8> Args(II->op_begin(), II->op_end() - 3);
-  // Insert a call instruction before the invoke.
-  CallInst *Call = CallInst::Create(II->getCalledValue(), Args, "", II);
-  Call->takeName(II);
-  Call->setCallingConv(II->getCallingConv());
-  Call->setAttributes(II->getAttributes());
-  Call->setDebugLoc(II->getDebugLoc());
-
-  // Anything that used the value produced by the invoke instruction now uses
-  // the value produced by the call instruction.  Note that we do this even
-  // for void functions and calls with no uses so that the callgraph edge is
-  // updated.
-  II->replaceAllUsesWith(Call);
-  II->getUnwindDest()->removePredecessor(II->getParent());
-
-  // Insert a branch to the normal destination right before the invoke.
-  BranchInst::Create(II->getNormalDest(), II);
-
-  // Finally, delete the invoke instruction!
-  II->eraseFromParent();
-}
-
 bool SimplifyCFGOpt::SimplifyResume(ResumeInst *RI, IRBuilder<> &Builder) {
   // If this is a trivial landing pad that just continues unwinding the caught
   // exception then zap the landing pad, turning its invokes into calls.
@@ -2944,8 +2919,8 @@ bool SimplifyCFGOpt::SimplifyResume(Resu
 
   // Turn all invokes that unwind here into calls and delete the basic block.
   for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB); PI != PE;) {
-    InvokeInst *II = cast<InvokeInst>((*PI++)->getTerminator());
-    convertInvokeToCall(II);
+    BasicBlock *Pred = *PI++;
+    removeUnwindEdge(Pred);
   }
 
   // The landingpad is now unreachable.  Zap it.
@@ -3056,62 +3031,11 @@ bool SimplifyCFGOpt::SimplifyCleanupRetu
   for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB); PI != PE;) {
     // The iterator must be updated here because we are removing this pred.
     BasicBlock *PredBB = *PI++;
-    TerminatorInst *TI = PredBB->getTerminator();
     if (UnwindDest == nullptr) {
-      if (auto *II = dyn_cast<InvokeInst>(TI)) {
-        // The cleanup return being simplified continues to the caller and this
-        // predecessor terminated with an invoke instruction.  Convert the
-        // invoke to a call.
-        // This call updates the predecessor/successor chain.
-        convertInvokeToCall(II);
-      } else {
-        // In the remaining cases the predecessor's terminator unwinds to the
-        // block we are removing.  We need to create a new instruction that
-        // unwinds to the caller.  Simply setting the unwind destination to
-        // nullptr would leave the objects internal data in an inconsistent
-        // state.
-        // FIXME: Consider whether it is better to update setUnwindDest to
-        //        keep things consistent.
-        if (auto *CRI = dyn_cast<CleanupReturnInst>(TI)) {
-          auto *NewCRI = CleanupReturnInst::Create(CRI->getCleanupPad(),
-                                                   nullptr, CRI);
-          NewCRI->takeName(CRI);
-          NewCRI->setDebugLoc(CRI->getDebugLoc());
-          CRI->eraseFromParent();
-        } else if (auto *CEP = dyn_cast<CatchEndPadInst>(TI)) {
-          auto *NewCEP = CatchEndPadInst::Create(CEP->getContext(), nullptr,
-                                                 CEP);
-          NewCEP->takeName(CEP);
-          NewCEP->setDebugLoc(CEP->getDebugLoc());
-          CEP->eraseFromParent();
-        } else if (auto *TPI = dyn_cast<TerminatePadInst>(TI)) {
-          SmallVector<Value *, 3> TerminatePadArgs;
-          for (Value *Operand : TPI->arg_operands())
-            TerminatePadArgs.push_back(Operand);
-          auto *NewTPI = TerminatePadInst::Create(TPI->getContext(), nullptr,
-                                                  TerminatePadArgs, TPI);
-          NewTPI->takeName(TPI);
-          NewTPI->setDebugLoc(TPI->getDebugLoc());
-          TPI->eraseFromParent();
-        } else {
-          llvm_unreachable("Unexpected predecessor to cleanup pad.");
-        }
-      }
+      removeUnwindEdge(PredBB);
     } else {
-      // If the predecessor did not terminate with an invoke instruction, it
-      // must be some variety of EH pad.
       TerminatorInst *TI = PredBB->getTerminator();
-      // FIXME: Introducing an EH terminator base class would simplify this.
-      if (auto *II = dyn_cast<InvokeInst>(TI))
-        II->setUnwindDest(UnwindDest);
-      else if (auto *CRI = dyn_cast<CleanupReturnInst>(TI))
-        CRI->setUnwindDest(UnwindDest);
-      else if (auto *CEP = dyn_cast<CatchEndPadInst>(TI))
-        CEP->setUnwindDest(UnwindDest);
-      else if (auto *TPI = dyn_cast<TerminatePadInst>(TI))
-        TPI->setUnwindDest(UnwindDest);
-      else
-        llvm_unreachable("Unexpected predecessor to cleanup pad.");
+      TI->replaceUsesOfWith(BB, UnwindDest);
     }
   }
 
@@ -3249,26 +3173,21 @@ bool SimplifyCFGOpt::SimplifyUnreachable
           --i; --e;
           Changed = true;
         }
-    } else if (InvokeInst *II = dyn_cast<InvokeInst>(TI)) {
-      if (II->getUnwindDest() == BB) {
-        // Convert the invoke to a call instruction.  This would be a good
-        // place to note that the call does not throw though.
-        BranchInst *BI = Builder.CreateBr(II->getNormalDest());
-        II->removeFromParent();   // Take out of symbol table
-
-        // Insert the call now...
-        SmallVector<Value*, 8> Args(II->op_begin(), II->op_end()-3);
-        Builder.SetInsertPoint(BI);
-        CallInst *CI = Builder.CreateCall(II->getCalledValue(),
-                                          Args, II->getName());
-        CI->setCallingConv(II->getCallingConv());
-        CI->setAttributes(II->getAttributes());
-        // If the invoke produced a value, the call does now instead.
-        II->replaceAllUsesWith(CI);
-        delete II;
-        Changed = true;
-      }
+    } else if ((isa<InvokeInst>(TI) &&
+                cast<InvokeInst>(TI)->getUnwindDest() == BB) ||
+               isa<CatchEndPadInst>(TI) || isa<TerminatePadInst>(TI)) {
+      removeUnwindEdge(TI->getParent());
+      Changed = true;
+    } else if (isa<CleanupReturnInst>(TI) || isa<CleanupEndPadInst>(TI) ||
+               isa<CatchReturnInst>(TI)) {
+      new UnreachableInst(TI->getContext(), TI);
+      TI->eraseFromParent();
+      Changed = true;
     }
+    // TODO: If TI is a CatchPadInst, then (BB must be its normal dest and)
+    // we can eliminate it, redirecting its preds to its unwind successor,
+    // or to the next outer handler if the removed catch is the last for its
+    // catchendpad.
   }
 
   // If this block is now dead, remove it.

Modified: llvm/trunk/test/CodeGen/WinEH/wineh-cloning.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/WinEH/wineh-cloning.ll?rev=248677&r1=248676&r2=248677&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/WinEH/wineh-cloning.ll (original)
+++ llvm/trunk/test/CodeGen/WinEH/wineh-cloning.ll Sat Sep 26 20:47:46 2015
@@ -421,3 +421,34 @@ unreachable:
 ; CHECK-NEXT:   catchendpad unwind to caller
 ; CHECK:      exit:
 ; CHECK-NEXT:   ret void
+
+define void @test11() personality i32 (...)* @__CxxFrameHandler3 {
+entry:
+  invoke void @f()
+    to label %exit unwind label %cleanup.outer
+cleanup.outer:
+  %outer = cleanuppad []
+  invoke void @f()
+    to label %outer.cont unwind label %cleanup.inner
+outer.cont:
+  br label %merge
+cleanup.inner:
+  %inner = cleanuppad []
+  br label %merge
+merge:
+  invoke void @f()
+    to label %unreachable unwind label %merge.end
+unreachable:
+  unreachable
+merge.end:
+  cleanupendpad %outer unwind to caller
+exit:
+  ret void
+}
+; merge.end will get cloned for outer and inner, but is implausible
+; from inner, so the invoke @f() in inner's copy of merge should be
+; rewritten to call @f()
+; CHECK-LABEL: define void @test11()
+; CHECK:      %inner = cleanuppad []
+; CHECK-NEXT: call void @f()
+; CHECK-NEXT: unreachable

Added: llvm/trunk/test/Transforms/SimplifyCFG/wineh-unreachable.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/wineh-unreachable.ll?rev=248677&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/wineh-unreachable.ll (added)
+++ llvm/trunk/test/Transforms/SimplifyCFG/wineh-unreachable.ll Sat Sep 26 20:47:46 2015
@@ -0,0 +1,88 @@
+; RUN: opt -S -simplifycfg < %s | FileCheck %s
+
+declare void @Personality()
+declare void @f()
+
+; CHECK-LABEL: define void @test1()
+define void @test1() personality i8* bitcast (void ()* @Personality to i8*) {
+entry:
+  ; CHECK: call void @f()
+  invoke void @f()
+    to label %exit unwind label %unreachable.unwind
+exit:
+  ret void
+unreachable.unwind:
+  cleanuppad []
+  unreachable  
+}
+
+; CHECK-LABEL: define void @test2()
+define void @test2() personality i8* bitcast (void ()* @Personality to i8*) {
+entry:
+  invoke void @f()
+    to label %exit unwind label %catch.pad
+catch.pad:
+  ; CHECK: catchpad []
+  ; CHECK-NEXT: to label %catch.body unwind label %catch.end
+  %catch = catchpad []
+    to label %catch.body unwind label %catch.end
+catch.body:
+  ; CHECK:      catch.body:
+  ; CHECK-NEXT:   call void @f()
+  ; CHECK-NEXT:   unreachable
+  call void @f()
+  catchret %catch to label %unreachable
+catch.end:
+  ; CHECK: catch.end:
+  ; CHECK-NEXT: catchendpad unwind to caller
+  catchendpad unwind label %unreachable.unwind
+exit:
+  ret void
+unreachable.unwind:
+  cleanuppad []
+  unreachable
+unreachable:
+  unreachable
+}
+
+; CHECK-LABEL: define void @test3()
+define void @test3() personality i8* bitcast (void ()* @Personality to i8*) {
+entry:
+  invoke void @f()
+    to label %exit unwind label %cleanup.pad
+cleanup.pad:
+  ; CHECK: %cleanup = cleanuppad []
+  ; CHECK-NEXT: call void @f()
+  ; CHECK-NEXT: unreachable
+  %cleanup = cleanuppad []
+  invoke void @f()
+    to label %cleanup.ret unwind label %cleanup.end
+cleanup.ret:
+  ; This cleanupret should be rewritten to unreachable,
+  ; and merged into the pred block.
+  cleanupret %cleanup unwind label %unreachable.unwind
+cleanup.end:
+  ; This cleanupendpad should be rewritten to unreachable,
+  ; causing the invoke to be rewritten to a call.
+  cleanupendpad %cleanup unwind label %unreachable.unwind
+exit:
+  ret void
+unreachable.unwind:
+  cleanuppad []
+  unreachable
+}
+
+; CHECK-LABEL: define void @test4()
+define void @test4() personality i8* bitcast (void ()* @Personality to i8*) {
+entry:
+  invoke void @f()
+    to label %exit unwind label %terminate.pad
+terminate.pad:
+  ; CHECK: terminatepad [] unwind to caller
+  terminatepad [] unwind label %unreachable.unwind
+exit:
+  ret void
+unreachable.unwind:
+  cleanuppad []
+  unreachable
+}




More information about the llvm-commits mailing list