[PATCH] D123531: [GlobalsModRef][FIX] Ensure we honor synchronizing effects of intrinsics

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a subscriber: ahatanak.
efriedma added a comment.

> DefaultIntrinsic is available for a long time by now, people will not switch if they don't have to, nor will they revisit their intrinsics. It's still plenty of time till a release. Let's not add a hatch too early, give people a chance to realize and adjust.

I guess this is fine.  But please post a note on Discourse.



================
Comment at: llvm/include/llvm/IR/Intrinsics.td:428
+def int_objc_autoreleasePoolPop             : Intrinsic<[], [llvm_ptr_ty], [IntrNoSync, IntrNoCallback]>;
+def int_objc_autoreleasePoolPush            : Intrinsic<[llvm_ptr_ty], [], [IntrNoSync, IntrNoCallback]>;
 def int_objc_autoreleaseReturnValue         : Intrinsic<[llvm_ptr_ty],
----------------
In general, popping an autorelease pool deallocates objects, and deallocating objects can run arbitrary user code. So the new markings on int_objc_autoreleasePoolPop are wrong, I think.  Marking up int_objc_autoreleasePoolPush should be fine, though.

@rjmccall @ahatanak can you confirm?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123531