[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-21 Thread Pete Cooper via Phabricator via cfe-commits
pete added a comment. In D55869#1339537 , @rjmccall wrote: > It sounds like it's fine. Thanks for the review! Just pushed it as r349952. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55869/new/

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-21 Thread Pete Cooper via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC349952: Convert some ObjC retain/release msgSends to runtime calls. (authored by pete, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55869/new/

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-21 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. It sounds like it's fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55869/new/ https://reviews.llvm.org/D55869 ___ cfe-commits

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-21 Thread Pete Cooper via Phabricator via cfe-commits
pete added a comment. Thanks for all the feedback so far. Is there anything else you'd like me to change before I can land this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55869/new/ https://reviews.llvm.org/D55869 ___ cfe-commits

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D55869#1338003 , @theraven wrote: > This should be fine for the GNUstep runtime (the GCC runtime doesn't support > ARC at all). My main concern is that it will break already-released versions > of the runtime built with a

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread David Chisnall via Phabricator via cfe-commits
theraven added a comment. This should be fine for the GNUstep runtime (the GCC runtime doesn't support ARC at all). My main concern is that it will break already-released versions of the runtime built with a newer version of clang. I can easily enable a new flag in the next release, but

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D55869#1337733 , @pete wrote: > In D55869#1337723 , @js wrote: > > > In D55869#1337711 , @dexonsmith > > wrote: > > > > > In D55869#1337706

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread Pete Cooper via Phabricator via cfe-commits
pete added a comment. In D55869#1337723 , @js wrote: > In D55869#1337711 , @dexonsmith > wrote: > > > In D55869#1337706 , @js wrote: > > > > > The ObjFW runtime itself does

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D55869#1337706 , @js wrote: > Thanks for tagging me! > > The ObjFW runtime itself does not contain anything about release, retain or > autorelease - these are all part of ObjFW (as in the framework). As ObjFW > also supports

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread Jonathan Schleifer via Phabricator via cfe-commits
js added a comment. In D55869#1337711 , @dexonsmith wrote: > In D55869#1337706 , @js wrote: > > > The ObjFW runtime itself does not contain anything about release, retain or > > autorelease - these are all part of

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D55869#1337706 , @js wrote: > The ObjFW runtime itself does not contain anything about release, retain or > autorelease - these are all part of ObjFW (as in the framework). As ObjFW > also supports the Apple runtime, as

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread Jonathan Schleifer via Phabricator via cfe-commits
js added a comment. In D55869#1337707 , @dexonsmith wrote: > In D55869#1337692 , @rjmccall wrote: > > > Okay. That's also presumably true for open-source runtimes that support > > ARC; tagging David Chisnall and

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In D55869#1337692 , @rjmccall wrote: > Okay. That's also presumably true for open-source runtimes that support ARC; > tagging David Chisnall and Jonathan Schleifer to get their input.

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread Jonathan Schleifer via Phabricator via cfe-commits
js added a comment. Thanks for tagging me! The ObjFW runtime itself does not contain anything about release, retain or autorelease - these are all part of ObjFW (as in the framework). As ObjFW also supports the Apple runtime, as well as mixing with Foundation code, it so far only provides

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added reviewers: theraven, js. rjmccall added a comment. Okay. That's also presumably true for open-source runtimes that support ARC; tagging David Chisnall and Jonathan Schleifer to get their input. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55869/new/

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-19 Thread Pete Cooper via Phabricator via cfe-commits
pete updated this revision to Diff 179017. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55869/new/ https://reviews.llvm.org/D55869 Files: include/clang/Basic/ObjCRuntime.h lib/CodeGen/CGObjC.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/CodeGenModule.h

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-19 Thread Pete Cooper via Phabricator via cfe-commits
pete marked 3 inline comments as done. pete added a comment. In D55869#1335783 , @rjmccall wrote: > So, once upon a time this was a problem because we were rewriting the method > calls in the runtime itself. Can you explain how that's not a problem now?

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. So, once upon a time this was a problem because we were rewriting the method calls in the runtime itself. Can you explain how that's not a problem now? Do we just expect the runtime to be compiled with the right switch? Comment at:

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

2018-12-18 Thread Pete Cooper via Phabricator via cfe-commits
pete created this revision. pete added reviewers: rjmccall, ahatanak, erik.pilkington. Herald added a subscriber: cfe-commits. It is faster to directly call the ObjC runtime for methods such as retain/release instead of sending a message to those functions. This is an extension of the work we