[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 10 16:49:59 PST 2021


dexonsmith added a comment.

In D92808#2552354 <https://reviews.llvm.org/D92808#2552354>, @rjmccall wrote:

> The ultimate code-generation consequences of this feature aren't very generic, so we shouldn't let ourselves get caught up designing an extremely general feature that ultimately either doesn't do what we need it to do or doesn't actually work for any of the generalizations.

Sure, those are two bad potential outcomes.

Stepping back, the goalĀ of this patch is to encode ARC's code generation invariants in the IR so that maintainers don't inadvertently break them. I think the IR should describe those invariants as clearly and precisely as is reasonable.

Besides idle / misplaced optimism about reuse, I was trying to make the following points along those lines, so I'll restate them more directly:

- Explicitly adding a use of the call avoids having to teach transformations / analyses about it. It might be simpler to add a self-reference carve-out for this operand bundle (or operand bundles in general) than to teach all the passes that this operand bundle implies a use of the callsite (but I'm not sure; maybe there are lots of assumptions about self-references not existing outside of PHIs...).
- Passing the function that's implicitly called into the operand bundle is simpler to reason about than an opaque table index.
- Naming the operand bundle after its code generation consequences makes it simpler reason about. (A few generic names thatĀ make some sense to me (besides what I listed yesterday) include "callafter", "callimmediately", "forwardto", and "attachedcall". If it's important to make this ARC only, it could be (e.g.) "clang.arc.attachedcall".)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92808



More information about the llvm-commits mailing list