Skip to content

[clang] Function Multi Versioning supports IFunc lowerings on Darwin platforms #73688

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

Conversation

jroelofs
Copy link
Contributor

@jroelofs jroelofs commented Nov 28, 2023

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 28, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 28, 2023

@llvm/pr-subscribers-clang

Author: Jon Roelofs (jroelofs)

Changes

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

2 Files Affected:

  • (modified) clang/include/clang/Basic/TargetInfo.h (+2)
  • (modified) clang/test/CodeGen/attr-target-mv-va-args.c (+19)
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 41f3c2e403cbef6..1fe2a18cd5dc9cc 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1424,6 +1424,8 @@ class TargetInfo : public TransferrableTargetInfo,
 
   /// Identify whether this target supports IFuncs.
   bool supportsIFunc() const {
+    if (getTriple().isOSBinFormatMachO())
+      return true;
     return getTriple().isOSBinFormatELF() &&
            ((getTriple().isOSLinux() && !getTriple().isMusl()) ||
             getTriple().isOSFreeBSD());
diff --git a/clang/test/CodeGen/attr-target-mv-va-args.c b/clang/test/CodeGen/attr-target-mv-va-args.c
index 96821c610235bdc..dbf5a74205c4c19 100644
--- a/clang/test/CodeGen/attr-target-mv-va-args.c
+++ b/clang/test/CodeGen/attr-target-mv-va-args.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -triple x86_64-windows-pc -emit-llvm %s -o - | FileCheck %s --check-prefixes=NO-IFUNC,WINDOWS
 // RUN: %clang_cc1 -triple x86_64-linux-musl -emit-llvm %s -o - | FileCheck %s --check-prefixes=NO-IFUNC,NO-IFUNC-ELF
 // RUN: %clang_cc1 -triple x86_64-fuchsia -emit-llvm %s -o - | FileCheck %s --check-prefixes=NO-IFUNC,NO-IFUNC-ELF
+// RUN: %clang_cc1 -triple x86_64-apple-macho -emit-llvm %s -o - | FileCheck %s --check-prefix=IFUNC-MACHO
 int __attribute__((target("sse4.2"))) foo(int i, ...) { return 0; }
 int __attribute__((target("arch=sandybridge"))) foo(int i, ...);
 int __attribute__((target("arch=ivybridge"))) foo(int i, ...) {return 1;}
@@ -30,6 +31,24 @@ int bar(void) {
 // IFUNC-ELF: ret ptr @foo
 // IFUNC-ELF: declare i32 @foo.arch_sandybridge(i32 noundef, ...)
 
+// IFUNC-MACHO: @foo.ifunc = weak_odr ifunc i32 (i32, ...), ptr @foo.resolver
+// IFUNC-MACHO: define{{.*}} i32 @foo.sse4.2(i32 noundef %i, ...)
+// IFUNC-MACHO: ret i32 0
+// IFUNC-MACHO: define{{.*}} i32 @foo.arch_ivybridge(i32 noundef %i, ...)
+// IFUNC-MACHO: ret i32 1
+// IFUNC-MACHO: define{{.*}} i32 @foo(i32 noundef %i, ...)
+// IFUNC-MACHO: ret i32 2
+// IFUNC-MACHO: define{{.*}} i32 @bar()
+// IFUNC-MACHO: call i32 (i32, ...) @foo.ifunc(i32 noundef 1, i32 noundef 97, double
+// IFUNC-MACHO: call i32 (i32, ...) @foo.ifunc(i32 noundef 2, double noundef 2.2{{[0-9Ee+]+}}, ptr noundef
+
+// IFUNC-MACHO: define weak_odr ptr @foo.resolver()
+// IFUNC-MACHO: ret ptr @foo.arch_sandybridge
+// IFUNC-MACHO: ret ptr @foo.arch_ivybridge
+// IFUNC-MACHO: ret ptr @foo.sse4.2
+// IFUNC-MACHO: ret ptr @foo
+// IFUNC-MACHO: declare i32 @foo.arch_sandybridge(i32 noundef, ...)
+
 // NO-IFUNC: define dso_local i32 @foo.sse4.2(i32 noundef %i, ...)
 // NO-IFUNC: ret i32 0
 // NO-IFUNC: define dso_local i32 @foo.arch_ivybridge(i32 noundef %i, ...)

@jroelofs jroelofs requested review from MaskRay and ilinpv November 28, 2023 18:52
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@tahonermann
Copy link
Contributor

I recommend extending testing via additional RUN lines to all of the following tests:

  • clang/test/CodeGen/attr-cpuspecific.c
  • clang/test/CodeGen/attr-target-clones.c
  • clang/test/CodeGen/attr-target-mv-func-ptrs.c
  • clang/test/CodeGen/attr-target-mv-va-args.c (already done)
  • clang/test/CodeGen/attr-target-mv.c
  • clang/test/CodeGenCXX/attr-cpuspecific.cpp
  • clang/test/CodeGenCXX/attr-target-mv-diff-ns.cpp
  • clang/test/CodeGenCXX/attr-target-mv-inalloca.cpp
  • clang/test/CodeGenCXX/attr-target-mv-member-funcs.cpp
  • clang/test/CodeGenCXX/attr-target-mv-modules.cpp
  • clang/test/CodeGenCXX/attr-target-mv-out-of-line-defs.cpp
  • clang/test/CodeGenCXX/attr-target-mv-overloads.cpp
  • clang/test/CodeGenCXX/attr-target-clones.cpp

Note that, for many of the above tests, changes planned in #71706 will impact the expected behavior.

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@tahonermann
Copy link
Contributor

Added @erichkeane as a reviewer as well.

Will this cause an ABI compatibility issue for any existing use of the function multiversioning attributes on Darwin? I think it might, but I haven't tried to confirm.

@jroelofs
Copy link
Contributor Author

jroelofs commented Nov 29, 2023

Will this cause an ABI compatibility issue for any existing use of the function multiversioning attributes on Darwin? I think it might, but I haven't tried to confirm.

I don't think there is an ABI compatibility concern here for a couple of reasons:

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR just updates supportsIFunc and extends clang tests. LGTM

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Copy link
Contributor

@ilinpv ilinpv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s --check-prefix=LINUX
// RUN: %clang_cc1 -triple x86_64-apple-macosx -emit-llvm -o - %s | FileCheck %s --check-prefix=LINUX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "macosx" a deprecated OS name?

Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@jroelofs jroelofs changed the base branch from users/jroelofs/spr/main.clang-function-multi-versioning-supports-ifunc-lowerings-on-darwin-platforms to main December 14, 2023 21:55
Created using spr 1.3.4

[skip ci]
Created using spr 1.3.4
@jroelofs jroelofs merged commit 6c12fd9 into main Dec 14, 2023
@jroelofs jroelofs deleted the users/jroelofs/spr/clang-function-multi-versioning-supports-ifunc-lowerings-on-darwin-platforms branch December 14, 2023 21:57
jroelofs added a commit to swiftlang/llvm-project that referenced this pull request Jan 10, 2024
jroelofs added a commit to swiftlang/llvm-project that referenced this pull request Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants