[CGObjC] Allow clang.arc.attachedcall on -O0

[AZero13]

It is supported in GlobalISel there. It is not supported on X86 GlobalISel.

[llvmbot]

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: AZero13 (AZero13)

Changes

It is supported in GlobalISel there. It is not supported on X86 GlobalISel.

---

Full diff: https://github.com/llvm/llvm-project/pull/164875.diff

1 Files Affected:

• (modified) clang/lib/CodeGen/CGObjC.cpp (+4-4)

diff --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index 10aad2e26938d..4920861327c4f 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -2417,7 +2417,7 @@ 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.
+  // -O0, doesn't know about the operand bundle on x86_64.
   ObjCEntrypoints &EPs = CGF.CGM.getObjCEntrypoints();
   llvm::Function *&EP = IsRetainRV
                             ? EPs.objc_retainAutoreleasedReturnValue
@@ -2431,9 +2431,9 @@ static llvm::Value *emitOptimizedARCReturnCall(llvm::Value *value,
 
   // FIXME: Do this on all targets and at -O0 too. This can be enabled only if
   // the target backend knows how to handle the operand bundle.
-  if (CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 &&
-      (Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_32 ||
-       Arch == llvm::Triple::x86_64)) {
+  if ((CGF.CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+       Arch == llvm::Triple::x86_64) ||
+      (Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_32)) {
     llvm::Value *bundleArgs[] = {EP};
     llvm::OperandBundleDef OB("clang.arc.attachedcall", bundleArgs);
     auto *oldCall = cast<llvm::CallBase>(value);

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[rjmccall]

Okay, what you're saying is that this is now supported in GlobalISel, so we can use it in -O0 on the same architectures that use it for -O? If so, great, let's remove the -O0 condition.

[AZero13]

Okay, what you're saying is that this is now supported in GlobalISel, so we can use it in -O0 on the same architectures that use it for -O? If so, great, let's remove the -O0 condition.

Already did!

[AZero13]

@rjmccall What do you think?

[rjmccall]

Okay, looking at the patch, I see what you're doing and what you're trying to say in the PR description: x86_64 GlobalISel still doesn't support this bundle, so you've just changed it to force the use of SelectionDAG when it sees such a bundle.

Now, I accept that the backend change is an improvement in the abstract — it's bad that one instruction selector just fails to compile IR that's valid for the other instruction selector, so if GlobalISel can't support these bundles, it should definitely kick over to SelectionDAG. However, the point of GlobalISel is to make -O0 builds faster, which means that Clang should not be emitting IR constructs for common ObjC code patterns that would force the use of SelectionDAG, because that means we'll necessarily regress -O0 build times. So either this needs to continue to not use the bundle on x86_64 at -O0, or you need to implement the bundle properly witout kicking over to SelectionDAG.