[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/

https://reviews.llvm.org/D55869



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

[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/

https://reviews.llvm.org/D55869

Files:
  include/clang/Basic/ObjCRuntime.h
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  test/CodeGenObjC/convert-messages-to-runtime-calls.m

Index: lib/CodeGen/CGObjC.cpp
===
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -396,6 +396,29 @@
 }
 break;
 
+  case OMF_autorelease:
+if (ResultType->isObjCObjectPointerType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease())
+  return CGF.EmitObjCAutorelease(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_retain:
+if (ResultType->isObjCObjectPointerType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease())
+  return CGF.EmitObjCRetainNonBlock(Receiver, CGF.ConvertType(ResultType));
+break;
+
+  case OMF_release:
+if (ResultType->isVoidType() &&
+CGM.getLangOpts().getGC() == LangOptions::NonGC &&
+Runtime.shouldUseARCFunctionsForRetainRelease()) {
+  CGF.EmitObjCRelease(Receiver, ARCPreciseLifetime);
+  return nullptr;
+}
+break;
+
   default:
 break;
   }
@@ -2006,6 +2029,11 @@
 llvm::FunctionType *fnType =
   llvm::FunctionType::get(CGF.Int8PtrTy, CGF.Int8PtrTy, false);
 fn = CGF.CGM.CreateRuntimeFunction(fnType, fnName);
+
+// We have Native ARC, so set nonlazybind attribute for performance
+if (llvm::Function *f = dyn_cast(fn))
+  if (fnName == "objc_retain")
+f->addFnAttr(llvm::Attribute::NonLazyBind);
   }
 
   // Cast the argument to 'id'.
@@ -2510,6 +2538,55 @@
   CGF.EmitARCIntrinsicUse(value);
 }
 
