clang: __HAVE_FUNCTION_MULTIVERSIONING is mis-reported in previously-shipped compilers

[jroelofs]

Clang's FMV support implicitly depends on support for ifuncs, which until recently [1, 2] weren't supported on Darwin platforms. This means that previously-shipped compilers report that they support it via the pre-defined macro, even when the feature does not work.

1: #73686
2: https://github.com/llvm/llvm-project/pull/73688/files#diff-7930fda388572c1b90c30151343a92d058fd44508c32406a6a955bec9e83ce82R1426

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

Author: Jon Roelofs (jroelofs)

Clang's FMV support implicitly depends on support for ifuncs, which until recently [1, 2] weren't supported on Darwin platforms. This means that previously-shipped compilers report that they support it via the pre-defined macro, even when the feature does not work.

1: #73686
2: https://github.com/llvm/llvm-project/pull/73688/files#diff-7930fda388572c1b90c30151343a92d058fd44508c32406a6a955bec9e83ce82R1426

[jroelofs]

@ilinpv what do you think about changing the definition of this pre-define to this:

diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index d47181bfca4f..6db927e46730 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -440,7 +440,7 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
     Builder.defineMacro("__ARM_FEATURE_RCPC", "1");
 
   if (HasFMV)
-    Builder.defineMacro("__HAVE_FUNCTION_MULTI_VERSIONING", "1");
+    Builder.defineMacro("__HAVE_FUNCTION_MULTI_VERSIONING", supportsIFunc() ? "3" : "1");
 
   // The __ARM_FEATURE_CRYPTO is deprecated in favor of finer grained feature
   // macros for AES, SHA2, SHA3 and SM4

I know the ACLE says this macro should be 1 when the feature works, but the presence of compilers in the wild that claim to but don't actually support it means we don't have a great way to gate usage of this feature via the preprocessor.

[jroelofs]

or:

diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index d47181bfca4f..bc8a9e09ffaf 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -439,8 +439,8 @@ void AArch64TargetInfo::getTargetDefines(const LangOptions &Opts,
   else if (HasRCPC)
     Builder.defineMacro("__ARM_FEATURE_RCPC", "1");
 
-  if (HasFMV)
-    Builder.defineMacro("__HAVE_FUNCTION_MULTI_VERSIONING", "1");
+  if (HasFMV && supportsIFunc())
+    Builder.defineMacro("__HAVE_FUNCTION_MULTI_VERSIONING", "3");
 
   // The __ARM_FEATURE_CRYPTO is deprecated in favor of finer grained feature
   // macros for AES, SHA2, SHA3 and SM4

[jroelofs]

Here's an example demoing this being mis-reported in 17.0.1 and 16.0.0, but working on trunk, for a Darwin target:

https://clang.godbolt.org/z/KaT8er7MT

[jroelofs]

cc @labrinea @DanielKristofKiss

[DanielKristofKiss]

From the spec point of view maybe we could change the __HAVE_FUNCTION_MULTI_VERSIONING to report the ACLE version{1].
and with ARM-software/acle#297
Such could be written:

#if __HAVE_FUNCTION_MULTI_VERSIONING >= __ARM_ACLE_VERSION(2024, 1, 0 )

[1] ARM-software/acle#294

[jroelofs]

I like that idea.