r177813 - Under ARC, when we're passing the address of a strong variable

John McCall rjmccall at apple.com
Fri Mar 22 19:35:54 PDT 2013


Author: rjmccall
Date: Fri Mar 22 21:35:54 2013
New Revision: 177813

URL: http://llvm.org/viewvc/llvm-project?rev=177813&view=rev
Log:
Under ARC, when we're passing the address of a strong variable
to an out-parameter using the indirect-writeback conversion,
and we copied the current value of the variable to the temporary,
make sure that we register an intrinsic use of that value with
the optimizer so that the value won't get released until we have
a chance to retain it.

rdar://13195034

Modified:
    cfe/trunk/lib/CodeGen/CGCall.cpp
    cfe/trunk/lib/CodeGen/CGCall.h
    cfe/trunk/lib/CodeGen/CGObjC.cpp
    cfe/trunk/lib/CodeGen/CodeGenFunction.h
    cfe/trunk/lib/CodeGen/CodeGenModule.h
    cfe/trunk/test/CodeGenObjC/arc-blocks.m
    cfe/trunk/test/CodeGenObjC/arc-ternary-op.m
    cfe/trunk/test/CodeGenObjC/arc.m
    cfe/trunk/test/CodeGenObjCXX/arc.mm

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Fri Mar 22 21:35:54 2013
@@ -1771,7 +1771,8 @@ static bool isProvablyNonNull(llvm::Valu
 /// Emit the actual writing-back of a writeback.
 static void emitWriteback(CodeGenFunction &CGF,
                           const CallArgList::Writeback &writeback) {
-  llvm::Value *srcAddr = writeback.Address;
+  const LValue &srcLV = writeback.Source;
+  llvm::Value *srcAddr = srcLV.getAddress();
   assert(!isProvablyNull(srcAddr) &&
          "shouldn't have writeback for provably null argument");
 
@@ -1798,9 +1799,35 @@ static void emitWriteback(CodeGenFunctio
                             "icr.writeback-cast");
   
   // Perform the writeback.
-  QualType srcAddrType = writeback.AddressType;
-  CGF.EmitStoreThroughLValue(RValue::get(value),
-                             CGF.MakeAddrLValue(srcAddr, srcAddrType));
+
+  // If we have a "to use" value, it's something we need to emit a use
+  // of.  This has to be carefully threaded in: if it's done after the
+  // release it's potentially undefined behavior (and the optimizer
+  // will ignore it), and if it happens before the retain then the
+  // optimizer could move the release there.
+  if (writeback.ToUse) {
+    assert(srcLV.getObjCLifetime() == Qualifiers::OCL_Strong);
+
+    // Retain the new value.  No need to block-copy here:  the block's
+    // being passed up the stack.
+    value = CGF.EmitARCRetainNonBlock(value);
+
+    // Emit the intrinsic use here.
+    CGF.EmitARCIntrinsicUse(writeback.ToUse);
+
+    // Load the old value (primitively).
+    llvm::Value *oldValue = CGF.EmitLoadOfScalar(srcLV);
+
+    // Put the new value in place (primitively).
+    CGF.EmitStoreOfScalar(value, srcLV, /*init*/ false);
+
+    // Release the old value.
+    CGF.EmitARCRelease(oldValue, srcLV.isARCPreciseLifetime());
+
+  // Otherwise, we can just do a normal lvalue store.
+  } else {
+    CGF.EmitStoreThroughLValue(RValue::get(value), srcLV);
+  }
 
   // Jump to the continuation block.
   if (!provablyNonNull)
@@ -1814,11 +1841,33 @@ static void emitWritebacks(CodeGenFuncti
     emitWriteback(CGF, *i);
 }
 
+static const Expr *maybeGetUnaryAddrOfOperand(const Expr *E) {
+  if (const UnaryOperator *uop = dyn_cast<UnaryOperator>(E->IgnoreParens()))
+    if (uop->getOpcode() == UO_AddrOf)
+      return uop->getSubExpr();
+  return 0;
+}
+
 /// Emit an argument that's being passed call-by-writeback.  That is,
 /// we are passing the address of 
 static void emitWritebackArg(CodeGenFunction &CGF, CallArgList &args,
                              const ObjCIndirectCopyRestoreExpr *CRE) {
-  llvm::Value *srcAddr = CGF.EmitScalarExpr(CRE->getSubExpr());
+  LValue srcLV;
+
+  // Make an optimistic effort to emit the address as an l-value.
+  // This can fail if the the argument expression is more complicated.
+  if (const Expr *lvExpr = maybeGetUnaryAddrOfOperand(CRE->getSubExpr())) {
+    srcLV = CGF.EmitLValue(lvExpr);
+
+  // Otherwise, just emit it as a scalar.
+  } else {
+    llvm::Value *srcAddr = CGF.EmitScalarExpr(CRE->getSubExpr());
+
+    QualType srcAddrType =
+      CRE->getSubExpr()->getType()->castAs<PointerType>()->getPointeeType();
+    srcLV = CGF.MakeNaturalAlignAddrLValue(srcAddr, srcAddrType);
+  }
+  llvm::Value *srcAddr = srcLV.getAddress();
 
   // The dest and src types don't necessarily match in LLVM terms
   // because of the crazy ObjC compatibility rules.
@@ -1833,9 +1882,6 @@ static void emitWritebackArg(CodeGenFunc
     return;
   }
 
-  QualType srcAddrType =
-    CRE->getSubExpr()->getType()->castAs<PointerType>()->getPointeeType();
-
   // Create the temporary.
   llvm::Value *temp = CGF.CreateTempAlloca(destType->getElementType(),
                                            "icr.temp");
@@ -1855,6 +1901,7 @@ static void emitWritebackArg(CodeGenFunc
   }
   
   llvm::BasicBlock *contBB = 0;
+  llvm::BasicBlock *originBB = 0;
 
   // If the address is *not* known to be non-null, we need to switch.
   llvm::Value *finalArgument;
@@ -1872,6 +1919,7 @@ static void emitWritebackArg(CodeGenFunc
     // If we need to copy, then the load has to be conditional, which
     // means we need control flow.
     if (shouldCopy) {
+      originBB = CGF.Builder.GetInsertBlock();
       contBB = CGF.createBasicBlock("icr.cont");
       llvm::BasicBlock *copyBB = CGF.createBasicBlock("icr.copy");
       CGF.Builder.CreateCondBr(isNull, contBB, copyBB);
@@ -1880,9 +1928,10 @@ static void emitWritebackArg(CodeGenFunc
     }
   }
 
+  llvm::Value *valueToUse = 0;
+
   // Perform a copy if necessary.
   if (shouldCopy) {
-    LValue srcLV = CGF.MakeAddrLValue(srcAddr, srcAddrType);
     RValue srcRV = CGF.EmitLoadOfLValue(srcLV);
     assert(srcRV.isScalar());
 
@@ -1892,15 +1941,37 @@ static void emitWritebackArg(CodeGenFunc
 
     // Use an ordinary store, not a store-to-lvalue.
     CGF.Builder.CreateStore(src, temp);
+
+    // If optimization is enabled, and the value was held in a
+    // __strong variable, we need to tell the optimizer that this
+    // value has to stay alive until we're doing the store back.
+    // This is because the temporary is effectively unretained,
+    // and so otherwise we can violate the high-level semantics.
+    if (CGF.CGM.getCodeGenOpts().OptimizationLevel != 0 &&
+        srcLV.getObjCLifetime() == Qualifiers::OCL_Strong) {
+      valueToUse = src;
+    }
   }
   
   // Finish the control flow if we needed it.
   if (shouldCopy && !provablyNonNull) {
+    llvm::BasicBlock *copyBB = CGF.Builder.GetInsertBlock();
     CGF.EmitBlock(contBB);
+
+    // Make a phi for the value to intrinsically use.
+    if (valueToUse) {
+      llvm::PHINode *phiToUse = CGF.Builder.CreatePHI(valueToUse->getType(), 2,
+                                                      "icr.to-use");
+      phiToUse->addIncoming(valueToUse, copyBB);
+      phiToUse->addIncoming(llvm::UndefValue::get(valueToUse->getType()),
+                            originBB);
+      valueToUse = phiToUse;
+    }
+
     condEval.end(CGF);
   }
 
-  args.addWriteback(srcAddr, srcAddrType, temp);
+  args.addWriteback(srcLV, temp, valueToUse);
   args.add(RValue::get(finalArgument), CRE->getType());
 }
 

Modified: cfe/trunk/lib/CodeGen/CGCall.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.h?rev=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.h (original)
+++ cfe/trunk/lib/CodeGen/CGCall.h Fri Mar 22 21:35:54 2013
@@ -56,14 +56,15 @@ namespace CodeGen {
     public SmallVector<CallArg, 16> {
   public:
     struct Writeback {
-      /// The original argument.
-      llvm::Value *Address;
-
-      /// The pointee type of the original argument.
-      QualType AddressType;
+      /// The original argument.  Note that the argument l-value
+      /// is potentially null.
+      LValue Source;
 
       /// The temporary alloca.
       llvm::Value *Temporary;
+
+      /// A value to "use" after the writeback, or null.
+      llvm::Value *ToUse;
     };
 
     void add(RValue rvalue, QualType type, bool needscopy = false) {
@@ -76,12 +77,12 @@ namespace CodeGen {
                         other.Writebacks.begin(), other.Writebacks.end());
     }
 
-    void addWriteback(llvm::Value *address, QualType addressType,
-                      llvm::Value *temporary) {
+    void addWriteback(LValue srcLV, llvm::Value *temporary,
+                      llvm::Value *toUse) {
       Writeback writeback;
-      writeback.Address = address;
-      writeback.AddressType = addressType;
+      writeback.Source = srcLV;
       writeback.Temporary = temporary;
+      writeback.ToUse = toUse;
       Writebacks.push_back(writeback);
     }
 

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Fri Mar 22 21:35:54 2013
@@ -1707,6 +1707,21 @@ llvm::Value *CodeGenFunction::EmitObjCEx
   return EmitARCRetainAutorelease(type, value);
 }
 
+/// Given a number of pointers, inform the optimizer that they're
+/// being intrinsically used up until this point in the program.
+void CodeGenFunction::EmitARCIntrinsicUse(ArrayRef<llvm::Value*> values) {
+  llvm::Constant *&fn = CGM.getARCEntrypoints().clang_arc_use;
+  if (!fn) {
+    llvm::FunctionType *fnType =
+      llvm::FunctionType::get(CGM.VoidTy, ArrayRef<llvm::Type*>(), true);
+    fn = CGM.CreateRuntimeFunction(fnType, "clang.arc.use");
+  }
+
+  // This isn't really a "runtime" function, but as an intrinsic it
+  // doesn't really matter as long as we align things up.
+  EmitNounwindRuntimeCall(fn, values);
+}
+
 
 static llvm::Constant *createARCRuntimeFunction(CodeGenModule &CGM,
                                                 llvm::FunctionType *type,

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Fri Mar 22 21:35:54 2013
@@ -2478,6 +2478,8 @@ public:
   llvm::Value *EmitARCRetainScalarExpr(const Expr *expr);
   llvm::Value *EmitARCRetainAutoreleaseScalarExpr(const Expr *expr);
 
+  void EmitARCIntrinsicUse(llvm::ArrayRef<llvm::Value*> values);
+
   static Destroyer destroyARCStrongImprecise;
   static Destroyer destroyARCStrongPrecise;
   static Destroyer destroyARCWeak;

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h Fri Mar 22 21:35:54 2013
@@ -217,6 +217,9 @@ struct ARCEntrypoints {
   /// A void(void) inline asm to use to mark that the return value of
   /// a call will be immediately retain.
   llvm::InlineAsm *retainAutoreleasedReturnValueMarker;
+
+  /// void clang.arc.use(...);
+  llvm::Constant *clang_arc_use;
 };
 
 /// CodeGenModule - This class organizes the cross-function state that is used

Modified: cfe/trunk/test/CodeGenObjC/arc-blocks.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc-blocks.m?rev=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/arc-blocks.m (original)
+++ cfe/trunk/test/CodeGenObjC/arc-blocks.m Fri Mar 22 21:35:54 2013
@@ -80,13 +80,14 @@ void test3(void (^sink)(id*)) {
   // CHECK-NEXT: bitcast
   // CHECK-NEXT: getelementptr
   // CHECK-NEXT: [[BLOCK:%.*]] = bitcast
-  // CHECK-NEXT: [[T0:%.*]] = load i8** [[STRONG]]
-  // CHECK-NEXT: store i8* [[T0]], i8** [[TEMP]]
+  // CHECK-NEXT: [[V:%.*]] = load i8** [[STRONG]]
+  // CHECK-NEXT: store i8* [[V]], i8** [[TEMP]]
   // CHECK-NEXT: [[F0:%.*]] = load i8**
   // CHECK-NEXT: [[F1:%.*]] = bitcast i8* [[F0]] to void (i8*, i8**)*
   // CHECK-NEXT: call void [[F1]](i8* [[BLOCK]], i8** [[TEMP]])
   // CHECK-NEXT: [[T0:%.*]] = load i8** [[TEMP]]
   // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retain(i8* [[T0]])
+  // CHECK-NEXT: call void (...)* @clang.arc.use(i8* [[V]]) [[NUW]]
   // CHECK-NEXT: [[T2:%.*]] = load i8** [[STRONG]]
   // CHECK-NEXT: store i8* [[T1]], i8** [[STRONG]]
   // CHECK-NEXT: call void @objc_release(i8* [[T2]])

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=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/arc-ternary-op.m (original)
+++ cfe/trunk/test/CodeGenObjC/arc-ternary-op.m Fri Mar 22 21:35:54 2013
@@ -61,11 +61,13 @@ void test1(int cond) {
   // CHECK:      [[T0:%.*]] = load i8** [[ARG]]
   // CHECK-NEXT: store i8* [[T0]], i8** [[TEMP1]]
   // CHECK-NEXT: br label
-  // CHECK:      call void @test1_sink(i8** [[T1]])
+  // CHECK:      [[W:%.*]] = phi i8* [ [[T0]], {{%.*}} ], [ undef, {{%.*}} ]
+  // CHECK-NEXT: call void @test1_sink(i8** [[T1]])
   // CHECK-NEXT: [[T0:%.*]] = icmp eq i8** [[ARG]], null
   // CHECK-NEXT: br i1 [[T0]],
   // CHECK:      [[T0:%.*]] = load i8** [[TEMP1]]
   // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retain(i8* [[T0]])
+  // CHECK-NEXT: call void (...)* @clang.arc.use(i8* [[W]]) [[NUW]]
   // CHECK-NEXT: [[T2:%.*]] = load i8** [[ARG]]
   // CHECK-NEXT: store i8* [[T1]], i8** [[ARG]]
   // CHECK-NEXT: call void @objc_release(i8* [[T2]])

Modified: cfe/trunk/test/CodeGenObjC/arc.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc.m?rev=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/arc.m (original)
+++ cfe/trunk/test/CodeGenObjC/arc.m Fri Mar 22 21:35:54 2013
@@ -865,8 +865,8 @@ void test33(Test33 *ptr) {
   // CHECK-NEXT: store [[A_T]]* null, [[A_T]]** [[A]]
 
   // CHECK-NEXT: load [[TEST33]]** [[PTR]]
-  // CHECK-NEXT: [[T0:%.*]] = load [[A_T]]** [[A]]
-  // CHECK-NEXT: store [[A_T]]* [[T0]], [[A_T]]** [[TEMP0]]
+  // CHECK-NEXT: [[W0:%.*]] = load [[A_T]]** [[A]]
+  // CHECK-NEXT: store [[A_T]]* [[W0]], [[A_T]]** [[TEMP0]]
   // CHECK-NEXT: load i8** @"\01L_OBJC_SELECTOR_REFERENCES_
   // CHECK-NEXT: bitcast
   // CHECK-NEXT: objc_msgSend{{.*}}, [[A_T]]** [[TEMP0]])
@@ -874,14 +874,15 @@ void test33(Test33 *ptr) {
   // CHECK-NEXT: [[T1:%.*]] = bitcast [[A_T]]* [[T0]] to i8*
   // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]])
   // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[A_T]]*
+  // CHECK-NEXT: call void (...)* @clang.arc.use([[A_T]]* [[W0]]) [[NUW]]
   // CHECK-NEXT: [[T4:%.*]] = load [[A_T]]** [[A]]
   // CHECK-NEXT: store [[A_T]]* [[T3]], [[A_T]]** [[A]]
   // CHECK-NEXT: [[T5:%.*]] = bitcast [[A_T]]* [[T4]] to i8*
   // CHECK-NEXT: call void @objc_release(i8* [[T5]])
 
   // CHECK-NEXT: load [[TEST33]]** [[PTR]]
-  // CHECK-NEXT: [[T0:%.*]] = load [[A_T]]** [[A]]
-  // CHECK-NEXT: store [[A_T]]* [[T0]], [[A_T]]** [[TEMP1]]
+  // CHECK-NEXT: [[W0:%.*]] = load [[A_T]]** [[A]]
+  // CHECK-NEXT: store [[A_T]]* [[W0]], [[A_T]]** [[TEMP1]]
   // CHECK-NEXT: load i8** @"\01L_OBJC_SELECTOR_REFERENCES_
   // CHECK-NEXT: bitcast
   // CHECK-NEXT: objc_msgSend{{.*}}, [[A_T]]** [[TEMP1]])
@@ -889,6 +890,7 @@ void test33(Test33 *ptr) {
   // CHECK-NEXT: [[T1:%.*]] = bitcast [[A_T]]* [[T0]] to i8*
   // CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]])
   // CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] to [[A_T]]*
+  // CHECK-NEXT: call void (...)* @clang.arc.use([[A_T]]* [[W0]]) [[NUW]]
   // CHECK-NEXT: [[T4:%.*]] = load [[A_T]]** [[A]]
   // CHECK-NEXT: store [[A_T]]* [[T3]], [[A_T]]** [[A]]
   // CHECK-NEXT: [[T5:%.*]] = bitcast [[A_T]]* [[T4]] to i8*
@@ -962,15 +964,16 @@ void test37(void) {
   // CHECK-NEXT: [[TEMP:%.*]] = alloca i8*
   // CHECK-NEXT: store [[TEST37]]* null, [[TEST37]]** [[VAR]]
 
-  // CHECK-NEXT: [[T0:%.*]] = load [[TEST37]]** [[VAR]]
-  // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST37]]* [[T0]] to i8*
-  // CHECK-NEXT: store i8* [[T1]], i8** [[TEMP]]
+  // CHECK-NEXT: [[W0:%.*]] = load [[TEST37]]** [[VAR]]
+  // CHECK-NEXT: [[W1:%.*]] = bitcast [[TEST37]]* [[W0]] to i8*
+  // CHECK-NEXT: store i8* [[W1]], i8** [[TEMP]]
   // CHECK-NEXT: call void @test37_helper(i8** [[TEMP]])
   // CHECK-NEXT: [[T0:%.*]] = load i8** [[TEMP]]
   // CHECK-NEXT: [[T1:%.*]] = bitcast i8* [[T0]] to [[TEST37]]*
   // CHECK-NEXT: [[T2:%.*]] = bitcast [[TEST37]]* [[T1]] to i8*
   // CHECK-NEXT: [[T3:%.*]] = call i8* @objc_retain(i8* [[T2]])
   // CHECK-NEXT: [[T4:%.*]] = bitcast i8* [[T3]] to [[TEST37]]*
+  // CHECK-NEXT: call void (...)* @clang.arc.use(i8* [[W1]]) [[NUW]]
   // CHECK-NEXT: [[T5:%.*]] = load [[TEST37]]** [[VAR]]
   // CHECK-NEXT: store [[TEST37]]* [[T4]], [[TEST37]]** [[VAR]]
   // CHECK-NEXT: [[T6:%.*]] = bitcast [[TEST37]]* [[T5]] to i8*

Modified: cfe/trunk/test/CodeGenObjCXX/arc.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc.mm?rev=177813&r1=177812&r2=177813&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/arc.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/arc.mm Fri Mar 22 21:35:54 2013
@@ -76,11 +76,13 @@ void test34(int cond) {
   // CHECK:      [[T0:%.*]] = load i8** [[ARG]]
   // CHECK-NEXT: store i8* [[T0]], i8** [[TEMP1]]
   // CHECK-NEXT: br label
+  // CHECK:      [[W0:%.*]] = phi i8* [ [[T0]], {{%.*}} ], [ undef, {{%.*}} ]
   // CHECK:      call void @_Z11test34_sinkPU15__autoreleasingP11objc_object(i8** [[T1]])
   // CHECK-NEXT: [[T0:%.*]] = icmp eq i8** [[ARG]], null
   // CHECK-NEXT: br i1 [[T0]],
   // CHECK:      [[T0:%.*]] = load i8** [[TEMP1]]
   // CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retain(i8* [[T0]])
+  // CHECK-NEXT: call void (...)* @clang.arc.use(i8* [[W0]])
   // CHECK-NEXT: [[T2:%.*]] = load i8** [[ARG]]
   // CHECK-NEXT: store i8* [[T1]], i8** [[ARG]]
   // CHECK-NEXT: call void @objc_release(i8* [[T2]])





More information about the cfe-commits mailing list