[X86][AVX10] Disable m[no-]avx10.1 and switch m[no-]avx10.2 to alias of 512 bit options

[phoebewang]

Per the feedback we got, we’d like to switch m[no-]avx10.2 to alias of 512 bit options and disable m[no-]avx10.1 due to they were alias of 256 bit options.

We also change -mno-avx10.[1,2]-512 to alias of 256 bit options to disable both 256 and 512 instructions.

[llvmbot]

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Phoebe Wang (phoebewang)

Changes

Per the feedback we got, we’d like to switch m[no-]avx10.2 to alias of 512 bit options and disable m[no-]avx10.1 due to they were alias of 256 bit options.

We also change -mno-avx10.[1,2]-512 to alias of 256 bit options to disable both 256 and 512 instructions.

---

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

3 Files Affected:

• (modified) clang/include/clang/Driver/Options.td (+5-5)
• (modified) clang/test/Driver/x86-target-features.c (+9-6)
• (modified) clang/test/Preprocessor/x86_target_features.c (+1-2)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index c5b7fcb7c7f09b..c55e242d47dbfd 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6424,14 +6424,14 @@ def mno_avx : Flag<["-"], "mno-avx">, Group<m_x86_Features_Group>;
 def mavx10_1_256 : Flag<["-"], "mavx10.1-256">, Group<m_x86_AVX10_Features_Group>;
 def mno_avx10_1_256 : Flag<["-"], "mno-avx10.1-256">, Group<m_x86_AVX10_Features_Group>;
 def mavx10_1_512 : Flag<["-"], "mavx10.1-512">, Group<m_x86_AVX10_Features_Group>;
-def mno_avx10_1_512 : Flag<["-"], "mno-avx10.1-512">, Group<m_x86_AVX10_Features_Group>;
-def mavx10_1 : Flag<["-"], "mavx10.1">, Alias<mavx10_1_256>;
-def mno_avx10_1 : Flag<["-"], "mno-avx10.1">, Alias<mno_avx10_1_256>;
+def mno_avx10_1_512 : Flag<["-"], "mno-avx10.1-512">, Alias<mno_avx10_1_256>;
+def mavx10_1 : Flag<["-"], "mavx10.1">, Flags<[Unsupported]>;
+def mno_avx10_1 : Flag<["-"], "mno-avx10.1">, Flags<[Unsupported]>;
 def mavx10_2_256 : Flag<["-"], "mavx10.2-256">, Group<m_x86_AVX10_Features_Group>;
 def mno_avx10_2_256 : Flag<["-"], "mno-avx10.2-256">, Group<m_x86_AVX10_Features_Group>;
 def mavx10_2_512 : Flag<["-"], "mavx10.2-512">, Group<m_x86_AVX10_Features_Group>;
-def mno_avx10_2_512 : Flag<["-"], "mno-avx10.2-512">, Group<m_x86_AVX10_Features_Group>;
-def mavx10_2 : Flag<["-"], "mavx10.2">, Alias<mavx10_2_256>;
+def mno_avx10_2_512 : Flag<["-"], "mno-avx10.2-512">, Alias<mno_avx10_2_256>;
+def mavx10_2 : Flag<["-"], "mavx10.2">, Alias<mavx10_2_512>;
 def mno_avx10_2 : Flag<["-"], "mno-avx10.2">, Alias<mno_avx10_2_256>;
 def mavx2 : Flag<["-"], "mavx2">, Group<m_x86_Features_Group>;
 def mno_avx2 : Flag<["-"], "mno-avx2">, Group<m_x86_Features_Group>;
diff --git a/clang/test/Driver/x86-target-features.c b/clang/test/Driver/x86-target-features.c
index 339f593dc760a8..a977ceae0b01de 100644
--- a/clang/test/Driver/x86-target-features.c
+++ b/clang/test/Driver/x86-target-features.c
@@ -395,7 +395,9 @@
 // EVEX512: "-target-feature" "+evex512"
 // NO-EVEX512: "-target-feature" "-evex512"
 
-// RUN: %clang --target=i386 -mavx10.1 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_1_256 %s
+// RUN: not %clang --target=i386 -march=i386 -mavx10.1 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=UNSUPPORT-AVX10 %s
+// RUN: not %clang --target=i386 -march=i386 -mno-avx10.1 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=UNSUPPORT-AVX10 %s
+// RUN: %clang --target=i386 -mavx10.1-256 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_1_256 %s
 // RUN: %clang --target=i386 -mavx10.1-256 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_1_256 %s
 // RUN: %clang --target=i386 -mavx10.1-512 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_1_512 %s
 // RUN: %clang --target=i386 -mavx10.1-256 -mavx10.1-512 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_1_512 %s
