[llvm] r259338 - Revert r258580 and r258581.

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 31 19:29:39 PST 2016


Author: majnemer
Date: Sun Jan 31 21:29:38 2016
New Revision: 259338

URL: http://llvm.org/viewvc/llvm-project?rev=259338&view=rev
Log:
Revert r258580 and r258581.

Those commits created an artificial edge from a cleanup to a synthesized
catchswitch in order to get the MSVC personality routine to execute
cleanups which don't cleanupret and are not wrapped by a catchswitch.

This worked well enough but is not a complete solution in situations
where there the cleanup infinite loops.

However, the real deal breaker behind this approach comes about from a
degenerate case where the cleanup is post-dominated by unreachable *and*
throws an exception.  This ends poorly because the catchswitch will
inadvertently catch the exception.

Because of this we should go back to our previous behavior of not
executing certain cleanups (identical behavior with the Itanium ABI
implementation in clang, GCC and ICC).

N.B. I think this could be salvaged by making the catchpad rethrow the
exception and properly transforming throwing calls in the cleanup into
invokes.

Removed:
    llvm/trunk/test/CodeGen/WinEH/wineh-cleanuppad-nounwind.ll
Modified:
    llvm/trunk/lib/CodeGen/WinEHPrepare.cpp
    llvm/trunk/test/CodeGen/WinEH/wineh-cloning.ll
    llvm/trunk/test/CodeGen/WinEH/wineh-demotion.ll
    llvm/trunk/test/CodeGen/WinEH/wineh-no-demotion.ll

Modified: llvm/trunk/lib/CodeGen/WinEHPrepare.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/WinEHPrepare.cpp?rev=259338&r1=259337&r2=259338&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/WinEHPrepare.cpp (original)
+++ llvm/trunk/lib/CodeGen/WinEHPrepare.cpp Sun Jan 31 21:29:38 2016
@@ -81,7 +81,6 @@ private:
   void cloneCommonBlocks(Function &F);
   void removeImplausibleInstructions(Function &F);
   void cleanupPreparedFunclets(Function &F);
-  void fixupNoReturnCleanupPads(Function &F);
   void verifyPreparedFunclets(Function &F);
 
   // All fields are reset by runOnFunction.