+/// Autorelease the given object.
+///   call i8* \@objc_autorelease(i8* %value)
+llvm::Value *CodeGenFunction::EmitObjCAutorelease(llvm::Value *value,
+  llvm::Type *returnType) {
+  return emitObjCValueOperation(*this, value, returnType,
+  CGM.getObjCEntrypoints().objc_autoreleaseRuntimeFunction,
+"objc_autorelease");
+}
+
+/// Retain the given object, with normal retain semantics.
+///   call i8* \@objc_retain(i8* %value)
+llvm::Value *CodeGenFunction::EmitObjCRetainNonBlock(llvm::Value *value,
+ llvm::Type *returnType) {
+  return emitObjCValueOperation(*this, value, returnType,
+  CGM.getObjCEntrypoints().objc_retainRuntimeFunction,
+"objc_retain");
+}
+
+/// Release the given object.
+///   call void \@objc_release(i8* %value)
+void CodeGenFunction::EmitObjCRelease(llvm::Value *value,
+  ARCPreciseLifetime_t precise) {
+  if (isa(value)) return;
+
+  llvm::Constant * = CGM.getObjCEntrypoints().objc_release;
+  if (!fn) {
+if (!fn) {
+  llvm::FunctionType *fnType =
+llvm::FunctionType::get(Builder.getVoidTy(), Int8PtrTy, false);
+  fn = CGM.CreateRuntimeFunction(fnType, "objc_release");
+  setARCRuntimeFunctionLinkage(CGM, fn);
+  // We have Native ARC, so set nonlazybind attribute for performance
+  if (llvm::Function *f = dyn_cast(fn))
+f->addFnAttr(llvm::Attribute::NonLazyBind);
+}
+  }
+
+  // Cast the argument to 'id'.
+  value = Builder.CreateBitCast(value, Int8PtrTy);
+
+  // Call objc_release.
+  llvm::CallInst *call = EmitNounwindRuntimeCall(fn, value);
+
+  if (precise == ARCImpreciseLifetime) {
+call->setMetadata("clang.imprecise_release",
+  llvm::MDNode::get(Builder.getContext(), None));
+  }
+}
+
 namespace {
   struct CallObjCAutoreleasePoolObject final : EHScopeStack::Cleanup {
 llvm::Value *Token;
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -138,6 +138,10 @@
   /// id objc_autorelease(id);
   llvm::Constant *objc_autorelease;
 
+  /// id objc_autorelease(id);
+  /// Note this is the runtime method not the intrinsic.
+  llvm::Constant *objc_autoreleaseRuntimeFunction;
+
   /// id objc_autoreleaseReturnValue(id);
   llvm::Constant *objc_autoreleaseReturnValue;
 
@@ -162,6 +166,10 @@
   /// id objc_retain(id);
   llvm::Constant *objc_retain;
 
+  /// id objc_retain(id);
+  /// Note this is the runtime method not the intrinsic.
+  llvm::Constant *objc_retainRuntimeFunction;
+
   /// id objc_retainAutorelease(id);
   llvm::Constant *objc_retainAutorelease;
 
@@ -177,6 +185,10 @@
   /// void objc_release(id);

[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 mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/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 mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/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 newer version of clang.  I can easily enable a 
> new flag in the next release, but doing so for older ones is more problematic.


Well, it won't break as long as you don't tell the compiler that this is an 
acceptable rewrite to do.  I'm not sure there's a perfect way to stage this 
if/when you start thinking about enabling that — you can of course start 
passing the "don't rewrite message sends" flag in new releases of GNUstep, but 
it'll always be *possible* that someone might use a newer Clang to compile an 
older version of the runtime that doesn't do that workaround.  On the other 
hand, the brokenness of the result will not be subtle.


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

https://reviews.llvm.org/D55869



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

[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 doing so for older ones is more problematic.


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

https://reviews.llvm.org/D55869



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/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#1337733 , @pete wrote:

> In D55869#1337723 , @js wrote:
>
> > 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 ObjFW (as in the 
> > > > framework). As ObjFW also supports the Apple runtime, as well as mixing 
> > > > with Foundation code, it so far only provides objc_retain and 
> > > > objc_release when they are missing, to make ARC work. They just call 
> > > > into the corresponding methods on the object and do nothing else.
> > > >
> > > > How will this change work from the Apple runtime? Will 
> > > > objc_retain/objc_release call the retain/release on the object if it 
> > > > implements its own? Should I drop my own retain/release implementation 
> > > > from my root object if I am compiling against the Apple runtime?
> > >
> > >
> > > Yes, the idea is that the specialized runtime functions are fast paths 
> > > for the common case, but they still fall back if the object has 
> > > overridden them.
> >
> >
> > I am guessing not just overridden, but also a root class providing one? 
> > IOW, the fast path is used when the class does not have the retain or 
> > release method at all? In that case, LGTM as is.
>
>
> The Apple runtime is using a bit on each realized class to track the 
> overriding.  The root class defaults to not having a custom RR, ie, it 
> appears that its version of RR is considered equivalent to objc_retain.
>
> Presumably that would apply to other root classes too, although i'm not 100% 
> sure.  I did see some special handling of NSObject in there too so perhaps 
> only its RR is equivalent to objc_retain/objc_release.


`objc_retain` and `objc_release` are always defined as being equivalent to 
doing a normal message send of the corresponding messages.  Apple's 
implementations detect when a class simply inherits `NSObject`'s 
implementations: if so, they just directly use the `NSObject` RR 
implementation, and if no, they do a normal message send.  That fast path is 
only triggered if the class's root class is `NSObject` and it doesn't override 
any of the RR methods.


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

https://reviews.llvm.org/D55869



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

[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 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 objc_retain and objc_release when they are 
> > > missing, to make ARC work. They just call into the corresponding methods 
> > > on the object and do nothing else.
> > >
> > > How will this change work from the Apple runtime? Will 
> > > objc_retain/objc_release call the retain/release on the object if it 
> > > implements its own? Should I drop my own retain/release implementation 
> > > from my root object if I am compiling against the Apple runtime?
> >
> >
> > Yes, the idea is that the specialized runtime functions are fast paths for 
> > the common case, but they still fall back if the object has overridden them.
>
>
> I am guessing not just overridden, but also a root class providing one? IOW, 
> the fast path is used when the class does not have the retain or release 
> method at all? In that case, LGTM as is.


The Apple runtime is using a bit on each realized class to track the 
overriding.  The root class defaults to not having a custom RR, ie, it appears 
that its version of RR is considered equivalent to objc_retain.

Presumably that would apply to other root classes too, although i'm not 100% 
sure.  I did see some special handling of NSObject in there too so perhaps only 
its RR is equivalent to objc_retain/objc_release.


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

https://reviews.llvm.org/D55869



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/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#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 the Apple runtime, as well as mixing with Foundation code, it 
> so far only provides objc_retain and objc_release when they are missing, to 
> make ARC work. They just call into the corresponding methods on the object 
> and do nothing else.
>
> How will this change work from the Apple runtime? Will 
> objc_retain/objc_release call the retain/release on the object if it 
> implements its own? Should I drop my own retain/release implementation from 
> my root object if I am compiling against the Apple runtime?


Nothing is fundamentally changing.  The Apple runtime has fast paths in 
`objc_retain` / `objc_release` for `NSObject`'s implementation of 
retain/release, and this patch allows us to take advantage of that in non-ARC 
files which explicitly call `-retain` and `-release`.  Of course, the runtime's 
own implementation of `objc_retain` and `objc_release` contains such calls, 
which would be miscompiled into recursive calls to `objc_retain` and 
`objc_release` without the command-line flag.

> I'm mostly concerned about mixing ObjFW and Foundation code while using the 
> Apple runtime here. ObjFW with the ObjFW runtime and no Foundation is easy to 
> change.

I assume you only define `objc_retain` and `objc_release` when compiling 
against your own runtime.  In that case, as Duncan reminded me, there's no risk 
of miscompiling as long as `shouldUseARCFunctionsForRetainRelease()` returns 
false for your runtime.  Since it sounds like your implementation doesn't try 
to avoid the message-send for common classes, there's no reason you would make 
that method return true; but if you ever want to, you'll need this compiler 
flag to avoid miscompiles.


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

https://reviews.llvm.org/D55869



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

[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 ObjFW (as in the framework). As ObjFW 
> > also supports the Apple runtime, as well as mixing with Foundation code, it 
> > so far only provides objc_retain and objc_release when they are missing, to 
> > make ARC work. They just call into the corresponding methods on the object 
> > and do nothing else.
> >
> > How will this change work from the Apple runtime? Will 
> > objc_retain/objc_release call the retain/release on the object if it 
> > implements its own? Should I drop my own retain/release implementation from 
> > my root object if I am compiling against the Apple runtime?
>
>
> Yes, the idea is that the specialized runtime functions are fast paths for 
> the common case, but they still fall back if the object has overridden them.


I am guessing not just overridden, but also a root class providing one? IOW, 
the fast path is used when the class does not have the retain or release method 
at all? In that case, LGTM as is.


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

https://reviews.llvm.org/D55869



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

[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 well as mixing with Foundation code, it 
> so far only provides objc_retain and objc_release when they are missing, to 
> make ARC work. They just call into the corresponding methods on the object 
> and do nothing else.
>
> How will this change work from the Apple runtime? Will 
> objc_retain/objc_release call the retain/release on the object if it 
> implements its own? Should I drop my own retain/release implementation from 
> my root object if I am compiling against the Apple runtime?


Yes, the idea is that the specialized runtime functions are fast paths for the 
common case, but they still fall back if the object has overridden them.


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

https://reviews.llvm.org/D55869



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

[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 Jonathan Schleifer to get their input.
>
>
> `shouldUseARCFunctionsForRetainRelease()` returns `false` for the other 
> runtimes.  This seems like the right thing to do until/unless they add 
> support.
>
> Would make sense for the driver to additionally imply 
> `-fno-objc-convert-messages-to-runtime-calls` for other runtimes as a bigger 
> hammer?


We could make it true here (happy to implement it), but the question about how 
this works with multiple root objects with different retain/release 
implementations remains.


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

https://reviews.llvm.org/D55869



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

[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.


`shouldUseARCFunctionsForRetainRelease()` returns `false` for the other 
runtimes.  This seems like the right thing to do until/unless they add support.

Would make sense for the driver to additionally imply 
`-fno-objc-convert-messages-to-runtime-calls` for other runtimes as a bigger 
hammer?


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

https://reviews.llvm.org/D55869



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

[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 objc_retain and objc_release when they are missing, to make ARC 
work. They just call into the corresponding methods on the object and do 
nothing else.

How will this change work from the Apple runtime? Will objc_retain/objc_release 
call the retain/release on the object if it implements its own? Should I drop 
my own retain/release implementation from my root object if I am compiling 
against the Apple runtime?

I'm mostly concerned about mixing ObjFW and Foundation code while using the 
Apple runtime here. ObjFW with the ObjFW runtime and no Foundation is easy to 
change.


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

https://reviews.llvm.org/D55869



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/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 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/

https://reviews.llvm.org/D55869



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

[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
  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
@@ -14,16 +14,28 @@
 + (id)alloc;
 + (id)allocWithZone:(void*)zone;
 + (id)alloc2;
+- (id)retain;
+- (void)release;
+- (id)autorelease;
 @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}}
   // MSGS: {{call.*@objc_msgSend}}
   // CALLS: {{call.*@objc_alloc}}
   // CALLS: {{call.*@objc_allocWithZone}}
+  // CALLS: {{call.*@objc_retain}}
+  // CALLS: {{call.*@objc_release}}
+  // CALLS: {{call.*@objc_autorelease}}
   [NSObject alloc];
   [NSObject allocWithZone:nil];
+  [x retain];
+  [x release];
+  [x autorelease];
 }
 
 // CHECK-LABEL: define {{.*}}void @test2
@@ -43,6 +55,8 @@
 @interface B
 + (A*) alloc;
 + (A*)allocWithZone:(void*)zone;
+- (A*) retain;
+- (A*) autorelease;
 @end
 
 // Make sure we get a bitcast on the return type as the
@@ -65,9 +79,30 @@
   return [B allocWithZone:nil];
 }
 
+// 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: bitcast i8*
+  // CALLS-NEXT: ret
+  return [b retain];
+}
+
+// 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_autorelease_class_ptr
+A* test_autorelease_class_ptr(B *b) {
+  // CALLS: {{call.*@objc_autorelease}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [b autorelease];
+}
+
 
 @interface C
 + (id)allocWithZone:(int)intArg;
+- (float) retain;
 @end
 
 // Make sure we only accept pointer types
@@ -78,3 +113,37 @@
   return [C allocWithZone:3];
 }
 
+// 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}}
+  return [c retain];
+}
+
+@interface NSString : NSObject
++ (void)retain_self;
+- (void)retain_super;
+@end
+
+@implementation NSString
+
+// Make sure we can convert a message to a dynamic receiver to a call
+// CHECK-LABEL: define {{.*}}void @retain_self
++ (void)retain_self {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_retain}}
+  [self retain];
+}
+
+// Make sure we never convert a message to super to a call
+// CHECK-LABEL: define {{.*}}void @retain_super
+- (void)retain_super {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  [super retain];
+}
+
+@end
+
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -138,6 +138,10 @@
   /// id objc_autorelease(id);
   llvm::Constant *objc_autorelease;
 
+  /// id objc_autorelease(id);
+  /// Note this is the runtime method not the intrinsic.
+  llvm::Constant *objc_autoreleaseRuntimeFunction;
+
   /// id objc_autoreleaseReturnValue(id);
   llvm::Constant *objc_autoreleaseReturnValue;
 
@@ -162,6 +166,10 @@
   /// id objc_retain(id);
   llvm::Constant *objc_retain;
 
+  /// id objc_retain(id);
+  /// Note this is the runtime method not the intrinsic.
+  llvm::Constant *objc_retainRuntimeFunction;
+
   /// id objc_retainAutorelease(id);
   llvm::Constant *objc_retainAutorelease;
 
@@ -177,6 +185,10 @@
   /// void objc_release(id);
   llvm::Constant *objc_release;
 
+  /// void objc_release(id);
+  /// Note this is the runtime method not the intrinsic.
+  llvm::Constant *objc_releaseRuntimeFunction;
+
   /// void objc_storeStrong(id*, id);
   llvm::Constant *objc_storeStrong;
 
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3798,6 +3798,11 @@
   llvm::Value *EmitARCRetainAutoreleasedReturnValue(llvm::Value *value);
   llvm::Value *EmitARCUnsafeClaimAutoreleasedReturnValue(llvm::Value *value);
 
+  llvm::Value *EmitObjCAutorelease(llvm::Value *value, llvm::Type *returnType);
+  llvm::Value *EmitObjCRetainNonBlock(llvm::Value *value,
+  llvm::Type *returnType);
+  void EmitObjCRelease(llvm::Value *value, ARCPreciseLifetime_t precise);
+

[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?  
> Do we just expect the runtime to be compiled with the right switch?


Ah yeah, good point.

In the patch for rewriting objc_alloc messages (r348687) I added a new option 
(-fno-objc-convert-messages-to-runtime-calls) which the objc runtime will need 
to adopt.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55869



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

[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: include/clang/Basic/ObjCRuntime.h:191
+  ///   retain => objc_retain
+  ///   release => objc_release
+  bool shouldUseARCFunctionsForRetainRelease() const {

You're also rewriting `autorelease`.



Comment at: include/clang/Basic/ObjCRuntime.h:207
+  case ObjFW:
+return false;
+}

Case indentation.



Comment at: lib/CodeGen/CodeGenModule.h:170
+  /// id objc_retain(id);
+  /// Note this is the runtime method not the intrinsic
+  llvm::Constant *objc_retainRuntimeFunction;

Period (and on the other comments here)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55869



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

[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 already do to convert alloc message sends 
to objc_alloc calls.


Repository:
  rC Clang

https://reviews.llvm.org/D55869

Files:
  include/clang/Basic/ObjCRuntime.h
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  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
@@ -14,16 +14,28 @@
 + (id)alloc;
 + (id)allocWithZone:(void*)zone;
 + (id)alloc2;
+- (id)retain;
+- (void)release;
+- (id)autorelease;
 @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}}
   // MSGS: {{call.*@objc_msgSend}}
   // CALLS: {{call.*@objc_alloc}}
   // CALLS: {{call.*@objc_allocWithZone}}
+  // CALLS: {{call.*@objc_retain}}
+  // CALLS: {{call.*@objc_release}}
+  // CALLS: {{call.*@objc_autorelease}}
   [NSObject alloc];
   [NSObject allocWithZone:nil];
+  [x retain];
+  [x release];
+  [x autorelease];
 }
 
 // CHECK-LABEL: define {{.*}}void @test2
@@ -43,6 +55,8 @@
 @interface B
 + (A*) alloc;
 + (A*)allocWithZone:(void*)zone;
+- (A*) retain;
+- (A*) autorelease;
 @end
 
 // Make sure we get a bitcast on the return type as the
@@ -65,9 +79,30 @@
   return [B allocWithZone:nil];
 }
 
+// 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: bitcast i8*
+  // CALLS-NEXT: ret
+  return [b retain];
+}
+
+// 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_autorelease_class_ptr
+A* test_autorelease_class_ptr(B *b) {
+  // CALLS: {{call.*@objc_autorelease}}
+  // CALLS-NEXT: bitcast i8*
+  // CALLS-NEXT: ret
+  return [b autorelease];
+}
+
 
 @interface C
 + (id)allocWithZone:(int)intArg;
