r287291 - Forward ns_consumed delegate arguments with a move.

[John McCall via cfe-commits]

Author: rjmccall
Date: Thu Nov 17 19:08:24 2016
New Revision: 287291

URL: http://llvm.org/viewvc/llvm-project?rev=287291&view=rev
Log:
Forward ns_consumed delegate arguments with a move.

StartFunction enters a release cleanup for ns_consumed arguments in
ARC, so we need to balance that somehow.  We could teach StartFunction
that it's emitting a delegating function, so that the cleanup is
unnecessary, but that would be invasive and somewhat fraught.  We could
balance the consumed argument with an extra retain, but clearing the
original variable should be easier to optimize and avoid some extra work
at -O0.  And there shouldn't be any difference as long as nothing else
uses the argument, which should always be true for the places we emit
delegate arguments.

Fixes PR 27887.

Modified:
    cfe/trunk/lib/CodeGen/CGCall.cpp
    cfe/trunk/test/CodeGenObjCXX/arc-attrs.mm

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=287291&r1=287290&r2=287291&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Nov 17 19:08:24 2016
@@ -2912,12 +2912,30 @@ void CodeGenFunction::EmitDelegateCallAr
   assert(!isInAllocaArgument(CGM.getCXXABI(), type) &&
          "cannot emit delegate call arguments for inalloca arguments!");
 
+  // GetAddrOfLocalVar returns a pointer-to-pointer for references,
+  // but the argument needs to be the original pointer.
+  if (type->isReferenceType()) {
+    args.add(RValue::get(Builder.CreateLoad(local)), type);
+
+  // In ARC, move out of consumed arguments so that the release cleanup
+  // entered by StartFunction doesn't cause an over-release.  This isn't
+  // optimal -O0 code generation, but it should get cleaned up when
+  // optimization is enabled.  This also assumes that delegate calls are
+  // performed exactly once for a set of arguments, but that should be safe.
+  } else if (getLangOpts().ObjCAutoRefCount &&
+             param->hasAttr<NSConsumedAttr>() &&
+             type->isObjCRetainableType()) {
+    llvm::Value *ptr = Builder.CreateLoad(local);
+    auto null =
+      llvm::ConstantPointerNull::get(cast<llvm::PointerType>(ptr->getType()));
+    Builder.CreateStore(null, local);
+    args.add(RValue::get(ptr), type);
+
   // For the most part, we just need to load the alloca, except that
   // aggregate r-values are actually pointers to temporaries.
-  if (type->isReferenceType())
-    args.add(RValue::get(Builder.CreateLoad(local)), type);
-  else
+  } else {
     args.add(convertTempToRValue(local, type, loc), type);
+  }
 }
 
 static bool isProvablyNull(llvm::Value *addr) {

Modified: cfe/trunk/test/CodeGenObjCXX/arc-attrs.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-attrs.mm?rev=287291&r1=287290&r2=287291&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/arc-attrs.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/arc-attrs.mm Thu Nov 17 19:08:24 2016
@@ -46,3 +46,24 @@ void templateTest() {
   // CHECK-NEXT: call void @objc_storeStrong(i8** [[X]], i8* null)
   // CHECK-NEXT: ret void
 }
+
+// PR27887
+struct ForwardConsumed {
+  ForwardConsumed(__attribute__((ns_consumed)) id x);
+};
+
+ForwardConsumed::ForwardConsumed(__attribute__((ns_consumed)) id x) {}
+
+// CHECK: define void @_ZN15ForwardConsumedC2EP11objc_object(
+// CHECK-NOT:  objc_retain
+// CHECK:      store i8* {{.*}}, i8** [[X:%.*]],
+// CHECK-NOT:  [[X]]
+// CHECK:      call void @objc_storeStrong(i8** [[X]], i8* null)
+
+// CHECK: define void @_ZN15ForwardConsumedC1EP11objc_object(
+// CHECK-NOT:  objc_retain
+// CHECK:      store i8* {{.*}}, i8** [[X:%.*]],
+// CHECK:      [[T0:%.*]] = load i8*, i8** [[X]],
+// CHECK-NEXT: store i8* null, i8** [[X]],
+// CHECK-NEXT: call void @_ZN15ForwardConsumedC2EP11objc_object({{.*}}, i8* [[T0]])
+// CHECK:      call void @objc_storeStrong(i8** [[X]], i8* null)