-
Notifications
You must be signed in to change notification settings - Fork 11.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable direct methods and fast alloc calls for libobjc2. #78030
Conversation
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: David Chisnall (davidchisnall) ChangesThese will be supported in the upcoming 2.2 release and so are gated on that version. Direct methods call Full diff: https://github.com/llvm/llvm-project/pull/78030.diff 3 Files Affected:
diff --git a/clang/include/clang/Basic/ObjCRuntime.h b/clang/include/clang/Basic/ObjCRuntime.h
index f05debe6fea512..1ccf60f0b7bee7 100644
--- a/clang/include/clang/Basic/ObjCRuntime.h
+++ b/clang/include/clang/Basic/ObjCRuntime.h
@@ -211,7 +211,13 @@ class ObjCRuntime {
case GCC:
return false;
case GNUstep:
- return false;
+ // This could be enabled for all versions, except for the fact that the
+ // implementation of `objc_retain` and friends prior to 2.2 call [object
+ // retain] in their fall-back paths, which leads to infinite recursion if
+ // the runtime is built with this enabled. Since distributions typically
+ // build all Objective-C things with the same compiler version and flags,
+ // it's better to be conservative here.
+ return (getVersion() >= VersionTuple(2, 2));
case ObjFW:
return false;
}
@@ -248,7 +254,7 @@ class ObjCRuntime {
case GCC:
return false;
case GNUstep:
- return false;
+ return getVersion() >= VersionTuple(2, 2);
case ObjFW:
return false;
}
@@ -266,6 +272,8 @@ class ObjCRuntime {
return getVersion() >= VersionTuple(12, 2);
case WatchOS:
return getVersion() >= VersionTuple(5, 2);
+ case GNUstep:
+ return getVersion() >= VersionTuple(2, 2);
default:
return false;
}
@@ -463,7 +471,8 @@ class ObjCRuntime {
case iOS: return true;
case WatchOS: return true;
case GCC: return false;
- case GNUstep: return false;
+ case GNUstep:
+ return (getVersion() >= VersionTuple(2, 2));
case ObjFW: return false;
}
llvm_unreachable("bad kind");
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 4ca1a8cce64d89..a71f42169fc3e6 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -18,6 +18,8 @@
#include "CGObjCRuntime.h"
#include "CodeGenFunction.h"
#include "CodeGenModule.h"
+#include "CodeGenTypes.h"
+#include "SanitizerMetadata.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
#include "clang/AST/Decl.h"
@@ -595,6 +597,10 @@ class CGObjCGNU : public CGObjCRuntime {
llvm::Function *GenerateMethod(const ObjCMethodDecl *OMD,
const ObjCContainerDecl *CD) override;
+
+ // Map to unify direct method definitions.
+ llvm::DenseMap<const ObjCMethodDecl *, llvm::Function *>
+ DirectMethodDefinitions;
void GenerateDirectMethodPrologue(CodeGenFunction &CGF, llvm::Function *Fn,
const ObjCMethodDecl *OMD,
const ObjCContainerDecl *CD) override;
@@ -926,6 +932,8 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
/// structure describing the receiver and the class, and a selector as
/// arguments. Returns the IMP for the corresponding method.
LazyRuntimeFunction MsgLookupSuperFn;
+ /// Function to ensure that +initialize is sent to a class.
+ LazyRuntimeFunction SentInitializeFn;
/// A flag indicating if we've emitted at least one protocol.
/// If we haven't, then we need to emit an empty protocol, to ensure that the
/// __start__objc_protocols and __stop__objc_protocols sections exist.
@@ -1981,6 +1989,8 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
CGObjCGNUstep2(CodeGenModule &Mod) : CGObjCGNUstep(Mod, 10, 4, 2) {
MsgLookupSuperFn.init(&CGM, "objc_msg_lookup_super", IMPTy,
PtrToObjCSuperTy, SelectorTy);
+ SentInitializeFn.init(&CGM, "objc_send_initialize",
+ llvm::Type::getVoidTy(VMContext), IdTy);
// struct objc_property
// {
// const char *name;
@@ -1994,6 +2004,106 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
{ PtrToInt8Ty, PtrToInt8Ty, PtrToInt8Ty, PtrToInt8Ty, PtrToInt8Ty });
}
+ void GenerateDirectMethodPrologue(CodeGenFunction &CGF, llvm::Function *Fn,
+ const ObjCMethodDecl *OMD,
+ const ObjCContainerDecl *CD) override {
+ auto &Builder = CGF.Builder;
+ bool ReceiverCanBeNull = true;
+ auto selfAddr = CGF.GetAddrOfLocalVar(OMD->getSelfDecl());
+ auto selfValue = Builder.CreateLoad(selfAddr);
+
+ // Generate:
+ //
+ // /* unless the receiver is never NULL */
+ // if (self == nil) {
+ // return (ReturnType){ };
+ // }
+ //
+ // /* for class methods only to force class lazy initialization */
+ // if (!__objc_{class}_initialized)
+ // {
+ // objc_send_initialize(class);
+ // __objc_{class}_initialized = 1;
+ // }
+ //
+ // _cmd = @selector(...)
+ // ...
+
+ if (OMD->isClassMethod()) {
+ const ObjCInterfaceDecl *OID = cast<ObjCInterfaceDecl>(CD);
+
+ // Nullable `Class` expressions cannot be messaged with a direct method
+ // so the only reason why the receive can be null would be because
+ // of weak linking.
+ ReceiverCanBeNull = isWeakLinkedClass(OID);
+ }
+
+ if (ReceiverCanBeNull) {
+ llvm::BasicBlock *SelfIsNilBlock =
+ CGF.createBasicBlock("objc_direct_method.self_is_nil");
+ llvm::BasicBlock *ContBlock =
+ CGF.createBasicBlock("objc_direct_method.cont");
+
+ // if (self == nil) {
+ auto selfTy = cast<llvm::PointerType>(selfValue->getType());
+ auto Zero = llvm::ConstantPointerNull::get(selfTy);
+
+ llvm::MDBuilder MDHelper(CGM.getLLVMContext());
+ Builder.CreateCondBr(Builder.CreateICmpEQ(selfValue, Zero),
+ SelfIsNilBlock, ContBlock,
+ MDHelper.createBranchWeights(1, 1 << 20));
+
+ CGF.EmitBlock(SelfIsNilBlock);
+
+ // return (ReturnType){ };
+ auto retTy = OMD->getReturnType();
+ Builder.SetInsertPoint(SelfIsNilBlock);
+ if (!retTy->isVoidType()) {
+ CGF.EmitNullInitialization(CGF.ReturnValue, retTy);
+ }
+ CGF.EmitBranchThroughCleanup(CGF.ReturnBlock);
+ // }
+
+ // rest of the body
+ CGF.EmitBlock(ContBlock);
+ Builder.SetInsertPoint(ContBlock);
+ }
+
+ if (OMD->isClassMethod()) {
+ // Prefix of the class type.
+ auto *classStart =
+ llvm::StructType::get(PtrTy, PtrTy, PtrTy, LongTy, LongTy);
+ auto &astContext = CGM.getContext();
+ auto flags = Builder.CreateLoad(
+ Address{Builder.CreateStructGEP(classStart, selfValue, 4), LongTy,
+ CharUnits::fromQuantity(
+ astContext.getTypeAlign(astContext.UnsignedLongTy))});
+ auto isInitialized = Builder.CreateAnd(flags, (1 << 8));
+ llvm::BasicBlock *notInitializedBlock =
+ CGF.createBasicBlock("objc_direct_method.send_initialize");
+ llvm::BasicBlock *initializedBlock =
+ CGF.createBasicBlock("objc_direct_method.class_initialized");
+ llvm::MDBuilder MDHelper(CGM.getLLVMContext());
+ Builder.CreateCondBr(Builder.CreateICmpEQ(isInitialized, Zeros[0]),
+ notInitializedBlock, initializedBlock,
+ MDHelper.createBranchWeights(1, 1 << 20));
+ CGF.EmitBlock(notInitializedBlock);
+ Builder.SetInsertPoint(notInitializedBlock);
+ CGF.EmitRuntimeCall(SentInitializeFn, selfValue);
+ Builder.CreateBr(initializedBlock);
+ CGF.EmitBlock(initializedBlock);
+ Builder.SetInsertPoint(initializedBlock);
+ }
+
+ // only synthesize _cmd if it's referenced
+ if (OMD->getCmdDecl()->isUsed()) {
+ // `_cmd` is not a parameter to direct methods, so storage must be
+ // explicitly declared for it.
+ CGF.EmitVarDecl(*OMD->getCmdDecl());
+ Builder.CreateStore(GetSelector(CGF, OMD),
+ CGF.GetAddrOfLocalVar(OMD->getCmdDecl()));
+ }
+ }
};
const char *const CGObjCGNUstep2::SectionsBaseNames[8] =
@@ -2631,13 +2741,18 @@ CGObjCGNU::GenerateMessageSend(CodeGenFunction &CGF,
}
}
+ bool isDirect = Method && Method->isDirectMethod();
+
IdTy = cast<llvm::PointerType>(CGM.getTypes().ConvertType(ASTIdTy));
llvm::Value *cmd;
- if (Method)
- cmd = GetSelector(CGF, Method);
- else
- cmd = GetSelector(CGF, Sel);
- cmd = EnforceType(Builder, cmd, SelectorTy);
+ if (!isDirect) {
+ if (Method)
+ cmd = GetSelector(CGF, Method);
+ else
+ cmd = GetSelector(CGF, Sel);
+ cmd = EnforceType(Builder, cmd, SelectorTy);
+ }
+
Receiver = EnforceType(Builder, Receiver, IdTy);
llvm::Metadata *impMD[] = {
@@ -2649,7 +2764,8 @@ CGObjCGNU::GenerateMessageSend(CodeGenFunction &CGF,
CallArgList ActualArgs;
ActualArgs.add(RValue::get(Receiver), ASTIdTy);
- ActualArgs.add(RValue::get(cmd), CGF.getContext().getObjCSelType());
+ if (!isDirect)
+ ActualArgs.add(RValue::get(cmd), CGF.getContext().getObjCSelType());
ActualArgs.addFrom(CallArgs);
MessageSendInfo MSI = getMessageSendInfo(Method, ResultType, ActualArgs);
@@ -2668,7 +2784,7 @@ CGObjCGNU::GenerateMessageSend(CodeGenFunction &CGF,
// Rather than doing a whole target-specific analysis, we assume it
// only works for void, integer, and pointer types, and in all
// other cases we do an explicit nil check is emitted code. In
- // addition to ensuring we produe a zero value for other types, this
+ // addition to ensuring we produce a zero value for other types, this
// sidesteps the few outright CC incompatibilities we know about that
// could otherwise lead to crashes, like when a method is expected to
// return on the x87 floating point stack or adjust the stack pointer
@@ -2702,8 +2818,9 @@ CGObjCGNU::GenerateMessageSend(CodeGenFunction &CGF,
// FIXME: we probably need a size limit here, but we've
// never imposed one before
} else {
- // Otherwise, use an explicit check just to be sure.
- requiresExplicitZeroResult = true;
+ // Otherwise, use an explicit check just to be sure, unless we're
+ // calling a direct method, where the implementation does this for us.
+ requiresExplicitZeroResult = !isDirect;
}
}
@@ -2747,10 +2864,14 @@ CGObjCGNU::GenerateMessageSend(CodeGenFunction &CGF,
// Get the IMP to call
llvm::Value *imp;
- // If we have non-legacy dispatch specified, we try using the objc_msgSend()
- // functions. These are not supported on all platforms (or all runtimes on a
- // given platform), so we
- switch (CGM.getCodeGenOpts().getObjCDispatchMethod()) {
+ // If this is a direct method, just emit it here.
+ if (isDirect)
+ imp = GenerateMethod(Method, Method->getClassInterface());
+ else
+ // If we have non-legacy dispatch specified, we try using the
+ // objc_msgSend() functions. These are not supported on all platforms
+ // (or all runtimes on a given platform), so we
+ switch (CGM.getCodeGenOpts().getObjCDispatchMethod()) {
case CodeGenOptions::Legacy:
imp = LookupIMP(CGF, Receiver, cmd, node, MSI);
break;
@@ -2773,7 +2894,7 @@ CGObjCGNU::GenerateMessageSend(CodeGenFunction &CGF,
llvm::FunctionType::get(IdTy, IdTy, true), "objc_msgSend")
.getCallee();
}
- }
+ }
// Reset the receiver in case the lookup modified it
ActualArgs[0] = CallArg(RValue::get(Receiver), ASTIdTy);
@@ -2783,7 +2904,8 @@ CGObjCGNU::GenerateMessageSend(CodeGenFunction &CGF,
llvm::CallBase *call;
CGCallee callee(CGCalleeInfo(), imp);
RValue msgRet = CGF.EmitCall(MSI.CallInfo, callee, Return, ActualArgs, &call);
- call->setMetadata(msgSendMDKind, node);
+ if (!isDirect)
+ call->setMetadata(msgSendMDKind, node);
if (requiresNilReceiverCheck) {
llvm::BasicBlock *nonNilPathBB = CGF.Builder.GetInsertBlock();
@@ -3906,14 +4028,50 @@ llvm::Function *CGObjCGNU::GenerateMethod(const ObjCMethodDecl *OMD,
CodeGenTypes &Types = CGM.getTypes();
llvm::FunctionType *MethodTy =
Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
- std::string FunctionName = getSymbolNameForMethod(OMD);
-
- llvm::Function *Method
- = llvm::Function::Create(MethodTy,
- llvm::GlobalValue::InternalLinkage,
- FunctionName,
- &TheModule);
- return Method;
+
+ bool isDirect = OMD->isDirectMethod();
+ std::string FunctionName =
+ getSymbolNameForMethod(OMD, /*include category*/ !isDirect);
+
+ if (!isDirect)
+ return llvm::Function::Create(MethodTy,
+ llvm::GlobalVariable::InternalLinkage,
+ FunctionName, &TheModule);
+
+ auto *COMD = OMD->getCanonicalDecl();
+ auto I = DirectMethodDefinitions.find(COMD);
+ llvm::Function *OldFn = nullptr, *Fn = nullptr;
+
+ if (I != DirectMethodDefinitions.end()) {
+ // Objective-C allows for the declaration and implementation types
+ // to differ slightly.
+ //
+ // If we're being asked for the Function associated for a method
+ // implementation, a previous value might have been cached
+ // based on the type of the canonical declaration.
+ //
+ // If these do not match, then we'll replace this function with
+ // a new one that has the proper type below.
+ if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType())
+ return I->second;
+ OldFn = I->second;
+ }
+
+ if (OldFn) {
+ Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+ "", &CGM.getModule());
+ Fn->takeName(OldFn);
+ OldFn->replaceAllUsesWith(Fn);
+ OldFn->eraseFromParent();
+
+ // Replace the cached function in the map.
+ I->second = Fn;
+ } else {
+ Fn = llvm::Function::Create(MethodTy, llvm::GlobalVariable::ExternalLinkage,
+ FunctionName, &TheModule);
+ DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+ }
+ return Fn;
}
void CGObjCGNU::GenerateDirectMethodPrologue(CodeGenFunction &CGF,
diff --git a/clang/test/CodeGenObjC/gnustep2-direct-method.m b/clang/test/CodeGenObjC/gnustep2-direct-method.m
new file mode 100644
index 00000000000000..662ff07f0d8b3d
--- /dev/null
+++ b/clang/test/CodeGenObjC/gnustep2-direct-method.m
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-2.2 -o - %s | FileCheck %s
+
+@interface X
+@end
+
+@implementation X
+//- (int)x __attribute__((objc_direct)) { return 12; }
+- (int)x __attribute__((objc_direct)) { return 12; }
+
+// Check that the name is mangled like Objective-C methods and contains a nil check
+// CHECK-LABEL: @_i_X__x
+// CHECK: icmp eq ptr %0, null
+
++ (int)clsMeth __attribute__((objc_direct)) { return 42; }
+// Check that the name is mangled like Objective-C methods and contains an initialisation check
+// CHECK-LABEL: @_c_X__clsMeth
+// CHECK: getelementptr inbounds { ptr, ptr, ptr, i64, i64 }, ptr %0, i32 0, i32 4
+// CHECK: load i64, ptr %1, align 64
+// CHECK: and i64 %2, 256
+// CHECK: objc_direct_method.send_initialize:
+// CHECK: call void @objc_send_initialize(ptr %0)
+
+@end
+
+// Check that the call sides are set up correctly.
+void callCls(void)
+{
+ // CHECK: call i32 @_c_X__clsMeth
+ [X clsMeth];
+}
+
+void callInstance(X *x)
+{
+ // CHECK: call i32 @_i_X__x
+ [x x];
+}
+
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
b286b9b
to
c8d7334
Compare
Formatting issues were introduced by running clang-format15, should now be fixed. |
clang/lib/CodeGen/CGObjCGNU.cpp
Outdated
Address{Builder.CreateStructGEP(classStart, selfValue, 4), LongTy, | ||
CharUnits::fromQuantity( | ||
astContext.getTypeAlign(astContext.UnsignedLongTy))}); | ||
auto isInitialized = Builder.CreateAnd(flags, (1 << 8)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be nice to extract the flags enumeration into named constants for re-use.
clang/lib/CodeGen/CGObjCGNU.cpp
Outdated
astContext.getTypeAlign(astContext.UnsignedLongTy))}); | ||
auto isInitialized = Builder.CreateAnd(flags, (1 << 8)); | ||
llvm::BasicBlock *notInitializedBlock = | ||
CGF.createBasicBlock("objc_direct_method.send_initialize"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call this objc_direct_method.class_uninitialized
. The BB will fall through to the class_initialized block, so while the block itself is only sending the initialization message, the path does more.
clang/lib/CodeGen/CGObjCGNU.cpp
Outdated
CGF.createBasicBlock("objc_direct_method.send_initialize"); | ||
llvm::BasicBlock *initializedBlock = | ||
CGF.createBasicBlock("objc_direct_method.class_initialized"); | ||
llvm::MDBuilder MDHelper(CGM.getLLVMContext()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps hoist the MDBuilder as you need it previously as well.
c8d7334
to
02123c6
Compare
Thanks, all review comments should now be resolved. |
These will be supported in the upcoming 2.2 release and so are gated on that version. Direct methods call `objc_send_initialize` if they are class methods that may not have called initialize. This is guarded by checking for the class flag bit that is set on initialisation in the class. This bit now forms part of the ABI, but it's been stable for 30+ years so that's fine as a contract going forwards.
02123c6
to
008814b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor nits.
@@ -463,7 +471,8 @@ class ObjCRuntime { | |||
case iOS: return true; | |||
case WatchOS: return true; | |||
case GCC: return false; | |||
case GNUstep: return false; | |||
case GNUstep: | |||
return (getVersion() >= VersionTuple(2, 2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parenthesis are unnecessary.
Builder.SetInsertPoint(SelfIsNilBlock); | ||
if (!retTy->isVoidType()) { | ||
CGF.EmitNullInitialization(CGF.ReturnValue, retTy); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary braces.
if (Method) | ||
cmd = GetSelector(CGF, Method); | ||
else | ||
cmd = GetSelector(CGF, Sel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably more compact as a ternary.
These will be supported in the upcoming 2.2 release and so are gated on that version.
Direct methods call
objc_send_initialize
if they are class methods that may not have called initialize. This is guarded by checking for the class flag bit that is set on initialisation in the class. This bit now forms part of the ABI, but it's been stable for 30+ years so that's fine as a contract going forwards.