@@ -657,7 +656,6 @@ void llvm::calculateClrEHStateNumbers(co
 
 void WinEHPrepare::colorFunclets(Function &F) {
   BlockColors = colorEHFunclets(F);
-  FuncletBlocks.clear();
 
   // Invert the map from BB to colors to color to BBs.
   for (BasicBlock &BB : F) {
@@ -1003,115 +1001,6 @@ void WinEHPrepare::cleanupPreparedFuncle
   removeUnreachableBlocks(F);
 }
 
-// Cleanuppads which are postdominated by unreachable will not unwind to any
-// catchswitches, making them dead.  This is problematic if the original source
-// had a catch clause which could legitimately catch the exception, causing the
-// cleanup to run.
-//
-// This is only a problem for C++ where down-stream catches cause cleanups to
-// run.
-void WinEHPrepare::fixupNoReturnCleanupPads(Function &F) {
-  // We only need to do this for C++ personality routines,
-  // skip this work for all others.
-  if (Personality != EHPersonality::MSVC_CXX)
-    return;
-
-  // Do a quick sanity check on the color map before we throw it away so as to
-  // avoid hiding latent bugs.
-  DEBUG(verifyPreparedFunclets(F));
-  // Re-color the funclets because cleanupPreparedFunclets might have
-  // invalidated FuncletBlocks.
-  colorFunclets(F);
-
-  // We create a unique catchswitch for each parent it will be nested within.
-  SmallDenseMap<Value *, CatchSwitchInst *> NewCatchSwitches;
-  // Create a new catchswitch+catchpad where the catchpad is post-dominated by
-  // unreachable.
-  auto GetOrCreateCatchSwitch = [&](Value *ParentPad) {
-    auto &CatchSwitch = NewCatchSwitches[ParentPad];
-    if (CatchSwitch)
-      return CatchSwitch;
-
-    auto *ParentBB = isa<ConstantTokenNone>(ParentPad)
-                         ? &F.getEntryBlock()
-                         : cast<Instruction>(ParentPad)->getParent();
-
-    StringRef NameSuffix = ParentBB->getName();
-
-    BasicBlock *CatchSwitchBB = BasicBlock::Create(
-        F.getContext(), Twine("catchswitchbb.for.", NameSuffix));
-    CatchSwitchBB->insertInto(&F, ParentBB->getNextNode());
-    CatchSwitch = CatchSwitchInst::Create(ParentPad, /*UnwindDest=*/nullptr,
-                                          /*NumHandlers=*/1,
-                                          Twine("catchswitch.for.", NameSuffix),
-                                          CatchSwitchBB);
-
-    BasicBlock *CatchPadBB = BasicBlock::Create(
-        F.getContext(), Twine("catchpadbb.for.", NameSuffix));
-    CatchPadBB->insertInto(&F, CatchSwitchBB->getNextNode());
-    Value *CatchPadArgs[] = {
-        Constant::getNullValue(Type::getInt8PtrTy(F.getContext())),
-        ConstantInt::get(Type::getInt32Ty(F.getContext()), 64),
-        Constant::getNullValue(Type::getInt8PtrTy(F.getContext())),
-    };
-    CatchPadInst::Create(CatchSwitch, CatchPadArgs,
-                         Twine("catchpad.for.", NameSuffix), CatchPadBB);
-    new UnreachableInst(F.getContext(), CatchPadBB);
-
-    CatchSwitch->addHandler(CatchPadBB);
-
-    return CatchSwitch;
-  };
-
-  // Look for all basic blocks which are within cleanups which are postdominated
-  // by unreachable.
-  for (auto &Funclets : FuncletBlocks) {
-    BasicBlock *FuncletPadBB = Funclets.first;
-    auto *CleanupPad = dyn_cast<CleanupPadInst>(FuncletPadBB->getFirstNonPHI());
-    // Skip over any non-cleanup funclets.
-    if (!CleanupPad)
-      continue;
-    // Skip over any cleanups have unwind targets, they do not need this.
-    if (any_of(CleanupPad->users(),
-               [](const User *U) { return isa<CleanupReturnInst>(U); }))
-      continue;
-    // Walk the blocks within the cleanup which end in 'unreachable'.
-    // We will replace the unreachable instruction with a cleanupret;
-    // this cleanupret will unwind to a catchswitch with a lone catch-all
-    // catchpad.
-    std::vector<BasicBlock *> &BlocksInFunclet = Funclets.second;
-    for (BasicBlock *BB : BlocksInFunclet) {
-      auto *UI = dyn_cast<UnreachableInst>(BB->getTerminator());
-      if (!UI)
-        continue;
-      // Remove the unreachable instruction.
-      UI->eraseFromParent();
-
-      // Add our new cleanupret.
-      auto *ParentPad = CleanupPad->getParentPad();
-      CatchSwitchInst *CatchSwitch = GetOrCreateCatchSwitch(ParentPad);
-      CleanupReturnInst::Create(CleanupPad, CatchSwitch->getParent(), BB);
-    }
-  }
-
-  // Update BlockColors and FuncletBlocks to maintain WinEHPrepare's
-  // invariants.
-  for (auto CatchSwitchKV : NewCatchSwitches) {
-    CatchSwitchInst *CatchSwitch = CatchSwitchKV.second;
-    BasicBlock *CatchSwitchBB = CatchSwitch->getParent();
-
-    assert(CatchSwitch->getNumSuccessors() == 1);
-    BasicBlock *CatchPadBB = CatchSwitch->getSuccessor(0);
-    assert(isa<CatchPadInst>(CatchPadBB->getFirstNonPHI()));
-
-    BlockColors.insert({CatchSwitchBB, ColorVector(CatchSwitchBB)});
-    FuncletBlocks[CatchSwitchBB] = {CatchSwitchBB};
-
-    BlockColors.insert({CatchPadBB, ColorVector(CatchPadBB)});
-    FuncletBlocks[CatchPadBB] = {CatchPadBB};
-  }
-}
-
 void WinEHPrepare::verifyPreparedFunclets(Function &F) {
   for (BasicBlock &BB : F) {
     size_t NumColors = BlockColors[&BB].size();
@@ -1147,8 +1036,6 @@ bool WinEHPrepare::prepareExplicitEH(Fun
     cleanupPreparedFunclets(F);
   }
 
-  fixupNoReturnCleanupPads(F);
-
   DEBUG(verifyPreparedFunclets(F));
   // Recolor the CFG to verify that all is well.
   DEBUG(colorFunclets(F));

Removed: llvm/trunk/test/CodeGen/WinEH/wineh-cleanuppad-nounwind.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/WinEH/wineh-cleanuppad-nounwind.ll?rev=259337&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/WinEH/wineh-cleanuppad-nounwind.ll (original)
+++ llvm/trunk/test/CodeGen/WinEH/wineh-cleanuppad-nounwind.ll (removed)
@@ -1,95 +0,0 @@
-; RUN: opt -S -winehprepare < %s | FileCheck %s
-target triple = "x86_64-pc-windows-msvc"
-
-; CHECK-LABEL: @test1(
-define void @test1(i1 %b) personality i32 (...)* @__CxxFrameHandler3 {
-entry:
-  invoke void @f()
-          to label %try.cont unwind label %cleanup.bb
-
-; CHECK: entry:
-
-; CHECK: [[catchswitch_entry:.*]]:
-; CHECK-NEXT: %[[cs0:.*]] = catchswitch within none [label %[[catchpad:.*]]] unwind to caller
-; CHECK: [[catchpad]]:
-; CHECK-NEXT: %[[cp0:.*]] = catchpad within %[[cs0]] [i8* null, i32 64, i8* null]
-; CHECK-NEXT: unreachable
-
-try.cont:
-  invoke void @f()
-          to label %exit unwind label %catchswitch.bb
-
-cleanup.bb:
-  %cleanup = cleanuppad within none []
-  br i1 %b, label %left, label %right
-
-left:
-  call void @exit(i32 0)  [ "funclet"(token %cleanup) ]
-  unreachable
-
-right:
-  call void @exit(i32 1)  [ "funclet"(token %cleanup) ]
-  unreachable
-
-catchswitch.bb:
-  %cs = catchswitch within none [label %catchpad.bb] unwind to caller
-
-; CHECK: catchpad.bb:
-; CHECK-NEXT:  %catch = catchpad within %cs [i8* null, i32 64, i8* null]
-
-; CHECK: [[catchswitch_catch:.*]]:
-; CHECK-NEXT: %[[cs1:.*]] = catchswitch within %catch [label %[[catchpad_catch:.*]]] unwind to caller
-; CHECK: [[catchpad_catch]]:
-; CHECK-NEXT: %[[cp1:.*]] = catchpad within %[[cs1]] [i8* null, i32 64, i8* null]
-; CHECK-NEXT: unreachable
-
-; CHECK: nested.cleanup.bb:
-; CHECK-NEXT:  %nested.cleanup = cleanuppad within %catch []
-; CHECK-NEXT:  call void @exit(i32 2)  [ "funclet"(token %nested.cleanup) ]
-; CHECK-NEXT:  cleanupret from %nested.cleanup unwind label %[[catchswitch_catch]]
-
-catchpad.bb:
-  %catch = catchpad within %cs [i8* null, i32 64, i8* null]
-  invoke void @f() [ "funclet"(token %catch) ]
-          to label %unreachable unwind label %nested.cleanup.bb
-
-nested.cleanup.bb:
-  %nested.cleanup = cleanuppad within %catch []
-  call void @exit(i32 2)  [ "funclet"(token %nested.cleanup) ]
-  unreachable
-
-unreachable:
-  unreachable
-
-exit:
-  unreachable
-}
-
-; CHECK-LABEL: @test2(
-define void @test2(i1 %B) personality i32 (...)* @__CxxFrameHandler3 {
-entry:
-  invoke void @f()
-          to label %invoke.cont unwind label %ehcleanup
-
-invoke.cont:
-  unreachable
-
-ehcleanup:
-  %cp = cleanuppad within none []
-  br i1 %B, label %ret, label %if.then
-
-if.then:
-  call void @exit(i32 1) [ "funclet"(token %cp) ]
-  unreachable
-
-ret:
-  cleanupret from %cp unwind to caller
-}
-
-; CHECK: call void @exit(i32 1) [ "funclet"(token %cp) ]
-; CHECK-NEXT: unreachable
-
-declare void @f()
-declare void @exit(i32) nounwind noreturn
-
-declare i32 @__CxxFrameHandler3(...)

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=259338&r1=259337&r2=259338&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/WinEH/wineh-cloning.ll (original)
+++ llvm/trunk/test/CodeGen/WinEH/wineh-cloning.ll Sun Jan 31 21:29:38 2016
@@ -44,7 +44,7 @@ noreturn:
 ; CHECK:   call void @llvm.foo(i32 %x)
 
 
-define void @test2() personality i32 (...)* @__C_specific_handler {
+define void @test2() personality i32 (...)* @__CxxFrameHandler3 {
 entry:
   invoke void @f()
     to label %exit unwind label %cleanup
@@ -71,7 +71,7 @@ exit:
 ; CHECK-NEXT: ret void
 
 
-define void @test3() personality i32 (...)* @__C_specific_handler {
+define void @test3() personality i32 (...)* @__CxxFrameHandler3 {
 entry:
   invoke void @f()
     to label %invoke.cont unwind label %catch.switch

Modified: llvm/trunk/test/CodeGen/WinEH/wineh-demotion.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/WinEH/wineh-demotion.ll?rev=259338&r1=259337&r2=259338&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/WinEH/wineh-demotion.ll (original)
+++ llvm/trunk/test/CodeGen/WinEH/wineh-demotion.ll Sun Jan 31 21:29:38 2016
@@ -1,7 +1,6 @@
 ; RUN: opt -mtriple=x86_64-pc-windows-msvc -S -winehprepare  < %s | FileCheck %s
 
 declare i32 @__CxxFrameHandler3(...)
-declare i32 @__C_specific_handler(...)
 
 declare void @f()
 
@@ -328,7 +327,7 @@ exit:
 }
 
 ; CHECK-LABEL: @test8(
-define void @test8() personality i32 (...)* @__C_specific_handler { entry:
+define void @test8() personality i32 (...)* @__CxxFrameHandler3 { entry:
   invoke void @f()
           to label %done unwind label %cleanup1
   invoke void @f()

Modified: llvm/trunk/test/CodeGen/WinEH/wineh-no-demotion.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/WinEH/wineh-no-demotion.ll?rev=259338&r1=259337&r2=259338&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/WinEH/wineh-no-demotion.ll (original)
+++ llvm/trunk/test/CodeGen/WinEH/wineh-no-demotion.ll Sun Jan 31 21:29:38 2016
@@ -1,4 +1,4 @@
-; RUN: opt -mtriple=x86_64-pc-windows-msvc -S -winehprepare -disable-demotion -disable-cleanups < %s | FileCheck %s
+; RUN: opt -mtriple=x86_x64-pc-windows-msvc -S -winehprepare -disable-demotion -disable-cleanups < %s | FileCheck %s
 
 declare i32 @__CxxFrameHandler3(...)
 




More information about the llvm-commits mailing list