[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete added a comment.

In D55349#1323926 , @rjmccall wrote:

> LGTM.


Thanks for the review!  Landed in r348687.


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

https://reviews.llvm.org/D55349



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D55349



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete updated this revision to Diff 177308.
pete marked an inline comment as done.
pete added a comment.

Added test for integer argument and updated code to only accept pointer types 
to allocWithZone.


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

https://reviews.llvm.org/D55349

Files:
  include/clang/Basic/ObjCRuntime.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenObjC/convert-messages-to-runtime-calls.m
  test/Driver/objc-convert-messages-to-runtime-calls.m

Index: test/Driver/objc-convert-messages-to-runtime-calls.m
===
--- /dev/null
+++ test/Driver/objc-convert-messages-to-runtime-calls.m
@@ -0,0 +1,7 @@
+// RUN: %clang %s -### -o %t.o 2>&1 -fsyntax-only -fobjc-convert-messages-to-runtime-calls -fno-objc-convert-messages-to-runtime-calls -target x86_64-apple-macosx10.10.0 | FileCheck  %s --check-prefix=DISABLE
+// RUN: %clang %s -### -o %t.o 2>&1 -fsyntax-only -fno-objc-convert-messages-to-runtime-calls -fobjc-convert-messages-to-runtime-calls -target x86_64-apple-macosx10.10.0 | FileCheck  %s --check-prefix=ENABLE
+
+// Check that we pass fobjc-convert-messages-to-runtime-calls only when supported, and not explicitly disabled.
+
+// DISABLE: "-fno-objc-convert-messages-to-runtime-calls"
+// ENABLE-NOT: "-fno-objc-convert-messages-to-runtime-calls"
Index: test/CodeGenObjC/convert-messages-to-runtime-calls.m
===
--- /dev/null
+++ test/CodeGenObjC/convert-messages-to-runtime-calls.m
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fno-objc-convert-messages-to-runtime-calls | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.9.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-7.0 -emit-llvm -o - %s | 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 | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+
+#define nil (id)0
+
+@interface NSObject
++ (id)alloc;
++ (id)allocWithZone:(void*)zone;
++ (id)alloc2;
+@end
+
+// CHECK-LABEL: define {{.*}}void @test1
+void test1(id x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS: {{call.*@objc_allocWithZone}}
+  [NSObject alloc];
+  [NSObject allocWithZone:nil];
+}
+
+// CHECK-LABEL: define {{.*}}void @test2
+void test2(void* x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  [NSObject alloc2];
+  [NSObject allocWithZone:(void*)-1];
+  [NSObject allocWithZone:x];
+}
+
+@class A;
+@interface B
++ (A*) alloc;
++ (A*)allocWithZone:(void*)zone;
+@end
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_alloc_class_ptr
+A* test_alloc_class_ptr() {
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [B alloc];
+}
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_alloc_class_ptr
+A* test_allocWithZone_class_ptr() {
+  // CALLS: {{call.*@objc_allocWithZone}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [B allocWithZone:nil];
+}
+
+
+@interface C
++ (id)allocWithZone:(int)intArg;
+@end
+
+// Make sure we only accept pointer types
+// CHECK-LABEL: define {{.*}}void @test_allocWithZone_int
+C* test_allocWithZone_int() {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  return [C allocWithZone:3];
+}
+
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1155,6 +1155,10 @@
 }
   }
 
