[X86] Support APXF to enable __builtin_cpu_supports. #80636

FreddyLeaf · 2024-02-05T06:11:26Z

For referring, APX's spec: https://cdrdv2.intel.com/v1/dl/getContent/784266
APX's index in libgcc: https://github.com/gcc-mirror/gcc/blob/master/gcc/common/config/i386/i386-cpuinfo.h#L267

llvmbot · 2024-02-05T06:11:54Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: Freddy Ye (FreddyLeaf)

Changes

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

5 Files Affected:

(modified) clang/test/CodeGen/target-builtin-noerror.c (+1)
(modified) compiler-rt/lib/builtins/cpu_model/x86.c (+3-1)
(modified) llvm/include/llvm/TargetParser/X86TargetParser.def (+2-1)
(modified) llvm/lib/TargetParser/Host.cpp (+1)
(modified) llvm/lib/TargetParser/X86TargetParser.cpp (+1)

diff --git a/clang/test/CodeGen/target-builtin-noerror.c b/clang/test/CodeGen/target-builtin-noerror.c
index 9608b5f37baaa..b438e50848a4b 100644
--- a/clang/test/CodeGen/target-builtin-noerror.c
+++ b/clang/test/CodeGen/target-builtin-noerror.c
@@ -141,6 +141,7 @@ void verifyfeaturestrings(void) {
   (void)__builtin_cpu_supports("sm3");
   (void)__builtin_cpu_supports("sha512");
   (void)__builtin_cpu_supports("sm4");
+  (void)__builtin_cpu_supports("apxf");
   (void)__builtin_cpu_supports("usermsr");
   (void)__builtin_cpu_supports("avx10.1-256");
   (void)__builtin_cpu_supports("avx10.1-512");
diff --git a/compiler-rt/lib/builtins/cpu_model/x86.c b/compiler-rt/lib/builtins/cpu_model/x86.c
index 1afa468c4ae8c..35375c6e8d55b 100644
--- a/compiler-rt/lib/builtins/cpu_model/x86.c
+++ b/compiler-rt/lib/builtins/cpu_model/x86.c
@@ -217,7 +217,7 @@ enum ProcessorFeatures {
   FEATURE_SM3,
   FEATURE_SHA512,
   FEATURE_SM4,
-  // FEATURE_APXF,
+  FEATURE_APXF,
   FEATURE_USERMSR = 112,
   FEATURE_AVX10_1_256,
   FEATURE_AVX10_1_512,
@@ -983,6 +983,8 @@ static void getAvailableFeatures(unsigned ECX, unsigned EDX, unsigned MaxLeaf,
     setFeature(FEATURE_USERMSR);
   if (HasLeaf7Subleaf1 && ((EDX >> 19) & 1))
     setFeature(FEATURE_AVX10_1_256);
+  if (HasLeaf7Subleaf1 && ((EDX >> 21) & 1))
+    setFeature(FEATURE_APXF);
 
   unsigned MaxLevel;
   getX86CpuIDAndInfo(0, &MaxLevel, &EBX, &ECX, &EDX);
diff --git a/llvm/include/llvm/TargetParser/X86TargetParser.def b/llvm/include/llvm/TargetParser/X86TargetParser.def
index 4c630c1eb06e8..ec52062a2baac 100644
--- a/llvm/include/llvm/TargetParser/X86TargetParser.def
+++ b/llvm/include/llvm/TargetParser/X86TargetParser.def
@@ -248,10 +248,11 @@ X86_FEATURE_COMPAT(AVXVNNIINT16,    "avxvnniint16",           0)
 X86_FEATURE_COMPAT(SM3,             "sm3",                    0)
 X86_FEATURE_COMPAT(SHA512,          "sha512",                 0)
 X86_FEATURE_COMPAT(SM4,             "sm4",                    0)
-X86_FEATURE       (EGPR,            "egpr")
+X86_FEATURE_COMPAT(APXF,            "apxf",                   0)
 X86_FEATURE_COMPAT(USERMSR,         "usermsr",                0)
 X86_FEATURE_COMPAT(AVX10_1,         "avx10.1-256",            0)
 X86_FEATURE_COMPAT(AVX10_1_512,     "avx10.1-512",            0)
+X86_FEATURE       (EGPR,            "egpr")
 X86_FEATURE       (EVEX512,         "evex512")
 X86_FEATURE       (CF,              "cf")
 // These features aren't really CPU features, but the frontend can set them.
diff --git a/llvm/lib/TargetParser/Host.cpp b/llvm/lib/TargetParser/Host.cpp
index f1197c2965538..233ee12a00096 100644
--- a/llvm/lib/TargetParser/Host.cpp
+++ b/llvm/lib/TargetParser/Host.cpp
@@ -1845,6 +1845,7 @@ bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
   Features["prefetchi"]  = HasLeaf7Subleaf1 && ((EDX >> 14) & 1);
   Features["usermsr"]  = HasLeaf7Subleaf1 && ((EDX >> 15) & 1);
   Features["avx10.1-256"] = HasLeaf7Subleaf1 && ((EDX >> 19) & 1);
+  Features["apxf"] = HasLeaf7Subleaf1 && ((EDX >> 21) & 1);
 
   bool HasLeafD = MaxLevel >= 0xd &&
                   !getX86CpuIDAndInfoEx(0xd, 0x1, &EAX, &EBX, &ECX, &EDX);
diff --git a/llvm/lib/TargetParser/X86TargetParser.cpp b/llvm/lib/TargetParser/X86TargetParser.cpp
index 21f46f576490a..ea1f8517bb332 100644
--- a/llvm/lib/TargetParser/X86TargetParser.cpp
+++ b/llvm/lib/TargetParser/X86TargetParser.cpp
@@ -633,6 +633,7 @@ constexpr FeatureBitset ImpliedFeaturesPPX = {};
 constexpr FeatureBitset ImpliedFeaturesNDD = {};
 constexpr FeatureBitset ImpliedFeaturesCCMP = {};
 constexpr FeatureBitset ImpliedFeaturesCF = {};
+constexpr FeatureBitset ImpliedFeaturesAPXF = {};
 
 constexpr FeatureInfo FeatureInfos[X86::CPU_FEATURE_MAX] = {
 #define X86_FEATURE(ENUM, STR) {{"+" STR}, ImpliedFeatures##ENUM},

llvm/include/llvm/TargetParser/X86TargetParser.def

github-actions · 2024-02-06T09:27:09Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff c5e5661591a90094eeb5831de86d701419c74f07 cbdc67dcbe128f7ca89005930dc3b490f6eca567 -- clang/lib/Headers/cpuid.h clang/test/CodeGen/target-builtin-noerror.c compiler-rt/lib/builtins/cpu_model/x86.c llvm/lib/TargetParser/Host.cpp

View the diff from clang-format here.

diff --git a/clang/lib/Headers/cpuid.h b/clang/lib/Headers/cpuid.h
index 0bb9912b46..828e028af9 100644
--- a/clang/lib/Headers/cpuid.h
+++ b/clang/lib/Headers/cpuid.h
@@ -219,7 +219,7 @@
 #define bit_PREFETCHI     0x00004000
 #define bit_USERMSR       0x00008000
 #define bit_AVX10         0x00080000
-#define bit_APXF          0x00200000
+#define bit_APXF 0x00200000
 
 /* Features in %eax for leaf 13 sub-leaf 1 */
 #define bit_XSAVEOPT    0x00000001

FreddyLeaf · 2024-02-07T03:50:15Z

ping for review

phoebewang · 2024-02-07T06:20:19Z

llvm/lib/TargetParser/Host.cpp

@@ -1845,6 +1845,7 @@ bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
  Features["prefetchi"]  = HasLeaf7Subleaf1 && ((EDX >> 14) & 1);
  Features["usermsr"]  = HasLeaf7Subleaf1 && ((EDX >> 15) & 1);
  Features["avx10.1-256"] = HasLeaf7Subleaf1 && ((EDX >> 19) & 1);
+  Features["apxf"] = HasLeaf7Subleaf1 && ((EDX >> 21) & 1);


I don't think we have a apxf feature in LLVM, should it be egpr?

This patch defined one in clang through llvm/include/llvm/TargetParser/X86TargetParser.def. I think it's ok to do so.

My understanding is "apxf" here is used by front-end for FMV, right?
What would happen if there is "apxf" in the IR after this patch?

My understanding is "apxf" here is used by front-end for FMV, right?

Yes, not only FMV, but also __builtin_cpu_supports.

What would happen if there is "apxf" in the IR after this patch?

if llc read "apxf" in attribute, I think a warning will be given like:

'+apxf' is not a recognized feature for this target (ignoring feature)

But such attribute won't be generated from frontend, because we didn't add logics for "apxf" in X86TargetInfo::hasFeature.

if llc read "apxf" in attribute, I think a warning will be given like:

'+apxf' is not a recognized feature for this target (ignoring feature)

Did you check the behavior w/ this patch?

Emm, I happened to find an issue with this patch.

$ clang -march=native foo.c -S -emit-llvm '-apxf' is not a recognized feature for this target (ignoring feature)

It turns out even not adding logics in X86TargetInfo::hasFeature it will still return false for "apxf". I also found "-egpr" is not generated. How did you handle this?

I think you cannot actually enable APX features when using __attribute__((__target__("apxf"))) either.

Yeah, that's another problem. For attribute/target, it seems more reasonable to enable as "egpr" instead of "apxf" because apx needs such fine-grained enabling. We need to discuss with gcc on this.

FreddyLeaf · 2024-02-22T08:26:40Z

After fa42b33

An error will be thrown out if compiling __builtin_cpu_supports("egpr")
attribute((target("apxf"))) is not supported in this patch, no matter amend features or FMV.
attribute((target("egpr,cf"))) is already supported before this PR.

I think 3rd can be a workaround of 2nd, and gcc also hasn't supported FMV for "apxf".

llvm/lib/TargetParser/Host.cpp

phoebewang · 2024-02-22T08:31:52Z

You may also need to transfer "apxf" feature into subfeatures here https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/X86.cpp#L106

FreddyLeaf · 2024-02-22T08:35:53Z

You may also need to transfer "apxf" feature into subfeatures here https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/X86.cpp#L106

Thanks for pointing out! This looks like the change required by supporting attribute((target("apxf"))). While that supports require other relate changes such as adding supports for apxf in X86TargetInfo::isValidFeatureName. I prefer to do it in another PR.

compiler-rt/lib/builtins/cpu_model/x86.c

llvm/include/llvm/TargetParser/X86TargetParser.def

clang/lib/Headers/cpuid.h

FreddyLeaf · 2024-02-23T06:39:24Z

ping

KanRobert

LGTM

FreddyLeaf · 2024-02-23T07:18:22Z

Thank you both for all of the review!

This is a follow-up for #80636

[X86] Support APXF to enable __builtin_cpu_supports.

b131b09

FreddyLeaf requested review from KanRobert and phoebewang and removed request for KanRobert February 5, 2024 06:11

llvmbot added clang Clang issues not falling into any other category compiler-rt backend:X86 compiler-rt:builtins labels Feb 5, 2024

FreddyLeaf requested a review from KanRobert February 5, 2024 06:11

KanRobert reviewed Feb 6, 2024

View reviewed changes

llvm/include/llvm/TargetParser/X86TargetParser.def Outdated Show resolved Hide resolved

FreddyLeaf added 2 commits February 6, 2024 17:19

misc

a1ecdf5

cpuid.h

021c541

llvmbot added the clang:headers Headers provided by Clang, e.g. for intrinsics label Feb 6, 2024

phoebewang reviewed Feb 7, 2024

View reviewed changes

FreddyLeaf added 2 commits February 7, 2024 14:36

Merge branch 'main' into apxf_builtincpu

4b2441f

Utilize X86_MICROARCH_LEVEL

fa42b33

phoebewang reviewed Feb 22, 2024

View reviewed changes

llvm/lib/TargetParser/Host.cpp Outdated Show resolved Hide resolved

Address comments.

fd73a2a

KanRobert reviewed Feb 22, 2024

View reviewed changes

compiler-rt/lib/builtins/cpu_model/x86.c Show resolved Hide resolved

KanRobert reviewed Feb 22, 2024

View reviewed changes

llvm/include/llvm/TargetParser/X86TargetParser.def Show resolved Hide resolved

KanRobert reviewed Feb 22, 2024

View reviewed changes

clang/lib/Headers/cpuid.h Show resolved Hide resolved

typo

cbdc67d

KanRobert approved these changes Feb 23, 2024

View reviewed changes

FreddyLeaf merged commit 1fe6be8 into llvm:main Feb 23, 2024

FreddyLeaf deleted the apxf_builtincpu branch February 23, 2024 07:18

KanRobert added a commit that referenced this pull request Jul 30, 2024

[X86] Update sub-features of APX for host CPU

95e9aff

This is a follow-up for #80636

[X86] Support APXF to enable __builtin_cpu_supports. #80636

[X86] Support APXF to enable __builtin_cpu_supports. #80636

Uh oh!

Conversation

FreddyLeaf commented Feb 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FreddyLeaf commented Feb 7, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FreddyLeaf commented Feb 22, 2024

Uh oh!

Uh oh!

phoebewang commented Feb 22, 2024

Uh oh!

FreddyLeaf commented Feb 22, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FreddyLeaf commented Feb 23, 2024

Uh oh!

KanRobert left a comment

Choose a reason for hiding this comment

Uh oh!

FreddyLeaf commented Feb 23, 2024

Uh oh!

Uh oh!

FreddyLeaf commented Feb 5, 2024 •

edited

Loading

llvmbot commented Feb 5, 2024 •

edited

Loading

github-actions bot commented Feb 6, 2024 •

edited

Loading