[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd18fbfc09720: Relax the rules around objc_alloc and
objc_alloc_init optimizations. (authored by MadCoder).
Changed prior to commit:
https://reviews.llvm.org/D71682?vs=234644=238161#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71682/new/
https://reviews.llvm.org/D71682
Files:
clang/lib/CodeGen/CGObjC.cpp
clang/test/CodeGenObjC/objc-alloc-init.m
Index: clang/test/CodeGenObjC/objc-alloc-init.m
===
--- clang/test/CodeGenObjC/objc-alloc-init.m
+++ clang/test/CodeGenObjC/objc-alloc-init.m
@@ -22,21 +22,30 @@
}
@interface Y : X
++(Class)class;
+(void)meth;
-(void)instanceMeth;
@end
@implementation Y
++(Class)class {
+ return self;
+}
+(void)meth {
[[self alloc] init];
// OPTIMIZED: call i8* @objc_alloc_init(
// NOT_OPTIMIZED: call i8* @objc_alloc(
}
++ (void)meth2 {
+ [[[self class] alloc] init];
+ // OPTIMIZED: call i8* @objc_alloc_init(
+ // NOT_OPTIMIZED: call i8* @objc_alloc(
+}
-(void)instanceMeth {
// EITHER-NOT: call i8* @objc_alloc
// EITHER: call {{.*}} @objc_msgSend
// EITHER: call {{.*}} @objc_msgSend
- [[self alloc] init];
+ [[(id)self alloc] init];
}
@end
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -461,38 +461,39 @@
Sel.getNameForSlot(0) != "init")
return None;
- // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]' or
- // we are in an ObjC class method and 'receiver' is '[self alloc]'.
+ // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]'
+ // with 'cls' a Class.
auto *SubOME =
dyn_cast(OME->getInstanceReceiver()->IgnoreParenCasts());
if (!SubOME)
return None;
Selector SubSel = SubOME->getSelector();
- // Check if we are in an ObjC class method and the receiver expression is
- // 'self'.
- const Expr *SelfInClassMethod = nullptr;
- if (const auto *CurMD = dyn_cast_or_null(CGF.CurFuncDecl))
-if (CurMD->isClassMethod())
- if ((SelfInClassMethod = SubOME->getInstanceReceiver()))
-if (!SelfInClassMethod->isObjCSelfExpr())
- SelfInClassMethod = nullptr;
-
- if ((SubOME->getReceiverKind() != ObjCMessageExpr::Class &&
- !SelfInClassMethod) || !SubOME->getType()->isObjCObjectPointerType() ||
+ if (!SubOME->getType()->isObjCObjectPointerType() ||
!SubSel.isUnarySelector() || SubSel.getNameForSlot(0) != "alloc")
return None;
- llvm::Value *Receiver;
- if (SelfInClassMethod) {
-Receiver = CGF.EmitScalarExpr(SelfInClassMethod);
- } else {
+ llvm::Value *Receiver = nullptr;
+ switch (SubOME->getReceiverKind()) {
+ case ObjCMessageExpr::Instance:
+if (!SubOME->getInstanceReceiver()->getType()->isObjCClassType())
+ return None;
+Receiver = CGF.EmitScalarExpr(SubOME->getInstanceReceiver());
+break;
+
+ case ObjCMessageExpr::Class: {
QualType ReceiverType = SubOME->getClassReceiver();
const ObjCObjectType *ObjTy = ReceiverType->castAs();
const ObjCInterfaceDecl *ID = ObjTy->getInterface();
assert(ID && "null interface should be impossible here");
Receiver = CGF.CGM.getObjCRuntime().GetClass(CGF, ID);
+break;
+ }
+ case ObjCMessageExpr::SuperInstance:
+ case ObjCMessageExpr::SuperClass:
+return None;
}
+
return CGF.EmitObjCAllocInit(Receiver, CGF.ConvertType(OME->getType()));
}
@@ -540,10 +541,7 @@
switch (E->getReceiverKind()) {
case ObjCMessageExpr::Instance:
ReceiverType = E->getInstanceReceiver()->getType();
-if (auto *OMD = dyn_cast_or_null(CurFuncDecl))
- if (OMD->isClassMethod())
-if (E->getInstanceReceiver()->isObjCSelfExpr())
- isClassMessage = true;
+isClassMessage = ReceiverType->isObjCClassType();
if (retainSelf) {
TryEmitResult ter = tryEmitARCRetainScalarExpr(*this,
E->getInstanceReceiver());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71682/new/ https://reviews.llvm.org/D71682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.
ahatanak added inline comments.
Comment at: clang/test/CodeGenObjC/objc-alloc-init.m:30
@implementation Y
++(Class)class {
MadCoder wrote:
> ahatanak wrote:
> > Can you add a test case for `[[self class] alloc]` to test the code in
> > `tryGenerateSpecializedMessageSend`?
> isn't it what meth2 is doing?
Ah, right. I didn't see the NOT_OPTIMIZED line.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71682/new/
https://reviews.llvm.org/D71682
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.
MadCoder marked an inline comment as done.
MadCoder added inline comments.
Comment at: clang/test/CodeGenObjC/objc-alloc-init.m:30
@implementation Y
++(Class)class {
ahatanak wrote:
> Can you add a test case for `[[self class] alloc]` to test the code in
> `tryGenerateSpecializedMessageSend`?
isn't it what meth2 is doing?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71682/new/
https://reviews.llvm.org/D71682
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.
ahatanak added inline comments.
Comment at: clang/test/CodeGenObjC/objc-alloc-init.m:30
@implementation Y
++(Class)class {
Can you add a test case for `[[self class] alloc]` to test the code in
`tryGenerateSpecializedMessageSend`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71682/new/
https://reviews.llvm.org/D71682
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.
MadCoder created this revision.
MadCoder added reviewers: rjmccall, arphaman, erik.pilkington, ahatanak.
MadCoder added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.
MadCoder edited the summary of this revision.
Today the optimization is limited to:
- `[ClassName alloc]`
- `[self alloc] when within a class method`
However it means that when code is written this way:
@interface MyObject
- (id)copyWithZone:(NSZone *)zone
{
return [[self.class alloc] _initWith...];
}
@end
... then the optimization doesn't kick in and +[NSObject alloc] ends up in IMP
caches where it could have been avoided.
It turns out that +alloc -> +[NSObject alloc] is the most cached SEL/IMP pair
in the entire platform which is rather silly).
There's two theoretical risks allowing this optimization:
1. if the receiver is nil (which it can't be today), but it turns out that
objc_alloc()/objc_alloc_init() cope with a nil receiver,
2. if the `Class` type for the receiver is a lie. However, for such a code to
work today (and not fail witn an unrecognized selector anyway) you'd have to
have implemented the -alloc *instance method*.
Fortunately, objc_alloc() doesn't assume that the receiver is a Class, it
basically starts with a test that is similar to `if (receiver->isa->bits &
hasDefaultAWZ) { /* fastpath */ }`. This bit is only set on metaclasses by the
runtime, so if an instance is passed to this function by accident, its isa will
fail this test, and objc_alloc() will gracefully fallback to objc_msgSend().
The one thing objc_alloc() doesn't support is tagged pointer instances. None
of the tagged pointer classes implement an instance method called 'alloc'
(actually there's a single class in the entire Apple codebase that has such a
method).
Radar-Id: rdar://problem/58058316
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D71682
Files:
clang/lib/CodeGen/CGObjC.cpp
clang/test/CodeGenObjC/objc-alloc-init.m
Index: clang/test/CodeGenObjC/objc-alloc-init.m
===
--- clang/test/CodeGenObjC/objc-alloc-init.m
+++ clang/test/CodeGenObjC/objc-alloc-init.m
@@ -22,21 +22,30 @@
}
@interface Y : X
++(Class)class;
+(void)meth;
-(void)instanceMeth;
@end
@implementation Y
++(Class)class {
+ return self;
+}
+(void)meth {
[[self alloc] init];
// OPTIMIZED: call i8* @objc_alloc_init(
// NOT_OPTIMIZED: call i8* @objc_alloc(
}
++ (void)meth2 {
+ [[[self class] alloc] init];
+ // OPTIMIZED: call i8* @objc_alloc_init(
+ // NOT_OPTIMIZED: call i8* @objc_alloc(
+}
-(void)instanceMeth {
// EITHER-NOT: call i8* @objc_alloc
// EITHER: call {{.*}} @objc_msgSend
// EITHER: call {{.*}} @objc_msgSend
- [[self alloc] init];
+ [[(id)self alloc] init];
}
@end
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -461,38 +461,39 @@
Sel.getNameForSlot(0) != "init")
return None;
- // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]' or
- // we are in an ObjC class method and 'receiver' is '[self alloc]'.
+ // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]'
+ // with 'cls' a Class.
auto *SubOME =
dyn_cast(OME->getInstanceReceiver()->IgnoreParenCasts());
if (!SubOME)
return None;
Selector SubSel = SubOME->getSelector();
- // Check if we are in an ObjC class method and the receiver expression is
- // 'self'.
- const Expr *SelfInClassMethod = nullptr;
- if (const auto *CurMD = dyn_cast_or_null(CGF.CurFuncDecl))
-if (CurMD->isClassMethod())
- if ((SelfInClassMethod = SubOME->getInstanceReceiver()))
-if (!SelfInClassMethod->isObjCSelfExpr())
- SelfInClassMethod = nullptr;
-
- if ((SubOME->getReceiverKind() != ObjCMessageExpr::Class &&
- !SelfInClassMethod) || !SubOME->getType()->isObjCObjectPointerType() ||
+ if (!SubOME->getType()->isObjCObjectPointerType() ||
!SubSel.isUnarySelector() || SubSel.getNameForSlot(0) != "alloc")
return None;
- llvm::Value *Receiver;
- if (SelfInClassMethod) {
-Receiver = CGF.EmitScalarExpr(SelfInClassMethod);
- } else {
+ llvm::Value *Receiver = nullptr;
+ switch (SubOME->getReceiverKind()) {
+ case ObjCMessageExpr::Instance:
+if (!SubOME->getInstanceReceiver()->getType()->isObjCClassType())
+ return None;
+Receiver = CGF.EmitScalarExpr(SubOME->getInstanceReceiver());
+break;
+
+ case ObjCMessageExpr::Class: {
QualType ReceiverType = SubOME->getClassReceiver();
const ObjCObjectType *ObjTy = ReceiverType->getAs();
const ObjCInterfaceDecl *ID = ObjTy->getInterface();
assert(ID && "null interface should be impossible here");
Receiver = CGF.CGM.getObjCRuntime().GetClass(CGF, ID);
+break;
+ }
+ case ObjCMessageExpr::SuperInstance:
+ case
