[ObjC][ARC] Don't add operand bundle `clang.arc.attachedcall` to a call if the call already has the operand bundle

[ahatanak]

Summary

This bug was causing the call to replaceAllUsesWith to crash because the old call instruction and the new call instruction were the same.

rdar://74957948

[ahatanak]

We could emit a call to @llvm.objc.retain instead of the call to @llvm.objc.retainAutoreleasedReturnValue, but this doesn't seem to happen often and ARC optimizer can remove or replace the call anyway, so I'm just letting the front-end emit @llvm.objc.retainAutoreleasedReturnValue for simplicity.

[ahatanak]

On second thought, I think we should emit a call to @llvm.objc.retain instead of a call to @llvm.objc.retainAutoreleasedReturnValue. I think we might want to remove ARC optimizer's code that converts calls to @llvm.objc.retainAutoreleasedReturnValue to calls to @llvm.objc.retain since it sometimes does the transformation when it shouldn't and we won't need that transformation once we switch to using the clang.arc.attachedcall operand bundle.

[ahatanak]

Emit a call to @llvm.objc.retain instead of a call to @llvm.objc.retainAutoreleasedReturnValue.

[ahatanak]

Check the presence of the operand bundle earlier in emitARCOperationAfterCall.

[rjmccall]

LGTM

clang/lib/CodeGen/CGObjC.cpp

static llvm::Value *emitOptimizedARCReturnCall(llvm::Value *value,
   // Add operand bundle "clang.arc.attachedcall" to the call instead of emitting
   // retainRV or claimRV calls in the IR. We currently do this only when the
   // optimization level isn't -O0 since global-isel, which is currently run at
   // -O0, doesn't know about the operand bundle.
 
   // FIXME: Do this when the target isn't aarch64.
   if (CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 &&
       CGF.CGM.getTarget().getTriple().isAArch64()) {
+    auto *oldCall = cast<llvm::CallBase>(value);
+
+    // The operand bundle cannot be added if the call already has the operand
+    // bundle.
+    if (llvm::objcarc::hasAttachedCallOpBundle(oldCall))
+      // If the runtime function being called is retainRV, emit a call to
+      // @objc_retain. Do nothing if it is claimRV.
+      return IsRetainRV ? CGF.EmitARCRetainNonBlock(value) : value;
+
     llvm::Value *bundleArgs[] = {llvm::ConstantInt::get(
         CGF.Int64Ty,
         llvm::objcarc::getAttachedCallOperandBundleEnum(IsRetainRV))};
     llvm::OperandBundleDef OB("clang.arc.attachedcall", bundleArgs);
-    auto *oldCall = cast<llvm::CallBase>(value);
     llvm::CallBase *newCall = llvm::CallBase::addOperandBundle(
         oldCall, llvm::LLVMContext::OB_clang_arc_attachedcall, OB, oldCall);
     newCall->copyMetadata(*oldCall);
     oldCall->replaceAllUsesWith(newCall);
     oldCall->eraseFromParent();
     CGF.EmitARCNoopIntrinsicUse(newCall);
     return newCall;
   }

clang/test/CodeGenObjCXX/arc-rv-attr.mm

+// RUN: %clang_cc1 -triple arm64-apple-ios9 -fobjc-runtime=ios-9.0 -fobjc-arc -std=c++11 -O -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s -check-prefix=CHECK
+
+id foo(void);
+
+// CHECK-LABEL: define{{.*}} void @_Z14test_list_initv(
+// CHECK: %[[CALL1:.*]] = call i8* @_Z3foov() [ "clang.arc.attachedcall"(i64 0) ]
+// CHECK: call i8* @llvm.objc.retain(i8* %[[CALL1]])
+
+void test_list_init() {
+  auto t = id{foo()};
+}