@@ -403,15 +405,16 @@
 // RUN: not %clang --target=i386 -march=i386 -mavx10.1-128 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=BAD-AVX10 %s
 // RUN: not %clang --target=i386 -march=i386 -mavx10.a-256 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=BAD-AVX10 %s
 // RUN: not %clang --target=i386 -march=i386 -mavx10.1024-512 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=BAD-AVX10 %s
-// RUN: %clang --target=i386 -march=i386 -mavx10.1 -mavx512f %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10-AVX512 %s
-// RUN: %clang --target=i386 -march=i386 -mavx10.1 -mno-avx512f %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10-AVX512 %s
-// RUN: %clang --target=i386 -march=i386 -mavx10.1 -mevex512 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10-EVEX512 %s
-// RUN: %clang --target=i386 -march=i386 -mavx10.1 -mno-evex512 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10-EVEX512 %s
-// RUN: %clang --target=i386 -mavx10.2 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_2_256 %s
+// RUN: %clang --target=i386 -march=i386 -mavx10.1-256 -mavx512f %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10-AVX512 %s
+// RUN: %clang --target=i386 -march=i386 -mavx10.1-256 -mno-avx512f %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10-AVX512 %s
+// RUN: %clang --target=i386 -march=i386 -mavx10.1-256 -mevex512 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10-EVEX512 %s
+// RUN: %clang --target=i386 -march=i386 -mavx10.1-256 -mno-evex512 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10-EVEX512 %s
+// RUN: %clang --target=i386 -mavx10.2 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_2_512 %s
 // RUN: %clang --target=i386 -mavx10.2-256 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_2_256 %s
 // RUN: %clang --target=i386 -mavx10.2-512 %s -### -o %t.o 2>&1 | FileCheck -check-prefix=AVX10_2_512 %s
 // RUN: %clang --target=i386 -mavx10.2-256 -mavx10.1-512 %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=AVX10_2_256,AVX10_1_512 %s
 // RUN: %clang --target=i386 -mavx10.2-512 -mavx10.1-256 %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=AVX10_2_512,AVX10_1_256 %s
+// UNSUPPORT-AVX10: error: unsupported option '-m{{.*}}avx10.1' for target 'i386'
 // AVX10_2_256: "-target-feature" "+avx10.2-256"
 // AVX10_2_512: "-target-feature" "+avx10.2-512"
 // AVX10_1_256: "-target-feature" "+avx10.1-256"
diff --git a/clang/test/Preprocessor/x86_target_features.c b/clang/test/Preprocessor/x86_target_features.c
index fa3d0038f05a93..63222a882ff531 100644
--- a/clang/test/Preprocessor/x86_target_features.c
+++ b/clang/test/Preprocessor/x86_target_features.c
@@ -742,10 +742,8 @@
 // AVXVNNIINT16NOAVX2-NOT: #define __AVX2__ 1
 // AVXVNNIINT16NOAVX2-NOT: #define __AVXVNNIINT16__ 1
 
-// RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.1 -x c -E -dM -o - %s | FileCheck  -check-prefix=AVX10_1_256 %s
 // RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.1-256 -x c -E -dM -o - %s | FileCheck  -check-prefix=AVX10_1_256 %s
 // RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.1-256 -mno-avx512f -x c -E -dM -o - %s | FileCheck  -check-prefix=AVX10_1_256 %s
-// RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.2 -x c -E -dM -o - %s | FileCheck  -check-prefixes=AVX10_1_256,AVX10_2_256 %s
 // RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.2-256 -x c -E -dM -o - %s | FileCheck  -check-prefixes=AVX10_1_256,AVX10_2_256 %s
 // AVX10_1_256-NOT: __AVX10_1_512__
 // AVX10_1_256: #define __AVX10_1__ 1
@@ -758,6 +756,7 @@
 // RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.1-512 -x c -E -dM -o - %s | FileCheck  -check-prefix=AVX10_1_512 %s
 // RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.1-512 -mno-avx512f -x c -E -dM -o - %s | FileCheck  -check-prefix=AVX10_1_512 %s
 // RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.1-512 -mno-evex512 -x c -E -dM -o - %s | FileCheck  -check-prefix=AVX10_1_512 %s
+// RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.2 -x c -E -dM -o - %s | FileCheck  -check-prefixes=AVX10_1_512,AVX10_2_512 %s
 // RUN: %clang -target i686-unknown-linux-gnu -march=atom -mavx10.2-512 -x c -E -dM -o - %s | FileCheck  -check-prefixes=AVX10_1_512,AVX10_2_512 %s
 // AVX10_1_512: #define __AVX10_1_512__ 1
 // AVX10_1_512: #define __AVX10_1__ 1

[KanRobert]

We also change -mno-avx10.[1,2]-512 to alias of 256 bit options to disable both 256 and 512 instructions.

Why this?

[phoebewang]

We also change -mno-avx10.[1,2]-512 to alias of 256 bit options to disable both 256 and 512 instructions.

Why this?

Because we expect m[no-]avx10.2 cover both 256 and 512 bit instructions.

[e-kud]

We also change -mno-avx10.[1,2]-512 to alias of 256 bit options to disable both 256 and 512 instructions.

Why this?

Because we expect m[no-]avx10.2 cover both 256 and 512 bit instructions.

Interesting. Actually it makes weird combinations such as +avx10.2-512,-avx10.1-512 harder to access, now it should be +avx10.2-512,-avx10.1-512,+avx10.1-256. Or won't it work as avx10.1-256 aliases no-avx10.1-512 and we receive complete avx10.2-512?

[phoebewang]

We also change -mno-avx10.[1,2]-512 to alias of 256 bit options to disable both 256 and 512 instructions.

Why this?

Because we expect m[no-]avx10.2 cover both 256 and 512 bit instructions.

Interesting. Actually it makes weird combinations such as +avx10.2-512,-avx10.1-512 harder to access, now it should be +avx10.2-512,-avx10.1-512,+avx10.1-256. Or won't it work as avx10.1-256 aliases no-avx10.1-512 and we receive complete avx10.2-512?

Without this change, +avx10.2-512,-avx10.1-512 equals to +avx10.2-256 instead of +avx10.1-256. https://godbolt.org/z/797MGv1Y5 It is formulaic corrent, but it's easy to confuse user, and sometimes ourselves.

Users who care about 512-bit only, probably don't expect avx10.1-256 still enabled, not to mention avx10.2-256. With this change, they will get all AVX10.1 instrcutions disabled. (To make things more clear, we'd better to use 10.2 and 10.3 as an example, AVX10.1 intersects with AVX512 instruction, so they are not disabled in fact).

[e-kud]

I've been playing around and found that -mavx10.2 -mno-avx10.2-512 enables avx10.1-512 but -mavx10.2-512 -mno-avx10.2-512 obviously doesn't. Does it make sense? It happens because when options match, they are eliminated before processing. But this is a problem not related to the PR.

[e-kud]

LGTM. I think complete disable of AVX10 versions later or equal than specified in -mno is more clear than implicitly disabling only 512 version instructions.

[phoebewang]

I've been playing around and found that -mavx10.2 -mno-avx10.2-512 enables avx10.1-512 but -mavx10.2-512 -mno-avx10.2-512 obviously doesn't. Does it make sense? It happens because when options match, they are eliminated before processing. But this is a problem not related to the PR.

The old behavior is "correct". Using "" because I think it is just an implementation trick. Though the compliance to it is not important, we decide to remove the -256/-512 options after discussed with GCC folks, to further reduce some confusions. We have to keep 10.1 options for a while, because it's risky to switch avx10.1 to alias of avx10.1-512 directly.

Let me know whether you are happy with this solution or not.

[e-kud]

Let me know whether you are happy with this solution or not.

Yes, thanks, I agree, this is better. Since we disable AVX10 version completely with any of options 256 or 512 (some kind of duplication), it means that we can't partially disable it, so these options doesn't make much sense.

And some uncommon scenarios are also available e.g., to benchmark a function compiled with avx10.2-256 in avx10.2-512 context we need to use __attribute__((target("no-avx10.1-512"))). So basically no-avx10.1-512 is an option to disable 512 bit instructions of AVX10 regardless of enabled AVX10.x

[phoebewang]

Let me know whether you are happy with this solution or not.

Yes, thanks, I agree, this is better. Since we disable AVX10 version completely with any of options 256 or 512 (some kind of duplication), it means that we can't partially disable it, so these options doesn't make much sense.

And some uncommon scenarios are also available e.g., to benchmark a function compiled with avx10.2-256 in avx10.2-512 context we need to use __attribute__((target("no-avx10.1-512"))). So basically no-avx10.1-512 is an option to disable 512 bit instructions of AVX10 regardless of enabled AVX10.x

Yes, good point!