From: John McCall Date: Tue, 12 Feb 2013 00:25:08 +0000 (+0000) Subject: In ARC, emit non-peepholed +1s within the full-expression instead X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d21cdd49471783a6743e6d449434de401aa9a2b0;p=platform%2Fupstream%2Fllvm.git In ARC, emit non-peepholed +1s within the full-expression instead of immediately afterwards. llvm-svn: 174922 --- diff --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp index 4583367..b2f5aa3 100644 --- a/clang/lib/CodeGen/CGObjC.cpp +++ b/clang/lib/CodeGen/CGObjC.cpp @@ -2497,12 +2497,10 @@ static TryEmitResult tryEmitARCRetainPseudoObject(CodeGenFunction &CGF, static TryEmitResult tryEmitARCRetainScalarExpr(CodeGenFunction &CGF, const Expr *e) { - // Look through cleanups. - if (const ExprWithCleanups *cleanups = dyn_cast(e)) { - CGF.enterFullExpression(cleanups); - CodeGenFunction::RunCleanupsScope scope(CGF); - return tryEmitARCRetainScalarExpr(CGF, cleanups->getSubExpr()); - } + // We should *never* see a nested full-expression here, because if + // we fail to emit at +1, our caller must not retain after we close + // out the full-expression. + assert(!isa(e)); // The desired result type, if it differs from the type of the // ultimate opaque expression. @@ -2654,6 +2652,13 @@ static llvm::Value *emitARCRetainLoadOfScalar(CodeGenFunction &CGF, /// best-effort attempt to peephole expressions that naturally produce /// retained objects. llvm::Value *CodeGenFunction::EmitARCRetainScalarExpr(const Expr *e) { + // The retain needs to happen within the full-expression. + if (const ExprWithCleanups *cleanups = dyn_cast(e)) { + enterFullExpression(cleanups); + RunCleanupsScope scope(*this); + return EmitARCRetainScalarExpr(cleanups->getSubExpr()); + } + TryEmitResult result = tryEmitARCRetainScalarExpr(*this, e); llvm::Value *value = result.getPointer(); if (!result.getInt()) @@ -2663,6 +2668,13 @@ llvm::Value *CodeGenFunction::EmitARCRetainScalarExpr(const Expr *e) { llvm::Value * CodeGenFunction::EmitARCRetainAutoreleaseScalarExpr(const Expr *e) { + // The retain needs to happen within the full-expression. + if (const ExprWithCleanups *cleanups = dyn_cast(e)) { + enterFullExpression(cleanups); + RunCleanupsScope scope(*this); + return EmitARCRetainAutoreleaseScalarExpr(cleanups->getSubExpr()); + } + TryEmitResult result = tryEmitARCRetainScalarExpr(*this, e); llvm::Value *value = result.getPointer(); if (result.getInt()) @@ -2694,17 +2706,7 @@ llvm::Value *CodeGenFunction::EmitObjCThrowOperand(const Expr *expr) { // In ARC, retain and autorelease the expression. if (getLangOpts().ObjCAutoRefCount) { // Do so before running any cleanups for the full-expression. - // tryEmitARCRetainScalarExpr does make an effort to do things - // inside cleanups, but there are crazy cases like - // @throw A().foo; - // where a full retain+autorelease is required and would - // otherwise happen after the destructor for the temporary. - if (const ExprWithCleanups *ewc = dyn_cast(expr)) { - enterFullExpression(ewc); - expr = ewc->getSubExpr(); - } - - CodeGenFunction::RunCleanupsScope cleanups(*this); + // EmitARCRetainAutoreleaseScalarExpr does this for us. return EmitARCRetainAutoreleaseScalarExpr(expr); } diff --git a/clang/test/CodeGenObjC/arc-ternary-op.m b/clang/test/CodeGenObjC/arc-ternary-op.m index bd63a47..0566370 100644 --- a/clang/test/CodeGenObjC/arc-ternary-op.m +++ b/clang/test/CodeGenObjC/arc-ternary-op.m @@ -93,3 +93,42 @@ void test1(int cond) { // CHECK: call void @objc_destroyWeak(i8** [[WEAK]]) // CHECK: ret void } + +// rdar://13113981 +// Test that, when emitting an expression at +1 that we can't peephole, +// we emit the retain inside the full-expression. If we ever peephole +// +1s of conditional expressions (which we probably ought to), we'll +// need to find another example of something we need to do this for. +void test2(int cond) { + extern id test2_producer(void); + for (id obj in cond ? test2_producer() : (void*) 0) { + } + + // CHECK: define void @test2( + // CHECK: [[COND:%.*]] = alloca i32, + // CHECK: alloca i8* + // CHECK: [[CLEANUP_SAVE:%.*]] = alloca i8* + // CHECK: [[RUN_CLEANUP:%.*]] = alloca i1 + // Evaluate condition; cleanup disabled by default. + // CHECK: [[T0:%.*]] = load i32* [[COND]], + // CHECK-NEXT: icmp ne i32 [[T0]], 0 + // CHECK-NEXT: store i1 false, i1* [[RUN_CLEANUP]] + // CHECK-NEXT: br i1 + // Within true branch, cleanup enabled. + // CHECK: [[T0:%.*]] = call i8* @test2_producer() + // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retainAutoreleasedReturnValue(i8* [[T0]]) + // CHECK-NEXT: store i8* [[T1]], i8** [[CLEANUP_SAVE]] + // CHECK-NEXT: store i1 true, i1* [[RUN_CLEANUP]] + // CHECK-NEXT: br label + // Join point for conditional operator; retain immediately. + // CHECK: [[T0:%.*]] = phi i8* [ [[T1]], {{%.*}} ], [ null, {{%.*}} ] + // CHECK-NEXT: [[RESULT:%.*]] = call i8* @objc_retain(i8* [[T0]]) + // Leaving full-expression; run conditional cleanup. + // CHECK-NEXT: [[T0:%.*]] = load i1* [[RUN_CLEANUP]] + // CHECK-NEXT: br i1 [[T0]] + // CHECK: [[T0:%.*]] = load i8** [[CLEANUP_SAVE]] + // CHECK-NEXT: call void @objc_release(i8* [[T0]]) + // CHECK-NEXT: br label + // And way down at the end of the loop: + // CHECK: call void @objc_release(i8* [[RESULT]]) +}