fhahn added a comment. Thanks for the updates! The LLVM side of things now looks good to me. Could you also add a description of the new bundle to LangRef https://llvm.org/docs/LangRef.html#operand-bundles?
In D92808#2535593 <https://reviews.llvm.org/D92808#2535593>, @ahatanak wrote: > We could run another pass to clean up the IR after inlining, but I think it's > better to do the cleanup right after the callee function is cloned in the > caller function. Thanks for elaborating. Given that it is isolated to a single place and the inliner already has logic to deal with a few different intrinsics/bundle types, I think that should be fine. ================ Comment at: llvm/include/llvm/IR/Intrinsics.td:449 [llvm_vararg_ty]>; +def int_objc_clang_arc_noop_use : Intrinsic<[], + [llvm_vararg_ty], ---------------- ahatanak wrote: > fhahn wrote: > > Can this be a `DefaultAttrIntrinsics` (which adds ` nofree nosync nounwind > > willreturn`)? > > > > Same probably for all/most objc_* intrinsics. > I'll change the other intrinsics in a separate patch. SGTM. thanks! ================ Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1655 +/// +/// 1. If there is a call to autoreleasaeRV that takes a pointer to the returned +/// object in the callee return block, the autoreleaseRV call and the ---------------- nit: typo `autoreleasaeRV` ================ Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1669 + const SmallVectorImpl<ReturnInst *> &Returns) { + Module *Mod = CB.getParent()->getParent()->getParent(); + bool IsRetainRV = objcarc::hasRVOpBundle(&CB, true), IsClaimRV = !IsRetainRV; ---------------- nit: Can you just use `CB.getModule()`? ================ Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1673 + for (auto *RI : Returns) { + Value *RetOpnd = llvm::objcarc::GetRCIdentityRoot(RI->getOperand(0)); + BasicBlock::reverse_iterator I = ++(RI->getIterator().getReverse()); ---------------- we are in the `llvm::` namespace here I think, so this should not be needed ? above you use the `objcarc` namespace without `llvm::` prefix as well. Same in other places in the function. 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
