On Fri, May 27, 2011 at 1:01 PM, John McCall <[email protected]> wrote: > Author: rjmccall > Date: Fri May 27 15:01:14 2011 > New Revision: 132209 > > URL: http://llvm.org/viewvc/llvm-project?rev=132209&view=rev > Log: > Implement a new, much improved version of the cleanup hack. We just need > to be careful to emit landing pads that are always prepared to handle a > cleanup path. This is correct mostly because of the fix to the LLVM > inliner, r132200.
This appears to be breaking nightly tests; see http://smooshlab.apple.com:8013/builders/lnt_clang-x86_64-darwin10-gcc42-RA_x86_64-O3/builds/3084 . -Eli > > Modified: > cfe/trunk/lib/CodeGen/CGException.cpp > cfe/trunk/lib/CodeGen/CodeGenFunction.h > cfe/trunk/test/CXX/except/except.spec/p9-dynamic.cpp > cfe/trunk/test/CodeGenCXX/arm.cpp > cfe/trunk/test/CodeGenCXX/destructors.cpp > cfe/trunk/test/CodeGenCXX/eh.cpp > cfe/trunk/test/CodeGenCXX/nrvo.cpp > cfe/trunk/test/CodeGenCXX/threadsafe-statics-exceptions.cpp > cfe/trunk/test/CodeGenObjC/blocks-2.m > cfe/trunk/test/CodeGenObjC/unwind-fn.m > > Modified: cfe/trunk/lib/CodeGen/CGException.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGException.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGException.cpp Fri May 27 15:01:14 2011 > @@ -112,11 +112,18 @@ > return CGF.CGM.CreateRuntimeFunction(FTy, "__cxa_call_unexpected"); > } > > +llvm::Constant *CodeGenFunction::getUnwindResumeFn() { > + const llvm::FunctionType *FTy = > + llvm::FunctionType::get(VoidTy, Int8PtrTy, /*IsVarArgs=*/false); > + > + if (CGM.getLangOptions().SjLjExceptions) > + return CGM.CreateRuntimeFunction(FTy, "_Unwind_SjLj_Resume"); > + return CGM.CreateRuntimeFunction(FTy, "_Unwind_Resume"); > +} > + > llvm::Constant *CodeGenFunction::getUnwindResumeOrRethrowFn() { > - const llvm::Type *Int8PtrTy = llvm::Type::getInt8PtrTy(getLLVMContext()); > const llvm::FunctionType *FTy = > - llvm::FunctionType::get(llvm::Type::getVoidTy(getLLVMContext()), > Int8PtrTy, > - /*IsVarArgs=*/false); > + llvm::FunctionType::get(VoidTy, Int8PtrTy, /*IsVarArgs=*/false); > > if (CGM.getLangOptions().SjLjExceptions) > return CGM.CreateRuntimeFunction(FTy, "_Unwind_SjLj_Resume_or_Rethrow"); > @@ -563,47 +570,59 @@ > return LP; > } > > +// This code contains a hack to work around a design flaw in > +// LLVM's EH IR which breaks semantics after inlining. This same > +// hack is implemented in llvm-gcc. > +// > +// The LLVM EH abstraction is basically a thin veneer over the > +// traditional GCC zero-cost design: for each range of instructions > +// in the function, there is (at most) one "landing pad" with an > +// associated chain of EH actions. A language-specific personality > +// function interprets this chain of actions and (1) decides whether > +// or not to resume execution at the landing pad and (2) if so, > +// provides an integer indicating why it's stopping. In LLVM IR, > +// the association of a landing pad with a range of instructions is > +// achieved via an invoke instruction, the chain of actions becomes > +// the arguments to the @llvm.eh.selector call, and the selector > +// call returns the integer indicator. Other than the required > +// presence of two intrinsic function calls in the landing pad, > +// the IR exactly describes the layout of the output code. > +// > +// A principal advantage of this design is that it is completely > +// language-agnostic; in theory, the LLVM optimizers can treat > +// landing pads neutrally, and targets need only know how to lower > +// the intrinsics to have a functioning exceptions system (assuming > +// that platform exceptions follow something approximately like the > +// GCC design). Unfortunately, landing pads cannot be combined in a > +// language-agnostic way: given selectors A and B, there is no way > +// to make a single landing pad which faithfully represents the > +// semantics of propagating an exception first through A, then > +// through B, without knowing how the personality will interpret the > +// (lowered form of the) selectors. This means that inlining has no > +// choice but to crudely chain invokes (i.e., to ignore invokes in > +// the inlined function, but to turn all unwindable calls into > +// invokes), which is only semantically valid if every unwind stops > +// at every landing pad. > +// > +// Therefore, the invoke-inline hack is to guarantee that every > +// landing pad has a catch-all. > +enum CleanupHackLevel_t { > + /// A level of hack that requires that all landing pads have > + /// catch-alls. > + CHL_MandatoryCatchall, > + > + /// A level of hack that requires that all landing pads handle > + /// cleanups. > + CHL_MandatoryCleanup, > + > + /// No hacks at all; ideal IR generation. > + CHL_Ideal > +}; > +const CleanupHackLevel_t CleanupHackLevel = CHL_MandatoryCleanup; > + > llvm::BasicBlock *CodeGenFunction::EmitLandingPad() { > assert(EHStack.requiresLandingPad()); > > - // This function contains a hack to work around a design flaw in > - // LLVM's EH IR which breaks semantics after inlining. This same > - // hack is implemented in llvm-gcc. > - // > - // The LLVM EH abstraction is basically a thin veneer over the > - // traditional GCC zero-cost design: for each range of instructions > - // in the function, there is (at most) one "landing pad" with an > - // associated chain of EH actions. A language-specific personality > - // function interprets this chain of actions and (1) decides whether > - // or not to resume execution at the landing pad and (2) if so, > - // provides an integer indicating why it's stopping. In LLVM IR, > - // the association of a landing pad with a range of instructions is > - // achieved via an invoke instruction, the chain of actions becomes > - // the arguments to the @llvm.eh.selector call, and the selector > - // call returns the integer indicator. Other than the required > - // presence of two intrinsic function calls in the landing pad, > - // the IR exactly describes the layout of the output code. > - // > - // A principal advantage of this design is that it is completely > - // language-agnostic; in theory, the LLVM optimizers can treat > - // landing pads neutrally, and targets need only know how to lower > - // the intrinsics to have a functioning exceptions system (assuming > - // that platform exceptions follow something approximately like the > - // GCC design). Unfortunately, landing pads cannot be combined in a > - // language-agnostic way: given selectors A and B, there is no way > - // to make a single landing pad which faithfully represents the > - // semantics of propagating an exception first through A, then > - // through B, without knowing how the personality will interpret the > - // (lowered form of the) selectors. This means that inlining has no > - // choice but to crudely chain invokes (i.e., to ignore invokes in > - // the inlined function, but to turn all unwindable calls into > - // invokes), which is only semantically valid if every unwind stops > - // at every landing pad. > - // > - // Therefore, the invoke-inline hack is to guarantee that every > - // landing pad has a catch-all. > - const bool UseInvokeInlineHack = true; > - > for (EHScopeStack::iterator ir = EHStack.begin(); ; ) { > assert(ir != EHStack.end() && > "stack requiring landing pad is nothing but non-EH scopes?"); > @@ -736,16 +755,23 @@ > EHSelector.append(EHFilters.begin(), EHFilters.end()); > > // Also check whether we need a cleanup. > - if (UseInvokeInlineHack || HasEHCleanup) > - EHSelector.push_back(UseInvokeInlineHack > + if (CleanupHackLevel == CHL_MandatoryCatchall || HasEHCleanup) > + EHSelector.push_back(CleanupHackLevel == CHL_MandatoryCatchall > ? getCatchAllValue(*this) > : getCleanupValue(*this)); > > // Otherwise, signal that we at least have cleanups. > - } else if (UseInvokeInlineHack || HasEHCleanup) { > - EHSelector.push_back(UseInvokeInlineHack > + } else if (CleanupHackLevel == CHL_MandatoryCatchall || HasEHCleanup) { > + EHSelector.push_back(CleanupHackLevel == CHL_MandatoryCatchall > ? getCatchAllValue(*this) > : getCleanupValue(*this)); > + > + // At the MandatoryCleanup hack level, we don't need to actually > + // spuriously tell the unwinder that we have cleanups, but we do > + // need to always be prepared to handle cleanups. > + } else if (CleanupHackLevel == CHL_MandatoryCleanup) { > + // Just don't decrement LastToEmitInLoop. > + > } else { > assert(LastToEmitInLoop > 2); > LastToEmitInLoop--; > @@ -833,7 +859,7 @@ > > // If there was a cleanup, we'll need to actually check whether we > // landed here because the filter triggered. > - if (UseInvokeInlineHack || HasEHCleanup) { > + if (CleanupHackLevel != CHL_Ideal || HasEHCleanup) { > llvm::BasicBlock *RethrowBB = createBasicBlock("cleanup"); > llvm::BasicBlock *UnexpectedBB = createBasicBlock("ehspec.unexpected"); > > @@ -843,10 +869,11 @@ > Builder.CreateCondBr(FailsFilter, UnexpectedBB, RethrowBB); > > // The rethrow block is where we land if this was a cleanup. > - // TODO: can this be _Unwind_Resume if the InvokeInlineHack is off? > EmitBlock(RethrowBB); > - Builder.CreateCall(getUnwindResumeOrRethrowFn(), > - Builder.CreateLoad(getExceptionSlot())) > + llvm::Constant *RethrowFn = > + CleanupHackLevel == CHL_MandatoryCatchall ? > getUnwindResumeOrRethrowFn() > + : getUnwindResumeFn(); > + Builder.CreateCall(RethrowFn, Builder.CreateLoad(getExceptionSlot())) > ->setDoesNotReturn(); > Builder.CreateUnreachable(); > > @@ -863,7 +890,7 @@ > Builder.CreateUnreachable(); > > // ...or a normal catch handler... > - } else if (!UseInvokeInlineHack && !HasEHCleanup) { > + } else if (CleanupHackLevel == CHL_Ideal && !HasEHCleanup) { > llvm::Value *Type = EHSelector.back(); > EmitBranchThroughEHCleanup(EHHandlers[Type]); > > @@ -1444,7 +1471,9 @@ > if (!RethrowName.empty()) > RethrowFn = getCatchallRethrowFn(*this, RethrowName); > else > - RethrowFn = getUnwindResumeOrRethrowFn(); > + RethrowFn = (CleanupHackLevel == CHL_MandatoryCatchall > + ? getUnwindResumeOrRethrowFn() > + : getUnwindResumeFn()); > > Builder.CreateCall(RethrowFn, Builder.CreateLoad(getExceptionSlot())) > ->setDoesNotReturn(); > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri May 27 15:01:14 2011 > @@ -1705,6 +1705,7 @@ > void EmitObjCAtThrowStmt(const ObjCAtThrowStmt &S); > void EmitObjCAtSynchronizedStmt(const ObjCAtSynchronizedStmt &S); > > + llvm::Constant *getUnwindResumeFn(); > llvm::Constant *getUnwindResumeOrRethrowFn(); > void EnterCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock = false); > void ExitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock = false); > > Modified: cfe/trunk/test/CXX/except/except.spec/p9-dynamic.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p9-dynamic.cpp?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/test/CXX/except/except.spec/p9-dynamic.cpp (original) > +++ cfe/trunk/test/CXX/except/except.spec/p9-dynamic.cpp Fri May 27 15:01:14 > 2011 > @@ -7,5 +7,5 @@ > // CHECK: invoke void @_Z8externalv() > external(); > } > -// CHECK: call i32 (i8*, i8*, ...)* @llvm.eh.selector({{.*}} i8* bitcast > (i8** @_ZTIi to i8*), i8* null) nounwind > +// CHECK: call i32 (i8*, i8*, ...)* @llvm.eh.selector({{.*}} i8* bitcast > (i8** @_ZTIi to i8*)) nounwind > // CHECK: call void @__cxa_call_unexpected > > Modified: cfe/trunk/test/CodeGenCXX/arm.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/arm.cpp?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/arm.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/arm.cpp Fri May 27 15:01:14 2011 > @@ -310,7 +310,7 @@ > > // CHECK: call i8* @llvm.eh.exception() > // CHECK: call void @__cxa_guard_abort(i32* @_ZGVZN5test74testEvE1x) > - // CHECK: call void @_Unwind_Resume_or_Rethrow > + // CHECK: call void @_Unwind_Resume( > } > } > > @@ -349,7 +349,7 @@ > > // CHECK: call i8* @llvm.eh.exception() > // CHECK: call void @__cxa_guard_abort(i32* @_ZGVZN5test84testEvE1x) > - // CHECK: call void @_Unwind_Resume_or_Rethrow > + // CHECK: call void @_Unwind_Resume( > } > } > > > Modified: cfe/trunk/test/CodeGenCXX/destructors.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/destructors.cpp?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/destructors.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/destructors.cpp Fri May 27 15:01:14 2011 > @@ -334,7 +334,7 @@ > // CHECK: ret void > // CHECK: call i8* @llvm.eh.exception( > // CHECK: call void @_ZdlPv({{.*}}) nounwind > - // CHECK: call void @_Unwind_Resume_or_Rethrow > + // CHECK: call void @_Unwind_Resume( > > // Checked at top of file: > // @_ZN5test312_GLOBAL__N_11DD1Ev = alias internal {{.*}} > @_ZN5test312_GLOBAL__N_11DD2Ev > @@ -364,7 +364,7 @@ > // CHECK: ret void > // CHECK: call i8* @llvm.eh.exception() > // CHECK: call void @_ZdlPv({{.*}}) nounwind > - // CHECK: call void @_Unwind_Resume_or_Rethrow( > + // CHECK: call void @_Unwind_Resume( > > // CHECK: define internal void @_ZThn8_N5test312_GLOBAL__N_11CD1Ev( > // CHECK: getelementptr inbounds i8* {{.*}}, i64 -8 > > Modified: cfe/trunk/test/CodeGenCXX/eh.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/eh.cpp?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/eh.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/eh.cpp Fri May 27 15:01:14 2011 > @@ -190,7 +190,7 @@ > > // landing pad from first call to invoke > // CHECK: call i8* @llvm.eh.exception > - // CHECK: call i32 (i8*, i8*, ...)* @llvm.eh.selector(i8* {{.*}}, i8* > bitcast (i32 (...)* @__gxx_personality_v0 to i8*), i8* bitcast (i8** @_ZTIi > to i8*), i8* null) > + // CHECK: call i32 (i8*, i8*, ...)* @llvm.eh.selector(i8* {{.*}}, i8* > bitcast (i32 (...)* @__gxx_personality_v0 to i8*), i8* bitcast (i8** @_ZTIi > to i8*)) > } > > // __cxa_end_catch can throw for some kinds of caught exceptions. > > Modified: cfe/trunk/test/CodeGenCXX/nrvo.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/nrvo.cpp?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/nrvo.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/nrvo.cpp Fri May 27 15:01:14 2011 > @@ -95,7 +95,7 @@ > > // %invoke.cont17: rethrow block for %eh.cleanup. > // This really should be elsewhere in the function. > - // CHECK-EH: call void @_Unwind_Resume_or_Rethrow > + // CHECK-EH: call void @_Unwind_Resume( > // CHECK-EH-NEXT: unreachable > > // %terminate.lpad: terminate landing pad. > > Modified: cfe/trunk/test/CodeGenCXX/threadsafe-statics-exceptions.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/threadsafe-statics-exceptions.cpp?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenCXX/threadsafe-statics-exceptions.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/threadsafe-statics-exceptions.cpp Fri May 27 > 15:01:14 2011 > @@ -24,6 +24,6 @@ > // CHECK: call i8* @llvm.eh.exception() > // CHECK: call i32 (i8*, i8*, ...)* @llvm.eh.selector > // CHECK: call void @__cxa_guard_abort(i64* @_ZGVZ1fvE1x) > - // CHECK: call void @_Unwind_Resume_or_Rethrow > + // CHECK: call void @_Unwind_Resume( > // CHECK: unreachable > } > > Modified: cfe/trunk/test/CodeGenObjC/blocks-2.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/blocks-2.m?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenObjC/blocks-2.m (original) > +++ cfe/trunk/test/CodeGenObjC/blocks-2.m Fri May 27 15:01:14 2011 > @@ -33,5 +33,5 @@ > // CHECK: call i8* @llvm.eh.exception() > // CHECK: [[T1:%.*]] = bitcast [[N_T]]* [[N]] to i8* > // CHECK-NEXT: call void @_Block_object_dispose(i8* [[T1]], i32 8) > - // CHECK: call void @_Unwind_Resume_or_Rethrow( > + // CHECK: call void @_Unwind_Resume( > } > > Modified: cfe/trunk/test/CodeGenObjC/unwind-fn.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/unwind-fn.m?rev=132209&r1=132208&r2=132209&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenObjC/unwind-fn.m (original) > +++ cfe/trunk/test/CodeGenObjC/unwind-fn.m Fri May 27 15:01:14 2011 > @@ -1,8 +1,8 @@ > // RUN: %clang_cc1 -fobjc-nonfragile-abi -emit-llvm -fexceptions > -fobjc-exceptions -o - %s | FileCheck --check-prefix=DEFAULT_EH %s > // RUN: %clang_cc1 -fsjlj-exceptions -fobjc-nonfragile-abi -fexceptions > -fobjc-exceptions -emit-llvm -o - %s | FileCheck --check-prefix=SJLJ_EH %s > > -// DEFAULT_EH: declare void @_Unwind_Resume_or_Rethrow(i8*) > -// SJLJ_EH: declare void @_Unwind_SjLj_Resume_or_Rethrow(i8*) > +// DEFAULT_EH: declare void @_Unwind_Resume(i8*) > +// SJLJ_EH: declare void @_Unwind_SjLj_Resume(i8*) > > void f1(), f2(); > void f0() { > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