+- (float) retain;
 @end
 
 // Make sure we only accept pointer types
@@ -78,3 +113,37 @@
   return [C allocWithZone:3];
 }
 
+// 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}}
+  return [c retain];
+}
+
+@interface NSString : NSObject
++ (void)retain_self;
+- (void)retain_super;
+@end
+
+@implementation NSString
+
+// Make sure we can convert a message to a dynamic receiver to a call
+// CHECK-LABEL: define {{.*}}void @retain_self
++ (void)retain_self {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_retain}}
+  [self retain];
+}
+
+// Make sure we never convert a message to super to a call
+// CHECK-LABEL: define {{.*}}void @retain_super
+- (void)retain_super {
+  // MSGS: {{call.*@objc_msgSend}}
+  // CALLS: {{call.*@objc_msgSend}}
+  [super retain];
+}
+
+@end
+
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -138,6 +138,10 @@
   /// id objc_autorelease(id);
   llvm::Constant *objc_autorelease;
 
+  /// id objc_autorelease(id);
+  /// Note this is the runtime method not the intrinsic
+  llvm::Constant *objc_autoreleaseRuntimeFunction;
+
   /// id objc_autoreleaseReturnValue(id);
   llvm::Constant *objc_autoreleaseReturnValue;
 
@@ -162,6 +166,10 @@
   /// id objc_retain(id);
   llvm::Constant *objc_retain;
 
+  /// id objc_retain(id);
+  /// Note this is the runtime method not the intrinsic
+  llvm::Constant *objc_retainRuntimeFunction;
+
   /// id objc_retainAutorelease(id);
   llvm::Constant *objc_retainAutorelease;
 
@@ -177,6 +185,10 @@
   /// void objc_release(id);
   llvm::Constant *objc_release;
 
+  /// void objc_release(id);
+  /// Note this is the runtime method not the intrinsic
+  llvm::Constant *objc_releaseRuntimeFunction;
+
   /// void objc_storeStrong(id*, id);
   llvm::Constant *objc_storeStrong;
 
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3798,6 +3798,11 @@
   llvm::Value *EmitARCRetainAutoreleasedReturnValue(llvm::Value *value);
   llvm::Value *EmitARCUnsafeClaimAutoreleasedReturnValue(llvm::Value *value);
 
+