Hi, I would like to propose to remove the CleanupHackLevel from CGException.cpp. This hack was originally written for the old exception handling intrinsics, which have several issues with function inliner. (Read [1] for more background on this hack.)
Since LLVM 3.0, the new exception handling mechanism has been proposed and implemented [1]. The landingpad and resume instructions are added to LLVM IR. AFAIK, the new exception handling mechanism works very well, and solved most of the problems. >From the svn log, the CleanupHackLevel code hasn't be used for a long time. It is guarded by a compile-time constant `CleanupHackLevel`. The alternative cases will be considered as the dead code and removed by the compiler. In addition, there is no test case covering these cases. I feel that it's a good time to remove them. I have attached the patch to remove the code. Please have a look. Feel free to let me know if you have any suggestions. Thanks. Sincerely, Logan [1] http://lists.cs.uiuc.edu/pipermail/llvmdev/2011-July/041748.html
From c25de757cf9013f26e97224163d9994246c9d9d4 Mon Sep 17 00:00:00 2001 From: Logan Chien <[email protected]> Date: Mon, 30 Jun 2014 23:45:05 +0800 Subject: [PATCH 1/1] Remove CleanupHackLevel from CGException. The CleanupHackLevel was an hack to avoid misoptimization to the old exception handling intrinsics, which have several issues with function inliner. The landingpad and resume instructions are added to LLVM IR since LLVM 3.0. Now, we have a canonical way to generate exception handling code. Besides, the `CleanupHackLevel` is a compile-time constant, thus the other alternatives has been considered as dead code for a while. This patch removes the dead code, and refines the getEHResumeBlock() slightly. --- lib/CodeGen/CGException.cpp | 127 ++++++------------------------------------ lib/CodeGen/CodeGenFunction.h | 2 - 2 files changed, 16 insertions(+), 113 deletions(-) diff --git a/lib/CodeGen/CGException.cpp b/lib/CodeGen/CGException.cpp index 0938e83..ffcab07 100644 --- a/lib/CodeGen/CGException.cpp +++ b/lib/CodeGen/CGException.cpp @@ -97,24 +97,6 @@ static llvm::Constant *getUnexpectedFn(CodeGenModule &CGM) { return CGM.CreateRuntimeFunction(FTy, "__cxa_call_unexpected"); } -llvm::Constant *CodeGenFunction::getUnwindResumeFn() { - llvm::FunctionType *FTy = - llvm::FunctionType::get(VoidTy, Int8PtrTy, /*IsVarArgs=*/false); - - if (CGM.getLangOpts().SjLjExceptions) - return CGM.CreateRuntimeFunction(FTy, "_Unwind_SjLj_Resume"); - return CGM.CreateRuntimeFunction(FTy, "_Unwind_Resume"); -} - -llvm::Constant *CodeGenFunction::getUnwindResumeOrRethrowFn() { - llvm::FunctionType *FTy = - llvm::FunctionType::get(VoidTy, Int8PtrTy, /*IsVarArgs=*/false); - - if (CGM.getLangOpts().SjLjExceptions) - return CGM.CreateRuntimeFunction(FTy, "_Unwind_SjLj_Resume_or_Rethrow"); - return CGM.CreateRuntimeFunction(FTy, "_Unwind_Resume_or_Rethrow"); -} - static llvm::Constant *getTerminateFn(CodeGenModule &CGM) { // void __terminate(); @@ -721,56 +703,6 @@ llvm::BasicBlock *CodeGenFunction::getInvokeDestImpl() { 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()); @@ -897,11 +829,8 @@ llvm::BasicBlock *CodeGenFunction::EmitLandingPad() { LPadInst->setCleanup(true); // Otherwise, signal that we at least have cleanups. - } else if (CleanupHackLevel == CHL_MandatoryCatchall || hasCleanup) { - if (CleanupHackLevel == CHL_MandatoryCatchall) - LPadInst->addClause(getCatchAllValue(*this)); - else - LPadInst->setCleanup(true); + } else if (hasCleanup) { + LPadInst->setCleanup(true); } assert((LPadInst->getNumClauses() > 0 || LPadInst->isCleanup()) && @@ -1678,49 +1607,25 @@ llvm::BasicBlock *CodeGenFunction::getEHResumeBlock(bool isCleanup) { const char *RethrowName = Personality.CatchallRethrowFn; if (RethrowName != nullptr && !isCleanup) { EmitRuntimeCall(getCatchallRethrowFn(CGM, RethrowName), - getExceptionFromSlot()) + getExceptionFromSlot()) ->setDoesNotReturn(); - } else { - switch (CleanupHackLevel) { - case CHL_MandatoryCatchall: - // In mandatory-catchall mode, we need to use - // _Unwind_Resume_or_Rethrow, or whatever the personality's - // equivalent is. - EmitRuntimeCall(getUnwindResumeOrRethrowFn(), - getExceptionFromSlot()) - ->setDoesNotReturn(); - break; - case CHL_MandatoryCleanup: { - // In mandatory-cleanup mode, we should use 'resume'. - - // Recreate the landingpad's return value for the 'resume' instruction. - llvm::Value *Exn = getExceptionFromSlot(); - llvm::Value *Sel = getSelectorFromSlot(); - - llvm::Type *LPadType = llvm::StructType::get(Exn->getType(), - Sel->getType(), NULL); - llvm::Value *LPadVal = llvm::UndefValue::get(LPadType); - LPadVal = Builder.CreateInsertValue(LPadVal, Exn, 0, "lpad.val"); - LPadVal = Builder.CreateInsertValue(LPadVal, Sel, 1, "lpad.val"); - - Builder.CreateResume(LPadVal); - Builder.restoreIP(SavedIP); - return EHResumeBlock; - } - case CHL_Ideal: - // In an idealized mode where we don't have to worry about the - // optimizer combining landing pads, we should just use - // _Unwind_Resume (or the personality's equivalent). - EmitRuntimeCall(getUnwindResumeFn(), getExceptionFromSlot()) - ->setDoesNotReturn(); - break; - } + Builder.CreateUnreachable(); + Builder.restoreIP(SavedIP); + return EHResumeBlock; } - Builder.CreateUnreachable(); + // Recreate the landingpad's return value for the 'resume' instruction. + llvm::Value *Exn = getExceptionFromSlot(); + llvm::Value *Sel = getSelectorFromSlot(); - Builder.restoreIP(SavedIP); + llvm::Type *LPadType = llvm::StructType::get(Exn->getType(), + Sel->getType(), NULL); + llvm::Value *LPadVal = llvm::UndefValue::get(LPadType); + LPadVal = Builder.CreateInsertValue(LPadVal, Exn, 0, "lpad.val"); + LPadVal = Builder.CreateInsertValue(LPadVal, Sel, 1, "lpad.val"); + Builder.CreateResume(LPadVal); + Builder.restoreIP(SavedIP); return EHResumeBlock; } diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 2fe2b9b..2e40577 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -1884,8 +1884,6 @@ public: void EmitObjCAtSynchronizedStmt(const ObjCAtSynchronizedStmt &S); void EmitObjCAutoreleasePoolStmt(const ObjCAutoreleasePoolStmt &S); - llvm::Constant *getUnwindResumeFn(); - llvm::Constant *getUnwindResumeOrRethrowFn(); void EnterCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock = false); void ExitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock = false); -- 1.9.1
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
