[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:2326
+
+  if (CGF.CGM.getCodeGenOpts().OptimizationLevel != 0 &&
+      CGF.CGM.getTarget().getTriple().isAArch64() &&
----------------
Nit: I think that `> 0` is easier to read.


================
Comment at: clang/lib/CodeGen/CGObjC.cpp:2328
+      CGF.CGM.getTarget().getTriple().isAArch64() &&
+      !CGF.CGM.getTarget().getTriple().isOSWindows()) {
+    auto *callBase = cast<llvm::CallBase>(value);
----------------
Hmm, why the explicit check for not-Windows?


================
Comment at: llvm/include/llvm/Analysis/ObjCARCRVAttr.h:43
+    CB->removeAttribute(llvm::AttributeList::ReturnIndex, "claimRV");
+  if (RemoveMarker && hasRVMarkerAttr(CB))
+    CB->removeAttribute(llvm::AttributeList::ReturnIndex, "rv_marker");
----------------
Nit: A new line before this would be nice.


================
Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp:468
+    if (IsBundled)
+      return false;
+
----------------
It seems that `IsBundled` is unused after this point, why not just do:

```
if (BundledInsts->contains(Inst))
  return false;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808