+
+  if (Args.hasArg(OPT_fno_objc_convert_messages_to_runtime_calls))
+Opts.ObjCConvertMessagesToRuntimeCalls = 0;
+
   if (Args.getLastArg(OPT_femulated_tls) ||
   Args.getLastArg(OPT_fno_emulated_tls)) {
 Opts.Explic

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGObjC.cpp:385
+// prefix are identified as OMF_alloc but we only want to call the
+// runtime for this version.
+if (Sel.isUnarySelector() && Sel.getNameForSlot(0) == "alloc")

Actually, I'd suggest just rewriting this comment to look like the next one, it 
conveys the same idea but much more succinctly.



Comment at: lib/CodeGen/CGObjC.cpp:390
+if (Sel.isKeywordSelector() && Sel.getNumArgs() == 1 &&
+Args.size() == 1 && Args.front().getKnownRValue().isScalar() &&
+Sel.getNameForSlot(0) == "allocWithZone") {

You should check `!Args.front().hasLValue()` here before calling 
`getKnownRValue()`.  The test case would be some silly example where 
`-allocWithZone:` is declared with a parameter that's some big `struct` and the 
argument expression just loads it immediately from an l-value.  Or you could 
just check that the parameter type is a pointer type.


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

https://reviews.llvm.org/D55349



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete marked 5 inline comments as done.
pete added inline comments.



Comment at: include/clang/Basic/ObjCRuntime.h:200
+case WatchOS:
+  return true;
+

rjmccall wrote:
> Did we really add this so long ago?  Wow.
Yeah!  I was thinking that too!


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

https://reviews.llvm.org/D55349



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete updated this revision to Diff 177246.
pete added a comment.

Thanks for the review.  Have fixed the comments as suggested.


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

https://reviews.llvm.org/D55349

Files:
  include/clang/Basic/ObjCRuntime.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenObjC/convert-messages-to-runtime-calls.m
  test/Driver/objc-convert-messages-to-runtime-calls.m

Index: test/Driver/objc-convert-messages-to-runtime-calls.m
===
--- /dev/null
+++ test/Driver/objc-convert-messages-to-runtime-calls.m
@@ -0,0 +1,7 @@
+// RUN: %clang %s -### -o %t.o 2>&1 -fsyntax-only -fobjc-convert-messages-to-runtime-calls -fno-objc-convert-messages-to-runtime-calls -target x86_64-apple-macosx10.10.0 | FileCheck  %s --check-prefix=DISABLE
+// RUN: %clang %s -### -o %t.o 2>&1 -fsyntax-only -fno-objc-convert-messages-to-runtime-calls -fobjc-convert-messages-to-runtime-calls -target x86_64-apple-macosx10.10.0 | FileCheck  %s --check-prefix=ENABLE
+
+// Check that we pass fobjc-convert-messages-to-runtime-calls only when supported, and not explicitly disabled.
+
+// DISABLE: "-fno-objc-convert-messages-to-runtime-calls"
+// ENABLE-NOT: "-fno-objc-convert-messages-to-runtime-calls"
Index: test/CodeGenObjC/convert-messages-to-runtime-calls.m
===
--- /dev/null
+++ test/CodeGenObjC/convert-messages-to-runtime-calls.m
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fno-objc-convert-messages-to-runtime-calls | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.9.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-7.0 -emit-llvm -o - %s | 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 | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+
+#define nil (id)0
+
+@interface NSObject
++ (id)alloc;
++ (id)allocWithZone:(void*)zone;
++ (id)alloc2;
+@end
+
+// CHECK-LABEL: define {{.*}}void @test1
+void test1(id x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_alloc}}
+  [NSObject alloc];
+  [NSObject allocWithZone:nil];
+}
+
+// CHECK-LABEL: define {{.*}}void @test2
+void test2(void* x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  [NSObject alloc2];
+  [NSObject allocWithZone:(void*)-1];
+  [NSObject allocWithZone:x];
+}
+
+@class A;
+@interface B
++ (A*) alloc;
++ (A*)allocWithZone:(void*)zone;
+@end
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_alloc_class_ptr
+A* test_alloc_class_ptr() {
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [B alloc];
+}
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_alloc_class_ptr
+A* test_allocWithZone_class_ptr() {
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [B allocWithZone:nil];
+}
+
+
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1155,6 +1155,10 @@
 }
   }
 
+
+  if (Args.hasArg(OPT_fno_objc_convert_messages_to_runtime_calls))
+Opts.ObjCConvertMessagesToRuntimeCalls = 0;
+
   if (Args.getLastArg(OPT_femulated_tls) ||
   Args.getLastArg(OPT_fno_emulated_tls)) {
 Opts.ExplicitEmulatedTLS = true;
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2864,6 +2864,18 @@
 Args.ClaimAllArgs(options::OPT_fno_objc_arc_exceptions);
   }
 
