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

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
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],
----------------
jdoerfert wrote:
> efriedma wrote:
> > 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?
> I am waiting for ObjC people to look at this. I added the annotation based on two tests we otherwise fail, not based on the intrinsics (which I don't know).
> Though, I might have misunderstood: `llvm/test/Analysis/GlobalsModRef/intrinsic_addressnottaken1.ll`
I think those tests do not care specifically about `int_objc_autoreleasePoolPush/Pop` and should be changed to use something less special, e.g. to `@llvm.stacksave/@llvm.stackrestore` as I've done in D115302.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123531