[clang codegen] Add CreateRuntimeFunction overload that takes a clang type. #113506
+64
−22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
efriedma-quic
force-pushed
the
runtimefunction-clang-type
branch
from
852334c
to
bb58bdd
Compare
|
It would be great if this new method also called TargetLibraryInfo and set the extension attributes for any integer <=32 bits... Or are you maybe waiting with that as a separate patch? |
|
SetLLVMFunctionAttributes() should add the appropriate signext/zeroext attribute. At least, it appears to work in my testing. I'll add a regression test. |
Ah, I see. I ran this now on SystemZ and can confirm that this is now getting the proper extension attributes. A test might not even be needed, considering that this will (hopefully soon by default) be checked on SystemZ (general check in back-end), where it was discovered. This approach is making most sense to me as well now as there is already code that adds the attributes for clang types. |
|
What does it mean that this is a "draft"? It seems to me that this is ready for a regular review..? |
nikic
approved these changes
Nov 5, 2024
… type. Correctly computing the LLVM types/attributes is complicated in general, so add a variant which does that for you.
efriedma-quic
force-pushed
the
runtimefunction-clang-type
branch
from
bb58bdd
to
5d789d1
Compare
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Eli Friedman (efriedma-quic) ChangesCorrectly computing the LLVM types/attributes is complicated in general, so add a variant which does that for you. Full diff: https://github.com/llvm/llvm-project/pull/113506.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index bfa9b0a2f836bc8..1c46bac4bb232dd 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -2837,10 +2837,9 @@ llvm::FunctionCallee CodeGenModule::getBlockObjectDispose() {
if (BlockObjectDispose)
return BlockObjectDispose;
- llvm::Type *args[] = { Int8PtrTy, Int32Ty };
- llvm::FunctionType *fty
- = llvm::FunctionType::get(VoidTy, args, false);
- BlockObjectDispose = CreateRuntimeFunction(fty, "_Block_object_dispose");
+ QualType args[] = {Context.VoidPtrTy, Context.IntTy};
+ BlockObjectDispose =
+ CreateRuntimeFunction(Context.VoidTy, args, "_Block_object_dispose");
configureBlocksRuntimeObject(
*this, cast<llvm::Constant>(BlockObjectDispose.getCallee()));
return BlockObjectDispose;
@@ -2850,10 +2849,9 @@ llvm::FunctionCallee CodeGenModule::getBlockObjectAssign() {
if (BlockObjectAssign)
return BlockObjectAssign;
- llvm::Type *args[] = { Int8PtrTy, Int8PtrTy, Int32Ty };
- llvm::FunctionType *fty
- = llvm::FunctionType::get(VoidTy, args, false);
- BlockObjectAssign = CreateRuntimeFunction(fty, "_Block_object_assign");
+ QualType args[] = {Context.VoidPtrTy, Context.VoidPtrTy, Context.IntTy};
+ BlockObjectAssign =
+ CreateRuntimeFunction(Context.VoidTy, args, "_Block_object_assign");
configureBlocksRuntimeObject(
*this, cast<llvm::Constant>(BlockObjectAssign.getCallee()));
return BlockObjectAssign;
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index ba376f9ecfacde7..6a3af6890bae7de 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4903,6 +4903,52 @@ GetRuntimeFunctionDecl(ASTContext &C, StringRef Name) {
return nullptr;
}
+static void setWindowsItaniumDLLImport(CodeGenModule &CGM, bool Local,
+ llvm::Function *F, StringRef Name) {
+ // In Windows Itanium environments, try to mark runtime functions
+ // dllimport. For Mingw and MSVC, don't. We don't really know if the user
+ // will link their standard library statically or dynamically. Marking
+ // functions imported when they are not imported can cause linker errors
+ // and warnings.
+ if (!Local && CGM.getTriple().isWindowsItaniumEnvironment() &&
+ !CGM.getCodeGenOpts().LTOVisibilityPublicStd) {
+ const FunctionDecl *FD = GetRuntimeFunctionDecl(CGM.getContext(), Name);
+ if (!FD || FD->hasAttr<DLLImportAttr>()) {
+ F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+ F->setLinkage(llvm::GlobalValue::ExternalLinkage);
+ }
+ }
+}
+
+llvm::FunctionCallee CodeGenModule::CreateRuntimeFunction(
+ QualType ReturnTy, ArrayRef<QualType> ArgTys, StringRef Name,
+ llvm::AttributeList ExtraAttrs, bool Local, bool AssumeConvergent) {
+ if (AssumeConvergent) {
+ ExtraAttrs =
+ ExtraAttrs.addFnAttribute(VMContext, llvm::Attribute::Convergent);
+ }
+
+ QualType FTy = Context.getFunctionType(ReturnTy, ArgTys,
+ FunctionProtoType::ExtProtoInfo());
+ const CGFunctionInfo &Info = getTypes().arrangeFreeFunctionType(
+ Context.getCanonicalType(FTy).castAs<FunctionProtoType>());
+ auto *ConvTy = getTypes().GetFunctionType(Info);
+ llvm::Constant *C = GetOrCreateLLVMFunction(
+ Name, ConvTy, GlobalDecl(), /*ForVTable=*/false,
+ /*DontDefer=*/false, /*IsThunk=*/false, ExtraAttrs);
+
+ if (auto *F = dyn_cast<llvm::Function>(C)) {
+ if (F->empty()) {
+ SetLLVMFunctionAttributes(GlobalDecl(), Info, F, /*IsThunk*/ false);
+ // FIXME: Set calling-conv properly in ExtProtoInfo
+ F->setCallingConv(getRuntimeCC());
+ setWindowsItaniumDLLImport(*this, Local, F, Name);
+ setDSOLocal(F);
+ }
+ }
+ return {ConvTy, C};
+}
+
/// CreateRuntimeFunction - Create a new runtime function with the specified
/// type and name.
llvm::FunctionCallee
@@ -4922,20 +4968,7 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType *FTy, StringRef Name,
if (auto *F = dyn_cast<llvm::Function>(C)) {
if (F->empty()) {
F->setCallingConv(getRuntimeCC());
-
- // In Windows Itanium environments, try to mark runtime functions
- // dllimport. For Mingw and MSVC, don't. We don't really know if the user
- // will link their standard library statically or dynamically. Marking
- // functions imported when they are not imported can cause linker errors
- // and warnings.
- if (!Local && getTriple().isWindowsItaniumEnvironment() &&
- !getCodeGenOpts().LTOVisibilityPublicStd) {
- const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
- if (!FD || FD->hasAttr<DLLImportAttr>()) {
- F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
- F->setLinkage(llvm::GlobalValue::ExternalLinkage);
- }
- }
+ setWindowsItaniumDLLImport(*this, Local, F, Name);
setDSOLocal(F);
// FIXME: We should use CodeGenModule::SetLLVMFunctionAttributes() instead
// of trying to approximate the attributes using the LLVM function
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 8d5787769382f68..a0586af9ad3e27f 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1252,6 +1252,12 @@ class CodeGenModule : public CodeGenTypeCache {
llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
bool Local = false, bool AssumeConvergent = false);
+ llvm::FunctionCallee
+ CreateRuntimeFunction(QualType ReturnTy, ArrayRef<QualType> ArgTys,
+ StringRef Name,
+ llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
+ bool Local = false, bool AssumeConvergent = false);
+
/// Create a new runtime global variable with the specified type and name.
llvm::Constant *CreateRuntimeVariable(llvm::Type *Ty,
StringRef Name);
diff --git a/clang/test/CodeGen/blocks.c b/clang/test/CodeGen/blocks.c
index d508de9c9f80143..d104078a9816cce 100644
--- a/clang/test/CodeGen/blocks.c
+++ b/clang/test/CodeGen/blocks.c
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -triple i386-unknown-unknown %s -emit-llvm -Wno-strict-prototypes -o - -fblocks | FileCheck --check-prefix=CHECK --check-prefix=SIG_STR %s
// RUN: %clang_cc1 -triple i386-unknown-unknown %s -emit-llvm -Wno-strict-prototypes -o - -fblocks -fdisable-block-signature-string | FileCheck --check-prefix=CHECK --check-prefix=NO_SIG_STR %s
+// RUN: %clang_cc1 -triple s390x-unknown-unknown %s -emit-llvm -Wno-strict-prototypes -o - -fblocks | FileCheck --check-prefix=SYSTEMZ %s
// SIG_STR: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v4@?0\00", align 1
// SIG_STR: @{{.*}} = internal constant { ptr, i32, i32, ptr, ptr } { ptr @_NSConcreteGlobalBlock, i32 1342177280, i32 0, ptr @f_block_invoke, ptr @{{.*}} }, align 4
@@ -50,6 +51,8 @@ void (^test1)(void) = ^(void) {
// CHECK-NEXT: call void @_Block_object_assign(ptr %[[V5]], ptr %[[BLOCKCOPY_SRC]], i32 8)
// CHECK-NEXT: ret void
+// SYSTEMZ: declare void @_Block_object_assign(ptr noundef, ptr noundef, i32 noundef signext)
+
// CHECK-LABEL: define linkonce_odr hidden void @__destroy_helper_block_4_20r(ptr noundef %0) unnamed_addr
// CHECK: %[[_ADDR:.*]] = alloca ptr, align 4
// CHECK-NEXT: store ptr %0, ptr %[[_ADDR]], align 4
@@ -59,6 +62,8 @@ void (^test1)(void) = ^(void) {
// CHECK-NEXT: call void @_Block_object_dispose(ptr %[[V3]], i32 8)
// CHECK-NEXT: ret void
+// SYSTEMZ: declare void @_Block_object_dispose(ptr noundef, i32 noundef signext)
+
typedef double ftype(double);
// It's not clear that we *should* support this syntax, but until that decision
// is made, we should support it properly and not crash.
|
|
@llvm/pr-subscribers-clang-codegen Author: Eli Friedman (efriedma-quic) ChangesCorrectly computing the LLVM types/attributes is complicated in general, so add a variant which does that for you. Full diff: https://github.com/llvm/llvm-project/pull/113506.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index bfa9b0a2f836bc8..1c46bac4bb232dd 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -2837,10 +2837,9 @@ llvm::FunctionCallee CodeGenModule::getBlockObjectDispose() {
if (BlockObjectDispose)
return BlockObjectDispose;
- llvm::Type *args[] = { Int8PtrTy, Int32Ty };
- llvm::FunctionType *fty
- = llvm::FunctionType::get(VoidTy, args, false);
- BlockObjectDispose = CreateRuntimeFunction(fty, "_Block_object_dispose");
+ QualType args[] = {Context.VoidPtrTy, Context.IntTy};
+ BlockObjectDispose =
+ CreateRuntimeFunction(Context.VoidTy, args, "_Block_object_dispose");
configureBlocksRuntimeObject(
*this, cast<llvm::Constant>(BlockObjectDispose.getCallee()));
return BlockObjectDispose;
@@ -2850,10 +2849,9 @@ llvm::FunctionCallee CodeGenModule::getBlockObjectAssign() {
if (BlockObjectAssign)
return BlockObjectAssign;
- llvm::Type *args[] = { Int8PtrTy, Int8PtrTy, Int32Ty };
- llvm::FunctionType *fty
- = llvm::FunctionType::get(VoidTy, args, false);
- BlockObjectAssign = CreateRuntimeFunction(fty, "_Block_object_assign");
+ QualType args[] = {Context.VoidPtrTy, Context.VoidPtrTy, Context.IntTy};
+ BlockObjectAssign =
+ CreateRuntimeFunction(Context.VoidTy, args, "_Block_object_assign");
configureBlocksRuntimeObject(
*this, cast<llvm::Constant>(BlockObjectAssign.getCallee()));
return BlockObjectAssign;
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index ba376f9ecfacde7..6a3af6890bae7de 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4903,6 +4903,52 @@ GetRuntimeFunctionDecl(ASTContext &C, StringRef Name) {
return nullptr;
}
+static void setWindowsItaniumDLLImport(CodeGenModule &CGM, bool Local,
+ llvm::Function *F, StringRef Name) {
+ // In Windows Itanium environments, try to mark runtime functions
+ // dllimport. For Mingw and MSVC, don't. We don't really know if the user
+ // will link their standard library statically or dynamically. Marking
+ // functions imported when they are not imported can cause linker errors
+ // and warnings.
+ if (!Local && CGM.getTriple().isWindowsItaniumEnvironment() &&
+ !CGM.getCodeGenOpts().LTOVisibilityPublicStd) {
+ const FunctionDecl *FD = GetRuntimeFunctionDecl(CGM.getContext(), Name);
+ if (!FD || FD->hasAttr<DLLImportAttr>()) {
+ F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+ F->setLinkage(llvm::GlobalValue::ExternalLinkage);
+ }
+ }
+}
+
+llvm::FunctionCallee CodeGenModule::CreateRuntimeFunction(
+ QualType ReturnTy, ArrayRef<QualType> ArgTys, StringRef Name,
+ llvm::AttributeList ExtraAttrs, bool Local, bool AssumeConvergent) {
+ if (AssumeConvergent) {
+ ExtraAttrs =
+ ExtraAttrs.addFnAttribute(VMContext, llvm::Attribute::Convergent);
+ }
+
+ QualType FTy = Context.getFunctionType(ReturnTy, ArgTys,
+ FunctionProtoType::ExtProtoInfo());
+ const CGFunctionInfo &Info = getTypes().arrangeFreeFunctionType(
+ Context.getCanonicalType(FTy).castAs<FunctionProtoType>());
+ auto *ConvTy = getTypes().GetFunctionType(Info);
+ llvm::Constant *C = GetOrCreateLLVMFunction(
+ Name, ConvTy, GlobalDecl(), /*ForVTable=*/false,
+ /*DontDefer=*/false, /*IsThunk=*/false, ExtraAttrs);
+
+ if (auto *F = dyn_cast<llvm::Function>(C)) {
+ if (F->empty()) {
+ SetLLVMFunctionAttributes(GlobalDecl(), Info, F, /*IsThunk*/ false);
+ // FIXME: Set calling-conv properly in ExtProtoInfo
+ F->setCallingConv(getRuntimeCC());
+ setWindowsItaniumDLLImport(*this, Local, F, Name);
+ setDSOLocal(F);
+ }
+ }
+ return {ConvTy, C};
+}
+
/// CreateRuntimeFunction - Create a new runtime function with the specified
/// type and name.
llvm::FunctionCallee
@@ -4922,20 +4968,7 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType *FTy, StringRef Name,
if (auto *F = dyn_cast<llvm::Function>(C)) {
if (F->empty()) {
F->setCallingConv(getRuntimeCC());
-
- // In Windows Itanium environments, try to mark runtime functions
- // dllimport. For Mingw and MSVC, don't. We don't really know if the user
- // will link their standard library statically or dynamically. Marking
- // functions imported when they are not imported can cause linker errors
- // and warnings.
- if (!Local && getTriple().isWindowsItaniumEnvironment() &&
- !getCodeGenOpts().LTOVisibilityPublicStd) {
- const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
- if (!FD || FD->hasAttr<DLLImportAttr>()) {
- F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
- F->setLinkage(llvm::GlobalValue::ExternalLinkage);
- }
- }
+ setWindowsItaniumDLLImport(*this, Local, F, Name);
setDSOLocal(F);
// FIXME: We should use CodeGenModule::SetLLVMFunctionAttributes() instead
// of trying to approximate the attributes using the LLVM function
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 8d5787769382f68..a0586af9ad3e27f 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1252,6 +1252,12 @@ class CodeGenModule : public CodeGenTypeCache {
llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
bool Local = false, bool AssumeConvergent = false);
+ llvm::FunctionCallee
+ CreateRuntimeFunction(QualType ReturnTy, ArrayRef<QualType> ArgTys,
+ StringRef Name,
+ llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
+ bool Local = false, bool AssumeConvergent = false);
+
/// Create a new runtime global variable with the specified type and name.
llvm::Constant *CreateRuntimeVariable(llvm::Type *Ty,
StringRef Name);
diff --git a/clang/test/CodeGen/blocks.c b/clang/test/CodeGen/blocks.c
index d508de9c9f80143..d104078a9816cce 100644
--- a/clang/test/CodeGen/blocks.c
+++ b/clang/test/CodeGen/blocks.c
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -triple i386-unknown-unknown %s -emit-llvm -Wno-strict-prototypes -o - -fblocks | FileCheck --check-prefix=CHECK --check-prefix=SIG_STR %s
// RUN: %clang_cc1 -triple i386-unknown-unknown %s -emit-llvm -Wno-strict-prototypes -o - -fblocks -fdisable-block-signature-string | FileCheck --check-prefix=CHECK --check-prefix=NO_SIG_STR %s
+// RUN: %clang_cc1 -triple s390x-unknown-unknown %s -emit-llvm -Wno-strict-prototypes -o - -fblocks | FileCheck --check-prefix=SYSTEMZ %s
// SIG_STR: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v4@?0\00", align 1
// SIG_STR: @{{.*}} = internal constant { ptr, i32, i32, ptr, ptr } { ptr @_NSConcreteGlobalBlock, i32 1342177280, i32 0, ptr @f_block_invoke, ptr @{{.*}} }, align 4
@@ -50,6 +51,8 @@ void (^test1)(void) = ^(void) {
// CHECK-NEXT: call void @_Block_object_assign(ptr %[[V5]], ptr %[[BLOCKCOPY_SRC]], i32 8)
// CHECK-NEXT: ret void
+// SYSTEMZ: declare void @_Block_object_assign(ptr noundef, ptr noundef, i32 noundef signext)
+
// CHECK-LABEL: define linkonce_odr hidden void @__destroy_helper_block_4_20r(ptr noundef %0) unnamed_addr
// CHECK: %[[_ADDR:.*]] = alloca ptr, align 4
// CHECK-NEXT: store ptr %0, ptr %[[_ADDR]], align 4
@@ -59,6 +62,8 @@ void (^test1)(void) = ^(void) {
// CHECK-NEXT: call void @_Block_object_dispose(ptr %[[V3]], i32 8)
// CHECK-NEXT: ret void
+// SYSTEMZ: declare void @_Block_object_dispose(ptr noundef, i32 noundef signext)
+
typedef double ftype(double);
// It's not clear that we *should* support this syntax, but until that decision
// is made, we should support it properly and not crash.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Correctly computing the LLVM types/attributes is complicated in general, so add a variant which does that for you.