r174922 - In ARC, emit non-peepholed +1s within the full-expression instead

[John McCall]

Author: rjmccall
Date: Mon Feb 11 18:25:08 2013
New Revision: 174922

URL: http://llvm.org/viewvc/llvm-project?rev=174922&view=rev
Log:
In ARC, emit non-peepholed +1s within the full-expression instead
of immediately afterwards.

Modified:
    cfe/trunk/lib/CodeGen/CGObjC.cpp
    cfe/trunk/test/CodeGenObjC/arc-ternary-op.m

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=174922&r1=174921&r2=174922&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Mon Feb 11 18:25:08 2013
@@ -2497,12 +2497,10 @@ static TryEmitResult tryEmitARCRetainPse
 
 static TryEmitResult
 tryEmitARCRetainScalarExpr(CodeGenFunction &CGF, const Expr *e) {
-  // Look through cleanups.
-  if (const ExprWithCleanups *cleanups = dyn_cast<ExprWithCleanups>(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<ExprWithCleanups>(e));
 
   // The desired result type, if it differs from the type of the
   // ultimate opaque expression.
@@ -2654,6 +2652,13 @@ static llvm::Value *emitARCRetainLoadOfS
 /// 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<ExprWithCleanups>(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::EmitARCRet
 
 llvm::Value *
 CodeGenFunction::EmitARCRetainAutoreleaseScalarExpr(const Expr *e) {
+  // The retain needs to happen within the full-expression.
+  if (const ExprWithCleanups *cleanups = dyn_cast<ExprWithCleanups>(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::EmitObjCTh
   // 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<ExprWithCleanups>(expr)) {
-      enterFullExpression(ewc);
-      expr = ewc->getSubExpr();
-    }
-
-    CodeGenFunction::RunCleanupsScope cleanups(*this);
+    // EmitARCRetainAutoreleaseScalarExpr does this for us.
     return EmitARCRetainAutoreleaseScalarExpr(expr);
   }
 

Modified: cfe/trunk/test/CodeGenObjC/arc-ternary-op.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc-ternary-op.m?rev=174922&r1=174921&r2=174922&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/arc-ternary-op.m (original)
+++ cfe/trunk/test/CodeGenObjC/arc-ternary-op.m Mon Feb 11 18:25:08 2013
@@ -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]])
+}