[PATCH] D92808: [ObjC][ARC] Use operand bundle 'clang.arc.rv' instead of explicitly emitting retainRV or claimRV calls in the IR
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 1 10:06:30 PST 2021
fhahn added a comment.
Thanks for the update! The approach with operand bundles and the no-op uses looks cleaner than the original version. The special handling in the inliner seems a bit unfortunate, but I guess there's no way around that?
(as a side note, it might be easier to submit if this would be split up in the changes adding the bundle, followed by the ARC optimization & clang changes)
================
Comment at: llvm/include/llvm/Analysis/ObjCARCUtil.h:41
+static inline bool hasRVOpBundle(const CallBase *CB) {
+ return CB->getOperandBundle(LLVMContext::OB_clang_arc_rv).hasValue();
+}
----------------
I think we should include `LLVMContext.h` explicitly?
================
Comment at: llvm/include/llvm/IR/InstrTypes.h:1217
+ static CallBase *addOperandBundle(CallBase *CB, uint32_t ID,
+ OperandBundleDef OB,
----------------
Please add documentation for the new functions.
================
Comment at: llvm/include/llvm/IR/Intrinsics.td:449
[llvm_vararg_ty]>;
+def int_objc_clang_arc_noop_use : Intrinsic<[],
+ [llvm_vararg_ty],
----------------
Can this be a `DefaultAttrIntrinsics` (which adds ` nofree nosync nounwind willreturn`)?
Same probably for all/most objc_* intrinsics.
================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1650
+static void
+insertRetainOrClaimRVCalls(CallBase &CB,
+ const SmallVectorImpl<ReturnInst *> &Returns) {
----------------
Could you add a comment elaborating what this function does and why it is needed?
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