[PATCH] D111334: [ObjC][ARC] Handle operand bundle "clang.arc.attachedcall" on targets that don't use the inline asm marker

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Nov 7 01:26:32 PDT 2021


fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks! I left 2 small suggestions that can directly be addressed before committing.



================
Comment at: llvm/lib/Transforms/ObjCARC/ObjCARC.h:159
   bool ContractPass;
+  bool UseMarker;
 };
----------------
It might be worth adding a quick comment here, to explain what `UseMarker` indicates here.


================
Comment at: llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp:437
   case ARCInstKind::ClaimRV: {
-    // If we're compiling for a target which needs a special inline-asm
-    // marker to do the return value optimization and the retainRV/claimRV call
-    // wasn't bundled with a call, insert the marker now.
+    bool BundledInst = BundledInsts->contains(Inst);
+
----------------
nit: When I first read the code without looking at the precise definition I thought this is a bundle instruction, rather than whether the instruction is contained in a bundle. Perhaps the name could be improved slightly, e.g. `IsInstContainedInBundle`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111334



More information about the llvm-commits mailing list