r251576 - [WinEH] Mark calls inside cleanups as noinline

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 28 16:06:42 PDT 2015


Author: rnk
Date: Wed Oct 28 18:06:42 2015
New Revision: 251576

URL: http://llvm.org/viewvc/llvm-project?rev=251576&view=rev
Log:
[WinEH] Mark calls inside cleanups as noinline

This works around PR25162. The MSVC tables make it very difficult to
correctly inline a C++ destructor that contains try / catch.  We've
attempted to address PR25162 in LLVM's backend, but it feels pretty
infeasible.  MSVC and ICC both appear to avoid inlining such complex
destructors.

Long term, we want to fix this by making the inliner smart enough to
know when it is inlining into a cleanup, so it can inline simple
destructors (~unique_ptr and ~vector) while avoiding destructors
containing try / catch.

Modified:
    cfe/trunk/lib/CodeGen/CGCall.cpp
    cfe/trunk/lib/CodeGen/CGCleanup.cpp
    cfe/trunk/lib/CodeGen/CodeGenFunction.h
    cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=251576&r1=251575&r2=251576&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Oct 28 18:06:42 2015
@@ -3440,8 +3440,14 @@ RValue CodeGenFunction::EmitCall(const C
         Attrs.addAttribute(getLLVMContext(), llvm::AttributeSet::FunctionIndex,
                            llvm::Attribute::AlwaysInline);
 
-  // Disable inlining inside SEH __try blocks.
-  if (isSEHTryScope())
+  // Disable inlining inside SEH __try blocks and cleanup funclets. None of the
+  // funclet EH personalities that clang supports have tables that are
+  // expressive enough to describe catching an exception inside a cleanup.
+  // __CxxFrameHandler3, for example, will terminate the program without
+  // catching it.
+  // FIXME: Move this decision to the LLVM inliner. Before we can do that, the
+  // inliner needs to know if a given call site is part of a cleanuppad.
+  if (isSEHTryScope() || isCleanupPadScope())
     Attrs =
         Attrs.addAttribute(getLLVMContext(), llvm::AttributeSet::FunctionIndex,
                            llvm::Attribute::NoInline);

Modified: cfe/trunk/lib/CodeGen/CGCleanup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCleanup.cpp?rev=251576&r1=251575&r2=251576&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCleanup.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCleanup.cpp Wed Oct 28 18:06:42 2015
@@ -19,6 +19,7 @@
 
 #include "CGCleanup.h"
 #include "CodeGenFunction.h"
+#include "llvm/Support/SaveAndRestore.h"
 
 using namespace clang;
 using namespace CodeGen;
@@ -902,19 +903,19 @@ void CodeGenFunction::PopCleanupBlock(bo
 
     EmitBlock(EHEntry);
 
-    // Push terminate scopes around the potentially throwing destructor calls.
-    // We don't emit these when using funclets, because the runtime does it for
-    // us as part of unwinding out of a cleanuppad.
+    llvm::BasicBlock *NextAction = getEHDispatchBlock(EHParent);
+
+    // Push a terminate scope or cleanupendpad scope around the potentially
+    // throwing cleanups. For funclet EH personalities, the cleanupendpad models
+    // program termination when cleanups throw.
     bool PushedTerminate = false;
+    SaveAndRestore<bool> RestoreIsCleanupPadScope(IsCleanupPadScope);
+    llvm::CleanupPadInst *CPI = nullptr;
+    llvm::BasicBlock *CleanupEndBB = nullptr;
     if (!EHPersonality::get(*this).usesFuncletPads()) {
       EHStack.pushTerminate();
       PushedTerminate = true;
-    }
-
-    llvm::CleanupPadInst *CPI = nullptr;
-    llvm::BasicBlock *CleanupEndBB = nullptr;
-    llvm::BasicBlock *NextAction = getEHDispatchBlock(EHParent);
-    if (EHPersonality::get(*this).usesFuncletPads()) {
+    } else {
       CPI = Builder.CreateCleanupPad({});
 
       // Build a cleanupendpad to unwind through. Our insertion point should be
@@ -922,6 +923,10 @@ void CodeGenFunction::PopCleanupBlock(bo
       CleanupEndBB = createBasicBlock("ehcleanup.end");
       CGBuilderTy(*this, CleanupEndBB).CreateCleanupEndPad(CPI, NextAction);
       EHStack.pushPadEnd(CleanupEndBB);
+
+      // Mark that we're inside a cleanuppad to block inlining.
+      // FIXME: Remove this once the inliner knows when it's safe to do so.
+      IsCleanupPadScope = true;
     }
 
     // We only actually emit the cleanup code if the cleanup is either

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=251576&r1=251575&r2=251576&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Oct 28 18:06:42 2015
@@ -279,6 +279,8 @@ public:
   /// finally block or filter expression.
   bool IsOutlinedSEHHelper;
 
+  bool IsCleanupPadScope = false;
+
   const CodeGen::CGBlockInfo *BlockInfo;
   llvm::Value *BlockPointer;
 
@@ -372,6 +374,9 @@ public:
   /// Returns true inside SEH __try blocks.
   bool isSEHTryScope() const { return !SEHTryEpilogueStack.empty(); }
 
+  /// Returns true while emitting a cleanuppad.
+  bool isCleanupPadScope() const { return IsCleanupPadScope; }
+
   /// pushFullExprCleanup - Push a cleanup to be run at the end of the
   /// current full-expression.  Safe against the possibility that
   /// we're currently inside a conditionally-evaluated expression.

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp?rev=251576&r1=251575&r2=251576&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp Wed Oct 28 18:06:42 2015
@@ -26,7 +26,8 @@ void HasEHCleanup() {
 // WIN32:   ret void
 //
 //    There should be one dtor call for unwinding from the second getA.
-// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"
+// WIN32:   cleanuppad
+// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"({{.*}}) #[[noinline:[0-9]+]]
 // WIN32-NOT: @"\01??1A@@QAE at XZ"
 // WIN32: }
 
@@ -54,14 +55,14 @@ int HasDeactivatedCleanups() {
 //
 // WIN32:   invoke i32 @"\01?TakesTwo@@YAHUA@@0 at Z"([[argmem_ty]]* inalloca %[[argmem]])
 //        Destroy the two const ref temporaries.
-// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"
-// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"
+// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"({{.*}})
+// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"({{.*}})
 // WIN32:   ret i32
 //
 //        Conditionally destroy arg1.
 // WIN32:   %[[cond:.*]] = load i1, i1* %[[isactive]]
 // WIN32:   br i1 %[[cond]]
-// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"(%struct.A* %[[arg1]])
+// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"(%struct.A* %[[arg1]]) #[[noinline]]
 // WIN32: }
 
 // Test putting the cleanups inside a conditional.
@@ -84,7 +85,7 @@ int HasConditionalCleanup(bool cond) {
 // WIN32:   call i32 @"\01?CouldThrow@@YAHXZ"()
 //
 //        Only one dtor in the invoke for arg1
-// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"({{.*}})
+// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"({{.*}}) #[[noinline]]
 // WIN32-NOT: invoke x86_thiscallcc void @"\01??1A@@QAE at XZ"
 // WIN32: }
 
@@ -118,14 +119,14 @@ int HasConditionalDeactivatedCleanups(bo
 //        False condition.
 // WIN32:   invoke i32 @"\01?CouldThrow@@YAHXZ"()
 //        Two normal cleanups for TakeRef args.
-// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"
+// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"({{.*}})
 // WIN32-NOT:   invoke x86_thiscallcc void @"\01??1A@@QAE at XZ"
 // WIN32:   ret i32
 //
 //        Somewhere in the landing pad soup, we conditionally destroy arg1.
 // WIN32:   %[[isactive:.*]] = load i1, i1* %[[arg1_cond]]
 // WIN32:   br i1 %[[isactive]]
-// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"
+// WIN32:   call x86_thiscallcc void @"\01??1A@@QAE at XZ"({{.*}}) #[[noinline]]
 // WIN32: }
 
 namespace crash_on_partial_destroy {
@@ -163,7 +164,7 @@ C::C() { foo(); }
 // WIN32:      getelementptr inbounds i8, i8* %{{.*}}, i32 4
 // WIN32-NOT:  load
 // WIN32:      bitcast i8* %{{.*}} to %"struct.crash_on_partial_destroy::A"*
-// WIN32:      call x86_thiscallcc void @"\01??1A at crash_on_partial_destroy@@UAE at XZ"
+// WIN32:      call x86_thiscallcc void @"\01??1A at crash_on_partial_destroy@@UAE at XZ"({{.*}}) #[[noinline]]
 // WIN32: }
 }
 
@@ -186,7 +187,7 @@ void f() {
 //
 // WIN32: [[lpad]]
 // WIN32-NEXT: cleanuppad
-// WIN32: call x86_thiscallcc void @"\01??1C at dont_call_terminate@@QAE at XZ"({{.*}})
+// WIN32: call x86_thiscallcc void @"\01??1C at dont_call_terminate@@QAE at XZ"({{.*}}) #[[noinline]]
 }
 
 namespace noexcept_false_dtor {
@@ -203,6 +204,9 @@ void f() {
 // WIN32: invoke i32 @"\01?CouldThrow@@YAHXZ"()
 // WIN32: call x86_thiscallcc void @"\01??1D at noexcept_false_dtor@@QAE at XZ"(%"struct.noexcept_false_dtor::D"* %{{.*}})
 // WIN32: cleanuppad
-// WIN32: invoke x86_thiscallcc void @"\01??1D at noexcept_false_dtor@@QAE at XZ"(%"struct.noexcept_false_dtor::D"* %{{.*}})
+// WIN32: invoke x86_thiscallcc void @"\01??1D at noexcept_false_dtor@@QAE at XZ"(%"struct.noexcept_false_dtor::D"* %{{.*}}) #[[invoke_noinline:[0-9]+]]
 // WIN32: cleanupret
 // WIN32: cleanupendpad
+
+// WIN32: attributes #[[noinline]] = { noinline nounwind }
+// WIN32: attributes #[[invoke_noinline]] = { noinline }




More information about the cfe-commits mailing list