Page MenuHomePhabricator

[CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
AbandonedPublic

Authored by ahatanak on May 15 2019, 5:00 PM.

Details

Summary

This patch makes IRGen emit ObjC runtime functions (objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue) that were previously emitted only in ARC mode. This enables retain message sends in MRR code to participate in the retainRV/autoreleaseRV handshake, which keeps returned objects out of the autorelease pool. Also, it enables the ARC optimizer and ARC contract pass to remove retain/autorelease pairs and mark autorelease message sends converted to autoreleaseRV calls as tail calls, which is necessary for the retainRV/autoreleaseRV handshake to succeed.

rdar://problem/50353574

Diff Detail

Repository
rC Clang

Event Timeline

ahatanak created this revision.May 15 2019, 5:00 PM

Had a couple questions about using objc_retainAutoreleasedReturnValue without a return statement. (I'm just reading this from a layman's point of view; it may not actually matter in practice.)

test/CodeGenObjC/convert-messages-to-runtime-calls.m
32–33

Silly question: Should objc_retainAutoreleasedReturnValue really be called here when the value is not used in a return statement?

169–170

Same silly question: Should objc_retainAutoreleasedReturnValue really be called here when the value is not used in a return statement?

ahatanak updated this revision to Diff 199760.May 15 2019, 11:57 PM
ahatanak marked an inline comment as done.

Run the main ARC optimization passes when ObjCNoBuiltinRetainRelease is not set.

test/CodeGenObjC/convert-messages-to-runtime-calls.m
32–33

ARC optimizer has an optimization that converts a call to objc_retainAutoreleasedReturnValue to a call to objc_retain when it doesn't use the result of a call.

http://llvm.org/doxygen/ObjCARCOpts_8cpp_source.html

Also, it converts a call to objc_autoreleaseReturnValue to a call to objc_autorelease when its result isn't used by a return instruction.

rjmccall added inline comments.May 16 2019, 9:48 AM
include/clang/Driver/Options.td
1500

I know I suggested this name, but it should probably be -fno-objc-.... instead of -fobjc-no-.....

Please add help text. Also, there probably ought to be a subsection in the user docs under "Objective-C Features" which talks about message rewriting; it should start by discussing the general fact that we rewrite messages, including how to disable it with -fno-objc-convert-messages-to-runtime-calls, and then segue into the special assumptions for retain/release/autorelease and how to disable them with this option.

Should this be implied by -fno-objc-convert-messages-to-runtime-calls?

lib/CodeGen/CGObjC.cpp
2597

Is using autoreleaseRV in all situations a good idea? Among other things, won't this make it significantly more difficult to detect miscompiles where we fail to tail call autoreleaseRV?

2611

Same question: won't this make it significantly more difficult to detect when we're generating bad code? And the overhead here (on every retain in MRC!) is pretty significant; shouldn't this at least be restricted to situations where it's plausible that it might be reclaiming something, like when the receiver expression is a function call of some sort?

ahatanak marked an inline comment as done.May 16 2019, 3:05 PM
ahatanak added inline comments.
lib/CodeGen/CGObjC.cpp
2611

As I mentioned in my response to David's question, I was thinking the ARC optimizer would be able to convert an objc_retainAutoreleasedReturnValue call to an objc_retain call. However, after reading your comment in https://reviews.llvm.org/D61970, I don't think it's okay to use the ARC optimizer to transform code that was compiled in MRR mode.

ahatanak marked an inline comment as done.May 16 2019, 3:10 PM
ahatanak added inline comments.
lib/CodeGen/CGObjC.cpp
2611

I meant the comment in https://reviews.llvm.org/D61808.

ahatanak abandoned this revision.May 24 2019, 1:23 PM

This isn't the right way to make a call to objc_autorelease a tail call, so I'm closing this for now. If we decide to convert calls to objc_autorelease and objc_retain to calls to objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue for other reasons in the future, I can reopen this.