[PATCH] D103000: [ObjC][ARC] Use the addresses of the ARC runtime functions instead of integer 0/1 for the operand of bundle "clang.arc.attachedcall"

[Akira Hatanaka via Phabricator via llvm-commits]

ahatanak added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:4454
     unsigned RuntimeCallType =
-        objcarc::hasAttachedCallOpBundle(CLI.CB, true) ? 0 : 1;
+        ARCFn == M->getFunction("objc_retainAutoreleasedReturnValue") ? 0 : 1;
     Ops.insert(Ops.begin() + 1,
----------------
fhahn wrote:
> Would to be possible to add an operand with the symbol of the function instead a number here? 
I replaced the number with a target global address.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1675
   Module *Mod = CB.getModule();
-  bool IsRetainRV = objcarc::hasAttachedCallOpBundle(&CB, true),
+  assert(((RVCallKind == objcarc::ARCInstKind::RetainRV) ||
+          (RVCallKind == objcarc::ARCInstKind::ClaimRV)) &&
----------------
fhahn wrote:
> Would it make sense to have a function in the ‘obj arc’’ namespace to check IsRetainRV instead of duplicating the check + assertion in multiple paces?
I'm not sure how you can remove code duplication using `isRetainRV`, which just checks whether the function is `objc_retainAutoreleasedReturnValue`. Did you mean `isRetainOrClaimRV`, which checks whether the function is either `objc_retainAutoreleasedReturnValue` or `objc_unsafeClaimAutoreleasedReturnValue`? Also, we can't use anything from `objcarc` in Verifier.cpp as that would be a layering violation, so we'd need a separate helper function there.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103000/new/

https://reviews.llvm.org/D103000