theraven accepted this revision. theraven added a comment. This revision is now accepted and ready to land.
LGTM, a couple of extra comments would help. ================ Comment at: llvm/include/llvm/IR/IntrinsicInst.h:110 + // Check if this intrinsic might lower into a regular function call + static bool mayLowerToFunctionCall(Intrinsic::ID IID); ---------------- Minor nit, should be /// for a doc comment. ================ Comment at: llvm/lib/IR/IntrinsicInst.cpp:61 + case Intrinsic::objc_sync_enter: + case Intrinsic::objc_sync_exit: + return true; ---------------- I'm curious why memcpy and friends don't need to be on this list. Do they get expanded at a different point? Or is it that the front end inserts a memcpy call and the optimiser that turns them into intrinsics and back preserves operand bundles? It would be a good idea to have a comment explaining why these specific ones are on the list and not other libc (or math library) functions that may appear in cleanup blocks. From the code in `InlineFunction.cpp`, it looks as if this is a problem only for intrinsics that may throw, which excludes most of the C standard ones? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128190/new/ https://reviews.llvm.org/D128190 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
