[llvm-branch-commits] [cfe-branch] r111409 - in /cfe/branches/Apple/williamson: lib/CodeGen/CGObjCMac.cpp test/CodeGenObjC/exceptions.m test/CodeGenObjC/synchronized.m
Daniel Dunbar
daniel at zuster.org
Wed Aug 18 13:33:25 PDT 2010
Author: ddunbar
Date: Wed Aug 18 15:33:25 2010
New Revision: 111409
URL: http://llvm.org/viewvc/llvm-project?rev=111409&view=rev
Log:
Merge r110188:
--
Author: John McCall <rjmccall at apple.com>
Date: Wed Aug 4 05:59:32 2010 +0000
Some more correctness fixes and code-size optimizations for fragile-ABI
ObjC exceptions:
- don't enter a try for the catch blocks unless there's a finally
- put the setjmp buffer in the locals set for liveness reasons
- dump the sync object into an alloca in the locals set for liveness reasons
Some of this can go away if the backend starts to properly calculate liveness
in the presence of setjmp (which would also be a *much* stabler solution).
Modified:
cfe/branches/Apple/williamson/lib/CodeGen/CGObjCMac.cpp
cfe/branches/Apple/williamson/test/CodeGenObjC/exceptions.m
cfe/branches/Apple/williamson/test/CodeGenObjC/synchronized.m
Modified: cfe/branches/Apple/williamson/lib/CodeGen/CGObjCMac.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/williamson/lib/CodeGen/CGObjCMac.cpp?rev=111409&r1=111408&r2=111409&view=diff
==============================================================================
--- cfe/branches/Apple/williamson/lib/CodeGen/CGObjCMac.cpp (original)
+++ cfe/branches/Apple/williamson/lib/CodeGen/CGObjCMac.cpp Wed Aug 18 15:33:25 2010
@@ -2554,16 +2554,16 @@
namespace {
struct PerformFragileFinally : EHScopeStack::Cleanup {
const Stmt &S;
- llvm::Value *SyncArg;
+ llvm::Value *SyncArgSlot;
llvm::Value *CallTryExitVar;
llvm::Value *ExceptionData;
ObjCTypesHelper &ObjCTypes;
PerformFragileFinally(const Stmt *S,
- llvm::Value *SyncArg,
+ llvm::Value *SyncArgSlot,
llvm::Value *CallTryExitVar,
llvm::Value *ExceptionData,
ObjCTypesHelper *ObjCTypes)
- : S(*S), SyncArg(SyncArg), CallTryExitVar(CallTryExitVar),
+ : S(*S), SyncArgSlot(SyncArgSlot), CallTryExitVar(CallTryExitVar),
ExceptionData(ExceptionData), ObjCTypes(*ObjCTypes) {}
void Emit(CodeGenFunction &CGF, bool IsForEH) {
@@ -2592,6 +2592,7 @@
} else {
// Emit objc_sync_exit(expr); as finally's sole statement for
// @synchronized.
+ llvm::Value *SyncArg = CGF.Builder.CreateLoad(SyncArgSlot);
CGF.Builder.CreateCall(ObjCTypes.getSyncExitFn(), SyncArg)
->setDoesNotThrow();
}
@@ -2613,9 +2614,9 @@
public:
FragileHazards(CodeGenFunction &CGF);
- void emitReadHazard();
- void emitReadHazardsInNewBlocks();
+
void emitWriteHazard();
+ void emitHazardsInNewBlocks();
};
}
@@ -2674,11 +2675,6 @@
->setDoesNotThrow();
}
-void FragileHazards::emitReadHazard() {
- if (Locals.empty()) return;
- emitReadHazard(CGF.Builder);
-}
-
void FragileHazards::emitReadHazard(CGBuilderTy &Builder) {
assert(!Locals.empty());
Builder.CreateCall(ReadHazard, Locals.begin(), Locals.end())
@@ -2687,7 +2683,7 @@
/// Emit read hazards in all the protected blocks, i.e. all the blocks
/// which have been inserted since the beginning of the try.
-void FragileHazards::emitReadHazardsInNewBlocks() {
+void FragileHazards::emitHazardsInNewBlocks() {
if (Locals.empty()) return;
CGBuilderTy Builder(CGF.getLLVMContext());
@@ -2714,7 +2710,11 @@
llvm::CallSite CS(&I);
if (CS.doesNotThrow()) continue;
- // Insert a read hazard before the call.
+ // Insert a read hazard before the call. This will ensure that
+ // any writes to the locals are performed before making the
+ // call. If the call throws, then this is sufficient to
+ // guarantee correctness as long as it doesn't also write to any
+ // locals.
Builder.SetInsertPoint(&BB, BI);
emitReadHazard(Builder);
}
@@ -2876,42 +2876,46 @@
// For @synchronized, call objc_sync_enter(sync.expr). The
// evaluation of the expression must occur before we enter the
- // @synchronized. We can safely avoid a temp here because jumps into
- // @synchronized are illegal & this will dominate uses.
- llvm::Value *SyncArg = 0;
+ // @synchronized. We can't avoid a temp here because we need the
+ // value to be preserved. If the backend ever does liveness
+ // correctly after setjmp, this will be unnecessary.
+ llvm::Value *SyncArgSlot = 0;
if (!isTry) {
- SyncArg =
+ llvm::Value *SyncArg =
CGF.EmitScalarExpr(cast<ObjCAtSynchronizedStmt>(S).getSynchExpr());
SyncArg = CGF.Builder.CreateBitCast(SyncArg, ObjCTypes.ObjectPtrTy);
CGF.Builder.CreateCall(ObjCTypes.getSyncEnterFn(), SyncArg)
->setDoesNotThrow();
+
+ SyncArgSlot = CGF.CreateTempAlloca(SyncArg->getType(), "sync.arg");
+ CGF.Builder.CreateStore(SyncArg, SyncArgSlot);
}
+ // Allocate memory for the setjmp buffer. This needs to be kept
+ // live throughout the try and catch blocks.
+ llvm::Value *ExceptionData = CGF.CreateTempAlloca(ObjCTypes.ExceptionDataTy,
+ "exceptiondata.ptr");
+
// Create the fragile hazards. Note that this will not capture any
// of the allocas required for exception processing, but will
// capture the current basic block (which extends all the way to the
// setjmp call) as "before the @try".
FragileHazards Hazards(CGF);
- // Allocate memory for the exception data and rethrow pointer.
- llvm::Value *ExceptionData = CGF.CreateTempAlloca(ObjCTypes.ExceptionDataTy,
- "exceptiondata.ptr");
- llvm::Value *RethrowPtr = CGF.CreateTempAlloca(ObjCTypes.ObjectPtrTy,
- "_rethrow");
-
// Create a flag indicating whether the cleanup needs to call
// objc_exception_try_exit. This is true except when
// - no catches match and we're branching through the cleanup
// just to rethrow the exception, or
// - a catch matched and we're falling out of the catch handler.
+ // The setjmp-safety rule here is that we should always store to this
+ // variable in a place that dominates the branch through the cleanup
+ // without passing through any setjmps.
llvm::Value *CallTryExitVar = CGF.CreateTempAlloca(CGF.Builder.getInt1Ty(),
"_call_try_exit");
- CGF.Builder.CreateStore(llvm::ConstantInt::getTrue(VMContext),
- CallTryExitVar);
// Push a normal cleanup to leave the try scope.
CGF.EHStack.pushCleanup<PerformFragileFinally>(NormalCleanup, &S,
- SyncArg,
+ SyncArgSlot,
CallTryExitVar,
ExceptionData,
&ObjCTypes);
@@ -2941,9 +2945,11 @@
// Emit the protected block.
CGF.EmitBlock(TryBlock);
+ CGF.Builder.CreateStore(CGF.Builder.getTrue(), CallTryExitVar);
CGF.EmitStmt(isTry ? cast<ObjCAtTryStmt>(S).getTryBody()
: cast<ObjCAtSynchronizedStmt>(S).getSynchBody());
- CGF.EmitBranchThroughCleanup(FinallyEnd);
+
+ CGBuilderTy::InsertPoint TryFallthroughIP = CGF.Builder.saveAndClearIP();
// Emit the exception handler block.
CGF.EmitBlock(TryHandler);
@@ -2951,56 +2957,54 @@
// Don't optimize loads of the in-scope locals across this point.
Hazards.emitWriteHazard();
- // Retrieve the exception object. We may emit multiple blocks but
- // nothing can cross this so the value is already in SSA form.
- llvm::CallInst *Caught =
- CGF.Builder.CreateCall(ObjCTypes.getExceptionExtractFn(),
- ExceptionData, "caught");
- Caught->setDoesNotThrow();
-
- // Remember the exception to rethrow.
- CGF.Builder.CreateStore(Caught, RethrowPtr);
-
- // Note: at this point, objc_exception_throw already popped the
- // catch handler, so anything that branches to the cleanup needs
- // to set CallTryExitVar to false.
-
// For a @synchronized (or a @try with no catches), just branch
// through the cleanup to the rethrow block.
if (!isTry || !cast<ObjCAtTryStmt>(S).getNumCatchStmts()) {
// Tell the cleanup not to re-pop the exit.
- CGF.Builder.CreateStore(llvm::ConstantInt::getFalse(VMContext),
- CallTryExitVar);
-
+ CGF.Builder.CreateStore(CGF.Builder.getFalse(), CallTryExitVar);
CGF.EmitBranchThroughCleanup(FinallyRethrow);
// Otherwise, we have to match against the caught exceptions.
} else {
+ // Retrieve the exception object. We may emit multiple blocks but
+ // nothing can cross this so the value is already in SSA form.
+ llvm::CallInst *Caught =
+ CGF.Builder.CreateCall(ObjCTypes.getExceptionExtractFn(),
+ ExceptionData, "caught");
+ Caught->setDoesNotThrow();
+
// Push the exception to rethrow onto the EH value stack for the
// benefit of any @throws in the handlers.
CGF.ObjCEHValueStack.push_back(Caught);
const ObjCAtTryStmt* AtTryStmt = cast<ObjCAtTryStmt>(&S);
-
- // Enter a new exception try block (in case a @catch block throws
- // an exception). Now CallTryExitVar (currently true) is back in
- // synch with reality.
- CGF.Builder.CreateCall(ObjCTypes.getExceptionTryEnterFn(), ExceptionData)
- ->setDoesNotThrow();
- llvm::CallInst *SetJmpResult =
- CGF.Builder.CreateCall(ObjCTypes.getSetJmpFn(), SetJmpBuffer,
- "setjmp.result");
- SetJmpResult->setDoesNotThrow();
-
- llvm::Value *Threw =
- CGF.Builder.CreateIsNotNull(SetJmpResult, "did_catch_exception");
-
- llvm::BasicBlock *CatchBlock = CGF.createBasicBlock("catch");
- llvm::BasicBlock *CatchHandler = CGF.createBasicBlock("catch_for_catch");
- CGF.Builder.CreateCondBr(Threw, CatchHandler, CatchBlock);
+ bool HasFinally = (AtTryStmt->getFinallyStmt() != 0);
+
+ llvm::BasicBlock *CatchBlock = 0;
+ llvm::BasicBlock *CatchHandler = 0;
+ if (HasFinally) {
+ // Enter a new exception try block (in case a @catch block
+ // throws an exception).
+ CGF.Builder.CreateCall(ObjCTypes.getExceptionTryEnterFn(), ExceptionData)
+ ->setDoesNotThrow();
+
+ llvm::CallInst *SetJmpResult =
+ CGF.Builder.CreateCall(ObjCTypes.getSetJmpFn(), SetJmpBuffer,
+ "setjmp.result");
+ SetJmpResult->setDoesNotThrow();
+
+ llvm::Value *Threw =
+ CGF.Builder.CreateIsNotNull(SetJmpResult, "did_catch_exception");
- CGF.EmitBlock(CatchBlock);
+ CatchBlock = CGF.createBasicBlock("catch");
+ CatchHandler = CGF.createBasicBlock("catch_for_catch");
+ CGF.Builder.CreateCondBr(Threw, CatchHandler, CatchBlock);
+
+ CGF.EmitBlock(CatchBlock);
+ }
+
+ CGF.Builder.CreateStore(CGF.Builder.getInt1(HasFinally), CallTryExitVar);
// Handle catch list. As a special case we check if everything is
// matched and avoid generating code for falling off the end if
@@ -3073,7 +3077,7 @@
// Collect any cleanups for the catch variable. The scope lasts until
// the end of the catch body.
- CodeGenFunction::RunCleanupsScope CatchVarCleanups(CGF);
+ CodeGenFunction::RunCleanupsScope CatchVarCleanups(CGF);
CGF.EmitLocalBlockVarDecl(*CatchParam);
assert(CGF.HaveInsertPoint() && "DeclStmt destroyed insert point?");
@@ -3097,41 +3101,50 @@
CGF.ObjCEHValueStack.pop_back();
- if (!AllMatched) {
- // None of the handlers caught the exception, so store it to be
- // rethrown at the end of the @finally block.
- CGF.Builder.CreateStore(Caught, RethrowPtr);
- CGF.EmitBranchThroughCleanup(FinallyRethrow);
- }
+ // If nothing wanted anything to do with the caught exception,
+ // kill the extract call.
+ if (Caught->use_empty())
+ Caught->eraseFromParent();
- // Emit the exception handler for the @catch blocks.
- CGF.EmitBlock(CatchHandler);
+ if (!AllMatched)
+ CGF.EmitBranchThroughCleanup(FinallyRethrow);
- // Rethrow the new exception, not the old one.
- Caught = CGF.Builder.CreateCall(ObjCTypes.getExceptionExtractFn(),
- ExceptionData);
- Caught->setDoesNotThrow();
- CGF.Builder.CreateStore(Caught, RethrowPtr);
+ if (HasFinally) {
+ // Emit the exception handler for the @catch blocks.
+ CGF.EmitBlock(CatchHandler);
+
+ // In theory we might now need a write hazard, but actually it's
+ // unnecessary because there's no local-accessing code between
+ // the try's write hazard and here.
+ //Hazards.emitWriteHazard();
- // Don't pop the catch handler; the throw already did.
- CGF.Builder.CreateStore(llvm::ConstantInt::getFalse(VMContext),
- CallTryExitVar);
- CGF.EmitBranchThroughCleanup(FinallyRethrow);
+ // Don't pop the catch handler; the throw already did.
+ CGF.Builder.CreateStore(CGF.Builder.getFalse(), CallTryExitVar);
+ CGF.EmitBranchThroughCleanup(FinallyRethrow);
+ }
}
// Insert read hazards as required in the new blocks.
- Hazards.emitReadHazardsInNewBlocks();
+ Hazards.emitHazardsInNewBlocks();
// Pop the cleanup.
+ CGF.Builder.restoreIP(TryFallthroughIP);
+ if (CGF.HaveInsertPoint())
+ CGF.Builder.CreateStore(CGF.Builder.getTrue(), CallTryExitVar);
CGF.PopCleanupBlock();
- CGF.EmitBlock(FinallyEnd.getBlock());
+ CGF.EmitBlock(FinallyEnd.getBlock(), true);
// Emit the rethrow block.
CGBuilderTy::InsertPoint SavedIP = CGF.Builder.saveAndClearIP();
CGF.EmitBlock(FinallyRethrow.getBlock(), true);
if (CGF.HaveInsertPoint()) {
- CGF.Builder.CreateCall(ObjCTypes.getExceptionThrowFn(),
- CGF.Builder.CreateLoad(RethrowPtr))
+ // Just look in the buffer for the exception to throw.
+ llvm::CallInst *Caught =
+ CGF.Builder.CreateCall(ObjCTypes.getExceptionExtractFn(),
+ ExceptionData);
+ Caught->setDoesNotThrow();
+
+ CGF.Builder.CreateCall(ObjCTypes.getExceptionThrowFn(), Caught)
->setDoesNotThrow();
CGF.Builder.CreateUnreachable();
}
Modified: cfe/branches/Apple/williamson/test/CodeGenObjC/exceptions.m
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/williamson/test/CodeGenObjC/exceptions.m?rev=111409&r1=111408&r2=111409&view=diff
==============================================================================
--- cfe/branches/Apple/williamson/test/CodeGenObjC/exceptions.m (original)
+++ cfe/branches/Apple/williamson/test/CodeGenObjC/exceptions.m Wed Aug 18 15:33:25 2010
@@ -25,12 +25,13 @@
// CHECK-NEXT: icmp
// CHECK-NEXT: br i1
@try {
- // CHECK: call void @foo()
+ // CHECK: call void asm sideeffect "", "*m"
+ // CHECK-NEXT: call void @foo()
foo();
- // CHECK: call void @objc_exception_try_exit
+ // CHECK-NEXT: call void @objc_exception_try_exit
// CHECK-NEXT: ret void
- // CHECK: call i8* @objc_exception_extract
+ // CHECK: call void asm sideeffect "", "=*m"
// CHECK-NEXT: ret void
} @finally {
break;
@@ -46,40 +47,38 @@
extern void foo(void);
// CHECK: [[X:%.*]] = alloca i32
- // CHECK: store i32 0, i32* [[X]]
+ // CHECK: store i32 5, i32* [[X]]
int x = 0;
+ x += 5;
// CHECK: [[SETJMP:%.*]] = call i32 @_setjmp
// CHECK-NEXT: [[CAUGHT:%.*]] = icmp eq i32 [[SETJMP]], 0
// CHECK-NEXT: br i1 [[CAUGHT]]
@try {
- // This load should be coalescable with the store of 0, so if the
- // optimizers ever figure out that out, that's okay.
+ // If the optimizers ever figure out how to make this store 6,
+ // that's okay.
// CHECK: [[T1:%.*]] = load i32* [[X]]
// CHECK-NEXT: [[T2:%.*]] = add nsw i32 [[T1]], 1
// CHECK-NEXT: store i32 [[T2]], i32* [[X]]
x++;
- // CHECK-NEXT: call void asm sideeffect "", "*m"(i32* [[X]]) nounwind
+ // CHECK-NEXT: call void asm sideeffect "", "*m,*m"(i32* [[X]]
// CHECK-NEXT: call void @foo()
+ // CHECK-NEXT: call void @objc_exception_try_exit
+ // CHECK-NEXT: [[T:%.*]] = load i32* [[X]]
+ // CHECK-NEXT: ret i32 [[T]]
foo();
} @catch (id) {
- // Landing pad. It turns out that the re-enter is unnecessary here.
- // CHECK: call void asm sideeffect "", "=*m"(i32* [[X]]) nounwind
+ // Landing pad. Note that we elide the re-enter.
+ // CHECK: call void asm sideeffect "", "=*m,=*m"(i32* [[X]]
// CHECK-NEXT: call i8* @objc_exception_extract
- // CHECK-NEXT: call void @objc_exception_try_enter
- // CHECK-NEXT: call i32 @_setjmp
- // CHECK-NEXT: icmp eq i32
- // CHECK-NEXT: br i1
-
- // Catch handler.
- // CHECK: [[T1:%.*]] = load i32* [[X]]
+ // CHECK-NEXT: [[T1:%.*]] = load i32* [[X]]
// CHECK-NEXT: [[T2:%.*]] = add nsw i32 [[T1]], -1
+
+ // This store is dead.
// CHECK-NEXT: store i32 [[T2]], i32* [[X]]
- // CHECK-NEXT: br label
+
+ // CHECK-NEXT: ret i32 [[T2]]
x--;
}
- // CHECK: call void @objc_exception_try_exit
- // CHECK-NEXT: [[T:%.*]] = load i32* [[X]]
- // CHECK: ret i32 [[T]]
return x;
}
Modified: cfe/branches/Apple/williamson/test/CodeGenObjC/synchronized.m
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/Apple/williamson/test/CodeGenObjC/synchronized.m?rev=111409&r1=111408&r2=111409&view=diff
==============================================================================
--- cfe/branches/Apple/williamson/test/CodeGenObjC/synchronized.m (original)
+++ cfe/branches/Apple/williamson/test/CodeGenObjC/synchronized.m Wed Aug 18 15:33:25 2010
@@ -23,23 +23,28 @@
// CHECK: define void @foo(
void foo(id a) {
// CHECK: [[A:%.*]] = alloca i8*
+ // CHECK: [[SYNC:%.*]] = alloca i8*
- // CHECK: call void @objc_sync_enter
- // CHECK: call void @objc_exception_try_enter
- // CHECK: call i32 @_setjmp
+ // CHECK: store i8* [[AVAL:%.*]], i8** [[A]]
+ // CHECK-NEXT: call void @objc_sync_enter(i8* [[AVAL]])
+ // CHECK-NEXT: store i8* [[AVAL]], i8** [[SYNC]]
+ // CHECK-NEXT: call void @objc_exception_try_enter
+ // CHECK: call i32 @_setjmp
@synchronized(a) {
- // CHECK: call void @objc_exception_try_exit
+ // This is unreachable, but the optimizers can't know that.
+ // CHECK: call void asm sideeffect "", "=*m,=*m,=*m"(i8** [[A]], i8** [[SYNC]]
// CHECK: call void @objc_sync_exit
+ // CHECK: call i8* @objc_exception_extract
+ // CHECK: call void @objc_exception_throw
+ // CHECK: unreachable
+
+ // CHECK: call void @objc_exception_try_exit
+ // CHECK: [[T:%.*]] = load i8** [[SYNC]]
+ // CHECK-NEXT: call void @objc_sync_exit
// CHECK: ret void
return;
}
- // This is unreachable, but the optimizers can't know that.
- // CHECK: call void asm sideeffect "", "=*m"(i8** [[A]])
- // CHECK: call i8* @objc_exception_extract
- // CHECK: call void @objc_sync_exit
- // CHECK: call void @objc_exception_throw
- // CHECK: unreachable
}
// CHECK: define i32 @f0(
More information about the llvm-branch-commits
mailing list