Skip to content

[CodeGen] Create IFUNCs in the program address space, not hard-coded 0 #105726

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

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

jrtc27
Copy link
Collaborator

@jrtc27 jrtc27 commented Aug 22, 2024

Commit 0d527e5 ("GlobalIFunc: Make ifunc respect function address
spaces") added support for this within LLVM, but Clang does not properly
honour the target's address spaces when creating IFUNCs, crashing with
RAUW and verifier assertion failures when compiling C code on a target
with a non-zero program address space, so fix this.

@jrtc27 jrtc27 requested a review from arsenm August 22, 2024 20:24
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Aug 22, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 22, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Jessica Clarke (jrtc27)

Changes

Commit 0d527e5 ("GlobalIFunc: Make ifunc respect function address
spaces") added support for this within LLVM, but Clang does not properly
honour the target's address spaces when creating IFUNCs, crashing with
RAUW and verifier assertion failures when compiling C code on a target
with a non-zero program address space, so fix this.


Full diff: https://github.com/llvm/llvm-project/pull/105726.diff

2 Files Affected:

  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+9-7)
  • (modified) clang/test/CodeGen/ifunc.c (+5)
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 6d11bd17d0a5f5..ce681dd878bc7e 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4443,11 +4443,12 @@ void CodeGenModule::emitCPUDispatchDefinition(GlobalDecl GD) {
   if (getTarget().supportsIFunc()) {
     llvm::GlobalValue::LinkageTypes Linkage = getMultiversionLinkage(*this, GD);
     auto *IFunc = cast<llvm::GlobalValue>(GetOrCreateMultiVersionResolver(GD));
+    unsigned AS = IFunc->getType()->getPointerAddressSpace();
 
     // Fix up function declarations that were created for cpu_specific before
     // cpu_dispatch was known
     if (!isa<llvm::GlobalIFunc>(IFunc)) {
-      auto *GI = llvm::GlobalIFunc::create(DeclTy, 0, Linkage, "", ResolverFunc,
+      auto *GI = llvm::GlobalIFunc::create(DeclTy, AS, Linkage, "", ResolverFunc,
                                            &getModule());
       replaceDeclarationWith(IFunc, GI);
       IFunc = GI;
@@ -4457,8 +4458,8 @@ void CodeGenModule::emitCPUDispatchDefinition(GlobalDecl GD) {
         *this, GD, FD, /*OmitMultiVersionMangling=*/true);
     llvm::Constant *AliasFunc = GetGlobalValue(AliasName);
     if (!AliasFunc) {
-      auto *GA = llvm::GlobalAlias::create(DeclTy, 0, Linkage, AliasName, IFunc,
-                                           &getModule());
+      auto *GA = llvm::GlobalAlias::create(DeclTy, AS, Linkage, AliasName,
+                                           IFunc, &getModule());
       SetCommonAttributes(GD, GA);
     }
   }
@@ -4530,15 +4531,15 @@ llvm::Constant *CodeGenModule::GetOrCreateMultiVersionResolver(GlobalDecl GD) {
   // For cpu_specific, don't create an ifunc yet because we don't know if the
   // cpu_dispatch will be emitted in this translation unit.
   if (getTarget().supportsIFunc() && !FD->isCPUSpecificMultiVersion()) {
+    unsigned AS = getTypes().getTargetAddressSpace(FD->getType());
     llvm::Type *ResolverType = llvm::FunctionType::get(
-        llvm::PointerType::get(DeclTy,
-                               getTypes().getTargetAddressSpace(FD->getType())),
+        llvm::PointerType::get(DeclTy, AS),
         false);
     llvm::Constant *Resolver = GetOrCreateLLVMFunction(
         MangledName + ".resolver", ResolverType, GlobalDecl{},
         /*ForVTable=*/false);
     llvm::GlobalIFunc *GIF =
-        llvm::GlobalIFunc::create(DeclTy, 0, getMultiversionLinkage(*this, GD),
+        llvm::GlobalIFunc::create(DeclTy, AS, getMultiversionLinkage(*this, GD),
                                   "", Resolver, &getModule());
     GIF->setName(ResolverName);
     SetCommonAttributes(FD, GIF);
@@ -6144,8 +6145,9 @@ void CodeGenModule::emitIFuncDefinition(GlobalDecl GD) {
       GetOrCreateLLVMFunction(IFA->getResolver(), VoidTy, {},
                               /*ForVTable=*/false);
   llvm::Type *DeclTy = getTypes().ConvertTypeForMem(D->getType());
+  unsigned AS = getTypes().getTargetAddressSpace(D->getType());
   llvm::GlobalIFunc *GIF =
-      llvm::GlobalIFunc::create(DeclTy, 0, llvm::Function::ExternalLinkage,
+      llvm::GlobalIFunc::create(DeclTy, AS, llvm::Function::ExternalLinkage,
                                 "", Resolver, &getModule());
   if (Entry) {
     if (GIF->getResolver() == Entry) {
diff --git a/clang/test/CodeGen/ifunc.c b/clang/test/CodeGen/ifunc.c
index 58a00ada687cb0..2849246f93dc3b 100644
--- a/clang/test/CodeGen/ifunc.c
+++ b/clang/test/CodeGen/ifunc.c
@@ -11,6 +11,7 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx -fsanitize=thread -O2 -emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
 // RUN: %clang_cc1 -triple arm64-apple-macosx -fsanitize=address -O2 -emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
 // RUN: %clang_cc1 -triple x86_64-apple-macosx -fsanitize=address -O2 -emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
+// RUN: %clang_cc1 -triple avr-unknown-unknown -emit-llvm -o - %s | FileCheck %s --check-prefix=AVR
 
 /// The ifunc is emitted before its resolver.
 int foo(int) __attribute__ ((ifunc("foo_ifunc")));
@@ -55,6 +56,10 @@ extern void hoo(int) __attribute__ ((ifunc("hoo_ifunc")));
 // CHECK: @goo = ifunc void (), ptr @goo_ifunc
 // CHECK: @hoo = ifunc void (i32), ptr @hoo_ifunc
 
+// AVR: @foo = ifunc i16 (i16), ptr addrspace(1) @foo_ifunc
+// AVR: @goo = ifunc void (), ptr addrspace(1) @goo_ifunc
+// AVR: @hoo = ifunc void (i16), ptr addrspace(1) @hoo_ifunc
+
 // CHECK: call i32 @foo(i32
 // CHECK: call void @goo()
 

@jrtc27 jrtc27 requested a review from arichardson August 22, 2024 20:27
Copy link

github-actions bot commented Aug 22, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Commit 0d527e5 ("GlobalIFunc: Make ifunc respect function address
spaces") added support for this within LLVM, but Clang does not properly
honour the target's address spaces when creating IFUNCs, crashing with
RAUW and verifier assertion failures when compiling C code on a target
with a non-zero program address space, so fix this.
@jrtc27 jrtc27 force-pushed the clang-ifunc-addrspace branch from 4be2b44 to cfce8b0 Compare August 22, 2024 20:30
@jrtc27
Copy link
Collaborator Author

jrtc27 commented Aug 28, 2024

NB: I would appreciate a review from someone outside the CHERI project

@jrtc27 jrtc27 merged commit 73e0aa5 into llvm:main Aug 28, 2024
8 checks passed
@jrtc27 jrtc27 deleted the clang-ifunc-addrspace branch August 28, 2024 16:11
qiaojbao pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Sep 30, 2024
…f6fcf29c6

Local branch amd-gfx e64f6fc Merged main:fee48365b44d161575314c67328fa21104c4537d into amd-gfx:0447d312fac8
Remote branch main 73e0aa5 [CodeGen] Create IFUNCs in the program address space, not hard-coded 0 (llvm#105726)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants