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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