+  // Allow the user to control whether messages can be converted to runtime
+  // functions.
+  if (types::isObjC(Input.getType())) {
+auto *Arg = Args.getLastAr

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/ObjCRuntime.h:189
+  /// When this method returns true, Clang will turn non-super message sends of
+  /// certain selectors into calls to the correspond entrypoint:
+  ///   alloc => objc_alloc

"corresponding"



Comment at: include/clang/Basic/ObjCRuntime.h:190
+  /// certain selectors into calls to the correspond entrypoint:
+  ///   alloc => objc_alloc
+  bool shouldUseRuntimeFunctionsForAlloc() const {

The actual code also rewrites `allocWithZone:` (which is why it's reasonable to 
pluralize "functions" in the method name).



Comment at: include/clang/Basic/ObjCRuntime.h:200
+case WatchOS:
+  return true;
+

Did we really add this so long ago?  Wow.



Comment at: lib/CodeGen/CGObjC.cpp:383
+// prefix are identified as OMF_alloc but we only want to call the
+// runtime for this version.
+if (Runtime.shouldUseRuntimeFunctionsForAlloc() &&

Using the word "selector" instead of "name" is both more technically accurate 
and nicely side-steps the difference between "alloc" (which you want to 
include) and "alloc:" (which you don't).  You have this correct in the code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55349



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55349: Convert some ObjC alloc msgSends to runtime calls

[Pete Cooper via Phabricator via cfe-commits]

pete created this revision.
pete added reviewers: ahatanak, rjmccall, erik.pilkington.
Herald added a subscriber: cfe-commits.

It is faster to directly call the ObjC runtime for methods such as 
alloc/allocWithZone instead of sending a message to those functions.

This patch adds support for converting messages to alloc/allocWithZone to their 
equivalent runtime calls.

Tests included for the positive case of applying this transformation, negative 
tests that we ensure we only convert "alloc" to objc_alloc, not "alloc2", and 
also a driver test to ensure we enable this only for supported runtime versions.


Repository:
  rC Clang

https://reviews.llvm.org/D55349

Files:
  include/clang/Basic/ObjCRuntime.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenObjC/convert-messages-to-runtime-calls.m
  test/Driver/objc-convert-messages-to-runtime-calls.m

Index: test/Driver/objc-convert-messages-to-runtime-calls.m
===
--- /dev/null
+++ test/Driver/objc-convert-messages-to-runtime-calls.m
@@ -0,0 +1,7 @@
+// RUN: %clang %s -### -o %t.o 2>&1 -fsyntax-only -fobjc-convert-messages-to-runtime-calls -fno-objc-convert-messages-to-runtime-calls -target x86_64-apple-macosx10.10.0 | FileCheck  %s --check-prefix=DISABLE
+// RUN: %clang %s -### -o %t.o 2>&1 -fsyntax-only -fno-objc-convert-messages-to-runtime-calls -fobjc-convert-messages-to-runtime-calls -target x86_64-apple-macosx10.10.0 | FileCheck  %s --check-prefix=ENABLE
+
+// Check that we pass fobjc-convert-messages-to-runtime-calls only when supported, and not explicitly disabled.
+
+// DISABLE: "-fno-objc-convert-messages-to-runtime-calls"
+// ENABLE-NOT: "-fno-objc-convert-messages-to-runtime-calls"
Index: test/CodeGenObjC/convert-messages-to-runtime-calls.m
===
--- /dev/null
+++ test/CodeGenObjC/convert-messages-to-runtime-calls.m
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fno-objc-convert-messages-to-runtime-calls | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.9.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile-10.10.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=MSGS
+// RUN: %clang_cc1 -fobjc-runtime=ios-8.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=ios-7.0 -emit-llvm -o - %s | 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 | FileCheck %s --check-prefix=CALLS
+// RUN: %clang_cc1 -fobjc-runtime=watchos-2.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CALLS
+
+#define nil (id)0
+
+@interface NSObject
++ (id)alloc;
++ (id)allocWithZone:(void*)zone;
++ (id)alloc2;
+@end
+
+// CHECK-LABEL: define {{.*}}void @test1
+void test1(id x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_alloc}}
+  [NSObject alloc];
+  [NSObject allocWithZone:nil];
+}
+
+// CHECK-LABEL: define {{.*}}void @test2
+void test2(void* x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  [NSObject alloc2];
+  [NSObject allocWithZone:(void*)-1];
+  [NSObject allocWithZone:x];
+}
+
+@class A;
+@interface B
++ (A*) alloc;
++ (A*)allocWithZone:(void*)zone;
+@end
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_alloc_class_ptr
+A* test_alloc_class_ptr() {
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [B alloc];
+}
+
+// Make sure we get a bitcast on the return type as the
+// call will return i8* which we have to cast to A*
+// CHECK-LABEL: define {{.*}}void @test_alloc_class_ptr
+A* test_allocWithZone_class_ptr() {
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [B allocWithZone:nil];
+}
+
+
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1155,6 +1155,10 @@
 }
   }
 
+
+  if (Args.hasArg(OPT_fno_objc_convert_messages_to_runtime_calls))
+Opts.ObjCConvertMessagesToRuntimeCalls = 0;
+
   if (Args.getLastArg(OPT_femulated_tls) ||
   Args.getLastArg(OPT_fno_emulated_tls)) {
 Opts.ExplicitEmulatedTLS =