[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
ahatanak abandoned this revision. ahatanak added a comment. 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. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61970/new/ https://reviews.llvm.org/D61970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: lib/CodeGen/CGObjC.cpp:2611 + if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest()) +return EmitARCRetainAutoreleasedReturnValue(value, returnType); return emitObjCValueOperation( ahatanak wrote: > rjmccall wrote: > > 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? > 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. I meant the comment in https://reviews.llvm.org/D61808. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61970/new/ https://reviews.llvm.org/D61970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: lib/CodeGen/CGObjC.cpp:2611 + if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest()) +return EmitARCRetainAutoreleasedReturnValue(value, returnType); return emitObjCValueOperation( rjmccall wrote: > 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? 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. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61970/new/ https://reviews.llvm.org/D61970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
rjmccall added inline comments. Comment at: include/clang/Driver/Options.td:1500 + Flag<["-"], "fobjc-no-builtin-retain-release">, Group, + Flags<[CC1Option]>; def fno_objc_convert_messages_to_runtime_calls : 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`? Comment at: lib/CodeGen/CGObjC.cpp:2597 + if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest()) +return EmitARCAutoreleaseReturnValue(value, returnType); return emitObjCValueOperation( 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? Comment at: lib/CodeGen/CGObjC.cpp:2611 + if (!CGM.getCodeGenOpts().ObjCNoBuiltinRetainRelease && !getInvokeDest()) +return EmitARCRetainAutoreleasedReturnValue(value, returnType); return emitObjCValueOperation( 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? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61970/new/ https://reviews.llvm.org/D61970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
ahatanak updated this revision to Diff 199760.
ahatanak marked an inline comment as done.
ahatanak added a comment.
Run the main ARC optimization passes when `ObjCNoBuiltinRetainRelease` is not
set.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61970/new/
https://reviews.llvm.org/D61970
Files:
include/clang/Basic/CodeGenOptions.def
include/clang/Driver/Options.td
lib/CodeGen/BackendUtil.cpp
lib/CodeGen/CGObjC.cpp
lib/CodeGen/CodeGenFunction.h
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGenObjC/convert-messages-to-runtime-calls.m
Index: test/CodeGenObjC/convert-messages-to-runtime-calls.m
===
--- test/CodeGenObjC/convert-messages-to-runtime-calls.m
+++ test/CodeGenObjC/convert-messages-to-runtime-calls.m
@@ -1,12 +1,13 @@
// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fno-objc-convert-messages-to-runtime-calls -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
-// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
// RUN: %clang_cc1 -fobjc-runtime=macosx-10.9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
-// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
// RUN: %clang_cc1 -fobjc-runtime=ios-7.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
// Note: This line below is for tvos for which the driver passes through to use the ios9.0 runtime.
-// RUN: %clang_cc1 -fobjc-runtime=ios-9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
-// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
+// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions -fobjc-no-builtin-retain-release | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-NO-ARC-INTRINSICS
#define nil (id)0
@@ -28,9 +29,11 @@
// MSGS: {{call.*@objc_msgSend}}
// CALLS: {{call.*@objc_alloc}}
// CALLS: {{call.*@objc_allocWithZone}}
- // CALLS: {{call.*@objc_retain}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
// CALLS: {{call.*@objc_release}}
- // CALLS: {{call.*@objc_autorelease}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.autoreleaseReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_autorelease}}
[NSObject alloc];
[NSObject allocWithZone:nil];
[x retain];
@@ -111,7 +114,8 @@
// call will return i8* which we have to cast to A*
// CHECK-LABEL: define {{.*}}void @test_retain_class_ptr
A* test_retain_class_ptr(B *b) {
- // CALLS: {{call.*@objc_retain}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
// CALLS-NEXT: bitcast i8*
// CALLS-NEXT: ret
return [b retain];
@@ -121,7 +125,8 @@
// call will return i8* which we have to cast to A*
// CHECK-LABEL: define {{.*}}void @test_autorelease_class_ptr
A* test_autorelease_class_ptr(B *b) {
- // CALLS: {{call.*@objc_autorelease}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.autoreleaseReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_autorelease}}
// CALLS-NEXT: bitcast i8*
// CALLS-NEXT: ret
return [b autorelease];
@@ -161,7 +166,8 @@
// CHECK-LABEL: define {{.*}}void @retain_self
+ (void)retain_self {
// MSGS: {{call.*@objc_msgSend}}
- // CALLS: {{call.*@objc_retain}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
[self retain];
}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1163,6 +1163,8 @@
}
}
+ if
[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
ahatanak added inline comments.
Comment at: test/CodeGenObjC/convert-messages-to-runtime-calls.m:32
// CALLS: {{call.*@objc_allocWithZone}}
- // CALLS: {{call.*@objc_retain}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
ddkilzer wrote:
> Silly question: Should `objc_retainAutoreleasedReturnValue` really be called
> here when the value is not used in a `return` statement?
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.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61970/new/
https://reviews.llvm.org/D61970
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
ddkilzer added a comment.
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.)
Comment at: test/CodeGenObjC/convert-messages-to-runtime-calls.m:32
// CALLS: {{call.*@objc_allocWithZone}}
- // CALLS: {{call.*@objc_retain}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
Silly question: Should `objc_retainAutoreleasedReturnValue` really be called
here when the value is not used in a `return` statement?
Comment at: test/CodeGenObjC/convert-messages-to-runtime-calls.m:169
// MSGS: {{call.*@objc_msgSend}}
- // CALLS: {{call.*@objc_retain}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
Same silly question: Should `objc_retainAutoreleasedReturnValue` really be
called here when the value is not used in a `return` statement?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61970/new/
https://reviews.llvm.org/D61970
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D61970: [CodeGen][ObjC] Call objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue instead of objc_autorelease and objc_retain in MRR
ahatanak created this revision.
ahatanak added reviewers: pete, rjmccall, erik.pilkington.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.
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
Repository:
rC Clang
https://reviews.llvm.org/D61970
Files:
include/clang/Basic/CodeGenOptions.def
include/clang/Driver/Options.td
lib/CodeGen/CGObjC.cpp
lib/CodeGen/CodeGenFunction.h
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGenObjC/convert-messages-to-runtime-calls.m
Index: test/CodeGenObjC/convert-messages-to-runtime-calls.m
===
--- test/CodeGenObjC/convert-messages-to-runtime-calls.m
+++ test/CodeGenObjC/convert-messages-to-runtime-calls.m
@@ -1,12 +1,13 @@
// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fno-objc-convert-messages-to-runtime-calls -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
-// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
// RUN: %clang_cc1 -fobjc-runtime=macosx-10.9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
-// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
// RUN: %clang_cc1 -fobjc-runtime=ios-7.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=MSGS
// Note: This line below is for tvos for which the driver passes through to use the ios9.0 runtime.
-// RUN: %clang_cc1 -fobjc-runtime=ios-9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
-// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-9.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
+// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-ARC-INTRINSICS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-exceptions -fexceptions -fobjc-no-builtin-retain-release | FileCheck %s --check-prefix=CALLS --check-prefix=CALLS-NO-ARC-INTRINSICS
#define nil (id)0
@@ -28,9 +29,11 @@
// MSGS: {{call.*@objc_msgSend}}
// CALLS: {{call.*@objc_alloc}}
// CALLS: {{call.*@objc_allocWithZone}}
- // CALLS: {{call.*@objc_retain}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
// CALLS: {{call.*@objc_release}}
- // CALLS: {{call.*@objc_autorelease}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.autoreleaseReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_autorelease}}
[NSObject alloc];
[NSObject allocWithZone:nil];
[x retain];
@@ -111,7 +114,8 @@
// call will return i8* which we have to cast to A*
// CHECK-LABEL: define {{.*}}void @test_retain_class_ptr
A* test_retain_class_ptr(B *b) {
- // CALLS: {{call.*@objc_retain}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.retainAutoreleasedReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_retain}}
// CALLS-NEXT: bitcast i8*
// CALLS-NEXT: ret
return [b retain];
@@ -121,7 +125,8 @@
// call will return i8* which we have to cast to A*
// CHECK-LABEL: define {{.*}}void @test_autorelease_class_ptr
A* test_autorelease_class_ptr(B *b) {
- // CALLS: {{call.*@objc_autorelease}}
+ // CALLS-ARC-INTRINSICS: {{call.*@llvm.objc.autoreleaseReturnValue}}
+ // CALLS-NO-ARC-INTRINSICS: {{call.*@objc_autorelease}}
// CALLS-NEXT: bitcast i8*
// CALLS-NEXT: ret
return [b autorelease];
@@ -161,7 +166,8 @@
// CHECK-LABEL: define {{.*}}void @retain_self
+ (void)retain_self {
// MSGS:
