Re: [PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

Following up on this, AFAICT this is working as intended on the LLVM side. The different decision is made in GlobalsAA + MemoryDependencyAnalysis. IIUC, the logic there is along the lines of: if I have a global G that's "internal" ("static" in C), and an intrinsic method M, then M cannot read from

Re: [PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

Hi, I think the assumption that intrinsics can't read any global is, indeed, the problem here. `@llvm.objc.autoreleasePoolPop` may call any number of `-dealloc` methods with arbitrary side effects (though if they cause resurrection then that's undefined behaviour), so there should be no assu

Re: [PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

I only got a chance to look more into this now. I can reproduce it with re-inserting the "static". The miscompile is not related to MemorySSA, i.e. disabling all uses of MemorySSA doesn't help. It appears to be a GVN bug, but I haven't tracked it further than this. To repro, see attached .ll file

Re: [PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

Hi, Yes, I believe it does still reproduce (at least, with the most recent build that I tried). We worked around the clang bug to make the test pass: https://github.com/gnustep/libobjc2/commit/eab35fce379eb6ab1423dbb6d7908cb782f13458#diff-7f1eea7fdb5c7c3bf21f08d1cfe98a19 Reintroducing the 's

Re: [PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

Hi David, Does this still reproduce? I'm seeing the load after the call to autoreleasePoolPop at ToT, but perhaps I'm not using the proper flags? I only checked out the github repo and did "clang -fobjc-runtime=gnustep-2.0 -O3 -emit-llvm -S libobjc2/Test/AssociatedObject.m" with a ToT clang. Than

Re: [PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

Hey David Thanks for letting me know, and analysing it this far! I also can't see anything wrong with the intrinsic. Its just defined as: def int_objc_autoreleasePoolPop : Intrinsic<[], [llvm_ptr_ty]>; which (I believe) means it has unmodelled side effects so it should have been f

[PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

theraven added a comment. Herald added a project: LLVM. After some bisection, it appears that this is the revision that introduced the regression in the GNUstep Objective-C runtime test suite that I reported on the list a few weeks ago. In this is the test (compiled with `-fobjc-runtime=gnuste

[PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

pete added a comment. In D55802#1334008 , @rjmccall wrote: > You're making intrinsics with `weak_external` linkage? I feel like that's > going to be unnecessarily awkward in the future, but okay. Yeah... i was a little surprised that was possible, but

[PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. You're making intrinsics with `weak_external` linkage? I feel like that's going to be unnecessarily awkward in the future, but okay. Repository: rC Clang CHANGES SINCE LAST ACTION h