Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[Pete Cooper via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL263607: Convert some ObjC msgSends to runtime calls. 
(authored by pete).

Changed prior to commit:
  http://reviews.llvm.org/D14737?vs=50651=50785#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D14737

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

Index: cfe/trunk/lib/CodeGen/CGObjC.cpp
===
--- cfe/trunk/lib/CodeGen/CGObjC.cpp
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp
@@ -338,6 +338,69 @@
   return nullptr;
 }
 
+/// The ObjC runtime may provide entrypoints that are likely to be faster
+/// than an ordinary message send of the appropriate selector.
+///
+/// The entrypoints are guaranteed to be equivalent to just sending the
+/// corresponding message.  If the entrypoint is implemented naively as just a
+/// message send, using it is a trade-off: it sacrifices a few cycles of
+/// overhead to save a small amount of code.  However, it's possible for
+/// runtimes to detect and special-case classes that use "standard"
+/// retain/release behavior; if that's dynamically a large proportion of all
+/// retained objects, using the entrypoint will also be faster than using a
+/// message send.
+///
+/// If the runtime does support a required entrypoint, then this method will
+/// generate a call and return the resulting value.  Otherwise it will return
+/// None and the caller can generate a msgSend instead.
+static Optional
+tryGenerateSpecializedMessageSend(CodeGenFunction , QualType ResultType,
+  llvm::Value *Receiver, Selector Sel,
+  const ObjCMethodDecl *method) {
+  auto  = CGF.CGM;
+  if (!CGM.getCodeGenOpts().ObjCConvertMessagesToRuntimeCalls)
+return None;
+
+  auto  = CGM.getLangOpts().ObjCRuntime;
+  switch (Sel.getMethodFamily()) {
+  case OMF_alloc:
+// Make sure the name is exactly 'alloc'.  All methods with that
+// prefix are identified as OMF_alloc but we only want to call the
+// runtime for this version.
+if (Runtime.shouldUseRuntimeFunctionsForAlloc() && Sel.isUnarySelector() &&
+Sel.getNameForSlot(0) == "alloc" &&
+ResultType->isObjCObjectPointerType())
+  return CGF.EmitObjCAlloc(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_autorelease:
+if (ResultType->isObjCObjectPointerType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease())
+  return CGF.EmitARCAutorelease(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_retain:
+if (ResultType->isObjCObjectPointerType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease())
+  return CGF.EmitARCRetainNonBlock(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_release:
+if (ResultType->isVoidType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease()) {
+  CGF.EmitARCRelease(Receiver, ARCPreciseLifetime);
+  return nullptr;
+}
+break;
+  default:
+break;
+  }
+  return None;
+}
+
 RValue CodeGenFunction::EmitObjCMessageExpr(const ObjCMessageExpr *E,
 ReturnValueSlot Return) {
   // Only the lookup mechanism and first two arguments of the method
@@ -460,10 +523,16 @@
   Args,
   method);
   } else {
-result = Runtime.GenerateMessageSend(*this, Return, ResultType,
- E->getSelector(),
- Receiver, Args, OID,
- method);
+// Call runtime methods directly if we can.
+if (Optional SpecializedResult =
+tryGenerateSpecializedMessageSend(*this, ResultType, Receiver,
+  E->getSelector(), method)) {
+  result = RValue::get(SpecializedResult.getValue());
+} else {
+  result = Runtime.GenerateMessageSend(*this, Return, ResultType,
+   E->getSelector(), Receiver, Args,
+   OID, method);
+}
   }
 
   // For delegate init calls in ARC, implicitly store the result of
@@ -1814,6 +1883,7 @@
 /// where a null input causes a no-op and returns null.
 static llvm::Value 

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[John McCall via cfe-commits]

rjmccall added a comment.

Yes, LGTM.


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[Pete Cooper via cfe-commits]

pete added a comment.

In http://reviews.llvm.org/D14737#375739, @rjmccall wrote:

> Ah, okay, if you changed it to cast explicitly, that's all I was concerned 
> about.


Cool.  Thanks.  Any other concerns or does this look good to you?


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[John McCall via cfe-commits]

rjmccall added a comment.

Ah, okay, if you changed it to cast explicitly, that's all I was concerned 
about.


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[Pete Cooper via cfe-commits]

pete added a comment.

In http://reviews.llvm.org/D14737#375735, @rjmccall wrote:

> Can you find where that bitcast is being added?  I know that different parts 
> of IRGen are differently sensitive to types — it's possible that the return 
> code is one of those more-permissive places.


Sure, will do.

I should say that the bit cast here is my doing.  I should have said that I 
added code to emitARCValueOperation which optionally takes a return type to 
cast to, instead of always casting to the receiver type as you noted it was 
doing before.  But as you point out, even without that change, the IR is still 
getting bit casts when needed.


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[John McCall via cfe-commits]

rjmccall added a comment.

Can you find where that bitcast is being added?  I know that different parts of 
IRGen are differently sensitive to types — it's possible that the return code 
is one of those more-permissive places.


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[Pete Cooper via cfe-commits]

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

Thanks for the example John.  I understand what you mean now.

I've added this piece to the test case which verifies that the following IR has 
the correct bit cast in it.  Similarly added cases for alloc and autorelease.

@class A;
@interface B

- (A*) retain;

@end

A* test_retain_class_ptr(B *b) {

  return [b retain];

}

define %1* @test_retain_class_ptr(%2* %b) #0 {
entry:

  %b.addr = alloca %2*, align 8
  store %2* %b, %2** %b.addr, align 8
  %0 = load %2*, %2** %b.addr, align 8
  %1 = bitcast %2* %0 to i8*
  %2 = call i8* @objc_retain(i8* %1) #2
  %3 = bitcast i8* %2 to %1*
  ret %1* %3

}


http://reviews.llvm.org/D14737

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/Tools.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,129 @@
+// 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
+// Make sure we don't do calls to retain/release when using GC.
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-gc | FileCheck %s --check-prefix=GC
+// 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
+
+@interface NSObject
++ (id)alloc;
++ (id)alloc2;
+- (id)init;
+- (id)retain;
+- (void)release;
+- (id)autorelease;
+@end
+
+@interface NSString : NSObject
++ (void)retain_self;
+- (void)retain_super;
+@end
+
+// CHECK-LABEL: define {{.*}}void @test1
+void test1(id x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS: {{call.*@objc_retain}}
+  // CALLS: {{call.*@objc_release}}
+  // CALLS: {{call.*@objc_autorelease}}
+  // GC: {{call.*@objc_alloc}}
+  // GC: {{call.*@objc_msgSend}}
+  // GC: {{call.*@objc_msgSend}}
+  // GC: {{call.*@objc_msgSend}}
+  [NSObject alloc];
+  [x retain];
+  [x release];
+  [x autorelease];
+}
+
+// CHECK-LABEL: define {{.*}}void @test2
+void test2() {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // GC: {{call.*@objc_msgSend}}
+  // Make sure alloc has the correct name and number of types.
+  [NSObject alloc2];
+}
+
+@class A;
+@interface B
++ (A*) alloc;
+- (A*) retain;
+- (A*) autorelease;
+@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_retain_class_ptr
+A* test_retain_class_ptr(B *b) {
+  // CALLS: {{call.*@objc_retain}}
+  // CALLS-NEXT: 

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[John McCall via cfe-commits]

rjmccall added a comment.

In http://reviews.llvm.org/D14737#373532, @pete wrote:

> I stepped through this one in the debugger to make sure I had it right.
>
> So the reason the bit cast ends up not being needed is because we restricted 
> this optimization to cases where the result type "isObjCObjectPointerType()" 
> which conveniently ends up begin i8* and is exactly the same type as the 
> message receiver.  I guess if either the receiver type or the result type 
> wasn't represented as i8* then the IRBuilder would have crashed.
>
> If we permitted converting say messaging (int*)retain to a call to 
> objc_retain then the bit cast would be needed.  Would you like me to permit 
> this change on functions returning anything other than 
> isObjCObjectPointerType(), eg, change that to "isAnyPointerType()"?


The IR type for an AST type satisfying isObjCObjectPointerType() isn't always 
i8*; in particular, that's not true when the receiver has a concrete class type.

A test case here would be something like:

@interface Foo

- (id) retain;

@end

...

id test(Foo *foo) {

  return [foo retain];

}

Here the receiver type will be something like %struct.Foo* and the expected 
return type will be i8*.


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[Pete Cooper via cfe-commits]

pete added a comment.

Hi John

Sorry, getting back to this after way too long!

In http://reviews.llvm.org/D14737#294218, @rjmccall wrote:

> In http://reviews.llvm.org/D14737#293967, @pete wrote:
>
> > Added a couple of tests for retain returning types other than id.  
> > Returning a pointer should still be converted to a call, while returning a 
> > non-pointer such as float will get a message instead.
> >
> > I walked through the code in the debugger to check on the return cast.  
> > Turns out it is handled at the very end of emitARCValueOperation as follows:
> >
> >  
> >   // Cast the result back to the original type.
> >   return CGF.Builder.CreateBitCast(call, origType);
>
>
> Right, that'll cast back to the original type.  That will be the type of the 
> receiver of the message, which is not necessarily the type of the result of 
> the message.


I stepped through this one in the debugger to make sure I had it right.

So the reason the bit cast ends up not being needed is because we restricted 
this optimization to cases where the result type "isObjCObjectPointerType()" 
which conveniently ends up begin i8* and is exactly the same type as the 
message receiver.  I guess if either the receiver type or the result type 
wasn't represented as i8* then the IRBuilder would have crashed.

If we permitted converting say messaging (int*)retain to a call to objc_retain 
then the bit cast would be needed.  Would you like me to permit this change on 
functions returning anything other than isObjCObjectPointerType(), eg, change 
that to "isAnyPointerType()"?

Cheers,
Pete


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[Pete Cooper via cfe-commits]

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

Added a couple of tests for retain returning types other than id.  Returning a 
pointer should still be converted to a call, while returning a non-pointer such 
as float will get a message instead.

I walked through the code in the debugger to check on the return cast.  Turns 
out it is handled at the very end of emitARCValueOperation as follows:

  // Cast the result back to the original type.
  return CGF.Builder.CreateBitCast(call, origType);


http://reviews.llvm.org/D14737

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/Tools.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,109 @@
+// 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
+// Make sure we don't do calls to retain/release when using GC.
+// RUN: %clang_cc1 -fobjc-runtime=macosx-10.10.0 -emit-llvm -o - %s -fobjc-gc | FileCheck %s --check-prefix=GC
+// 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
+
+@interface NSObject
++ (id)alloc;
++ (id)alloc2;
+- (id)init;
+- (id)retain;
+- (void)release;
+- (id)autorelease;
+@end
+
+@interface NSString : NSObject
++ (void)retain_self;
+- (void)retain_super;
+@end
+
+// CHECK-LABEL: define {{.*}}void @test1
+void test1(id x) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_alloc}}
+  // CALLS: {{call.*@objc_retain}}
+  // CALLS: {{call.*@objc_release}}
+  // CALLS: {{call.*@objc_autorelease}}
+  // GC: {{call.*@objc_alloc}}
+  // GC: {{call.*@objc_msgSend}}
+  // GC: {{call.*@objc_msgSend}}
+  // GC: {{call.*@objc_msgSend}}
+  [NSObject alloc];
+  [x retain];
+  [x release];
+  [x autorelease];
+}
+
+// CHECK-LABEL: define {{.*}}void @test2
+void test2() {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // GC: {{call.*@objc_msgSend}}
+  // Make sure alloc has the correct name and number of types.
+  [NSObject alloc2];
+}
+
+@class A;
+@interface B
+- (A*) retain;
+@end
+
+// Make sure we get a bitcast on the return type as the
+// objc_retain call will return i8* which we have to cast
+// to A*
+// CHECK-LABEL: define {{.*}}void @test_return_bitcast
+A* test_return_bitcast(B *b) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_retain}}
+  // CALLS-NEXT: bitcast i8*
+  // GC: {{call.*@objc_msgSend}}
+  return [b retain];
+}
+
+@interface C
+- (float) retain;
+@end
+
+// Make sure we use a message and not a call as the return type is
+// not a pointer type.
+// CHECK-LABEL: define {{.*}}void @test_cannot_message_return_float
+float test_cannot_message_return_float(C *c) {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  // GC: {{call.*@objc_msgSend}}
+  return [c retain];

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[John McCall via cfe-commits]

rjmccall added a comment.

In http://reviews.llvm.org/D14737#293967, @pete wrote:

> Added a couple of tests for retain returning types other than id.  Returning 
> a pointer should still be converted to a call, while returning a non-pointer 
> such as float will get a message instead.
>
> I walked through the code in the debugger to check on the return cast.  Turns 
> out it is handled at the very end of emitARCValueOperation as follows:
>
>  
>   // Cast the result back to the original type.
>   return CGF.Builder.CreateBitCast(call, origType);


Right, that'll cast back to the original type.  That will be the type of the 
receiver of the message, which is not necessarily the type of the result of the 
message.


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[John McCall via cfe-commits]

rjmccall added a comment.

The casts done by emitARCValueOperation will handle the input, but they don't 
quite handle the result properly.  The right test case here is a method named 
"retain" that's declared to return something completely unrelated to its 
receiver type, e.g.

  @class A;
  @interface B
  - (A*) retain;
  @end

You should also add a test case for unusual return types that your mechanism 
just can't support and that need to go through the ordinary message-send 
emission, like returning a float.



Comment at: test/CodeGenObjC/convert-messages-to-runtime-calls.m:62
@@ +61,3 @@
+
+// Make sure we can convert a message to a dynamic receiver to a call
+// CHECK-LABEL: define {{.*}}void @retain_self

You already test this case.  If you make retain_self a class method, this will 
be a class message; but it should still be okay to use objc_retain.


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[Pete Cooper via cfe-commits]

> On Nov 18, 2015, at 4:21 PM, John McCall  wrote:
> 
> rjmccall added inline comments.
> 
> 
> Comment at: include/clang/Basic/ObjCRuntime.h:182
> @@ +181,3 @@
> +switch (getKind()) {
> +case FragileMacOSX: return false;
> +case MacOSX: return getVersion() >= VersionTuple(10, 10);
> 
> I went ahead and asked Greg, and he agreed that it's best to not enable this 
> on the Mac fragile runtime.  The functions exist, but they aren't profitable 
> to call, other than a small code-size decrease.
Ah cool.  Thanks for checking.

I’m working on updating the patch now with your feedback.  Should have a new 
version soon.

I will now also add tests to ensure the fragile runtime does not get this 
change, since i’d been missing that before.

Thanks,
Pete
> 
> 
> http://reviews.llvm.org/D14737
> 
> 
> 

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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[John McCall via cfe-commits]

rjmccall added inline comments.


Comment at: include/clang/Basic/ObjCRuntime.h:182
@@ +181,3 @@
+switch (getKind()) {
+case FragileMacOSX: return false;
+case MacOSX: return getVersion() >= VersionTuple(10, 10);

I went ahead and asked Greg, and he agreed that it's best to not enable this on 
the Mac fragile runtime.  The functions exist, but they aren't profitable to 
call, other than a small code-size decrease.


http://reviews.llvm.org/D14737



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

Re: [PATCH] D14737: Convert some ObjC msgSends to runtime calls

[Stephane Moore via cfe-commits]

stephanemoore added a subscriber: stephanemoore.
stephanemoore added a comment.

I hope that it's not presumptuous of me to inquire but I was wondering if the 
intent of this patch is to optimize calls to RR methods (and alloc) in non-ARC 
code? Would I be correct in assuming that clang will already emit direct calls 
to relevant RR runtime functions when ARC is enabled?


http://reviews.llvm.org/D14737



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