[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
phoebewang wrote: Thanks @ronlieb and @mstorsjo. Created #72126 for it. https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
mstorsjo wrote: > Hi Phoebe, starting seeing this error on rather old codes after this patch > landed . is there a particular flag you recommend i should compile with to > get previous behavior ? > > error: always_inline function '_mm_setzero_pd' requires target feature > 'evex512', but would be inlined into function '_mm_getexp_pd' that is > compiled without support for 'evex512' I also ran into something similar, when compiling Qt; I filed https://github.com/llvm/llvm-project/issues/72106 with a different reproducer. https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
ronlieb wrote:
smallest example and used latest upstream llvm build from this morning
#include
void log_d_scalar(double a_input)
{
__m128d va, ve;
ve = _mm_getexp_sd(va, va);
}
clang++ -march=haswell t.cpp
t.cpp:8:10: error: always_inline function '_mm_getexp_sd' requires target
feature 'avx512f', but would be inlined into function 'log_d_scalar' that is
compiled without support for 'avx512f'
8 | ve = _mm_getexp_sd(va, va);
| ^
1 error generated.
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
phoebewang wrote:
@ronlieb The reproducer can compile successfully in trunk:
https://godbolt.org/z/hvKhGq9bq
Are you using a downstream compiler? You can check if the "emmintrin.h" has the
same change as main trunk.
You can also check it through pre-compile the code:
```
$ clang++ -E fd_log_scalar.cpp -D_CPU | grep '_mm_setzero_pd.*{'
static __inline__ __m128d __attribute__((__always_inline__, __nodebug__,
__target__("sse2,no-evex512"), __min_vector_width__(128))) _mm_setzero_pd(void)
{
```
Make sure `no-evex512` is in the attribute too.
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
ronlieb wrote:
> @ronlieb Do you have a reproducer for this problem? I just checked the
> definition of both intrinsics have `no-evex512` already, so shouldn't have
> such problem. You can use -mno-evex512 as workaround for the problem anyway.
here is a small reproducer , compile with
clang++ -c fd_log_scalar.cpp -mtune=skylake-avx512 -march=skylake-avx512
-D_CPU=avx512
produces error :
lib/clang/18/include/avx512fintrin.h:5493:41: error: always_inline function
'_mm_setzero_pd' requires target feature 'evex512', but would be inlined into
function '_mm_getexp_sd' that is compiled without support for 'evex512'
5493 | (__v2df) __B, (__v2df) _mm_setzero_pd(), (__mmask8)
-1, _MM_FROUND_CUR_DIRECTION);
| ^
1 error generated.
#include
#if !(defined _CPU)
#error: please define _CPU - specific suffix to a function name
#endif
extern "C" double log_d_scalar(double);
double __attribute__ ((noinline)) log_d_scalar(double a_input)
{
__m128d va, vm, ve, vb;
double a, m, e, b, t;
long long mu, eu;
#ifdef __AVX512F__
va = _mm_set_sd(a_input);
vm = _mm_getmant_sd(va, va, _MM_MANT_NORM_p75_1p5, _MM_MANT_SIGN_nan);
ve = _mm_getexp_sd(va, va);
vb = _mm_getexp_sd(vm, vm);
ve = _mm_sub_sd(ve, vb);
m = _mm_cvtsd_f64(vm);
e = _mm_cvtsd_f64(ve);
#endif
return m + e;
}
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
ronlieb wrote: thanks, will try workaround for now ... https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
phoebewang wrote: @ronlieb Do you have a reproducer for this problem? I just checked the definition of both intrinsics have `no-evex512` already, so shouldn't have such problem. You can use -mno-evex512 as workaround for the problem anyway. https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
ronlieb wrote: Hi Phoebe, starting seeing this error on rather old codes after this patch landed . is there a particular flag you recommend i should compile with to get previous behavior ? error: always_inline function '_mm_setzero_pd' requires target feature 'evex512', but would be inlined into function '_mm_getexp_pd' that is compiled without support for 'evex512' https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
phoebewang wrote: Thanks @KanRobert @e-kud https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
https://github.com/phoebewang closed https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
https://github.com/e-kud approved this pull request. LGTM. Thanks! https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
https://github.com/phoebewang edited https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
https://github.com/KanRobert approved this pull request. LGTM. Maybe we can add some explanations about why we add attribute `no-evex512` for intrinsics in the description of the PR. It's a little tricky. https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature == "-avx10.1-256") {
+HasAVX10 = false;
+HasAVX10_512 = false;
+ } else if (HasAVX10_512 && Feature == "-avx10.1-512") {
+HasAVX10_512 = false;
}
+ // Postpone AVX10 features handling after AVX512 settled.
+ UpdatedAVX10FeaturesVec.push_back(Feature);
+ continue;
} else if (!HasAVX512F && Feature.substr(0, 7) == "+avx512") {
HasAVX512F = true;
+ LastAVX512 = Feature;
} else if (HasAVX512F && Feature == "-avx512f") {
HasAVX512F = false;
-} else if (HasAVX10 && Feature == "-avx10.1-256") {
- HasAVX10 = false;
- HasAVX512F = false;
-} else if (!HasEVEX512 && Feature == "+evex512") {
+} else if (HasEVEX512 != true && Feature == "+evex512") {
KanRobert wrote:
You can check `has_value` for uninitialized value. But enum looks good to me,
too.
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
phoebewang wrote: > I'm a little bit confused, What's the expected behavior of `+avx10.1-512 > -avx10.1-256` in codegen aspect? Should we generate only instructions in the > difference of sets? Or do we consider `avx10.1-256` as a base of > `avx10.1-512` and if it is disabled `avx10.1-512` can't be enabled? `-avx10.1-256` works like `-avx512f`, that says, they are special as a fundamental feature, which will turn off all derivative features for AVX10 and AVX512 respectively. OTOH, derivative features will only turn off the difference set, e.g., `+avx10.3-256 -avx10.2-256` equals to `+avx10.1-256`. https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature == "-avx10.1-256") {
+HasAVX10 = false;
+HasAVX10_512 = false;
+ } else if (HasAVX10_512 && Feature == "-avx10.1-512") {
+HasAVX10_512 = false;
}
+ // Postpone AVX10 features handling after AVX512 settled.
+ UpdatedAVX10FeaturesVec.push_back(Feature);
+ continue;
} else if (!HasAVX512F && Feature.substr(0, 7) == "+avx512") {
HasAVX512F = true;
+ LastAVX512 = Feature;
} else if (HasAVX512F && Feature == "-avx512f") {
HasAVX512F = false;
-} else if (HasAVX10 && Feature == "-avx10.1-256") {
- HasAVX10 = false;
- HasAVX512F = false;
-} else if (!HasEVEX512 && Feature == "+evex512") {
+} else if (HasEVEX512 != true && Feature == "+evex512") {
phoebewang wrote:
I think "std::optional" doesn't help here because we need to distinguish the
uninitialized status and false too.
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -50,11 +50,11 @@ typedef __bf16 __m128bh __attribute__((__vector_size__(16),
__aligned__(16)));
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS
\
- __attribute__((__always_inline__, __nodebug__, __target__("sse2"),
\
- __min_vector_width__(128)))
+ __attribute__((__always_inline__, __nodebug__,
\
+ __target__("sse2,no-evex512"), __min_vector_width__(128)))
#define __DEFAULT_FN_ATTRS_MMX
\
- __attribute__((__always_inline__, __nodebug__, __target__("mmx,sse2"),
\
- __min_vector_width__(64)))
+ __attribute__((__always_inline__, __nodebug__,
\
phoebewang wrote:
The same reason as above.
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -15,8 +15,12 @@
#define __AVX2INTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS256 __attribute__((__always_inline__, __nodebug__,
__target__("avx2"), __min_vector_width__(256)))
-#define __DEFAULT_FN_ATTRS128 __attribute__((__always_inline__, __nodebug__,
__target__("avx2"), __min_vector_width__(128)))
+#define __DEFAULT_FN_ATTRS256
\
+ __attribute__((__always_inline__, __nodebug__,
\
+ __target__("avx2,no-evex512"), __min_vector_width__(256)))
phoebewang wrote:
We have defined parts AVX512 intrinsics with `no-evex512` and some of them will
call into these AVX2 intrinsics.
Then we are facing a problem that we cannot call them in some cases because we
didn't specify `no-evex512` for them.
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -119,9 +119,13 @@ bool X86TargetInfo::initFeatureMap( setFeatureEnabled(Features, F, true); std::vector UpdatedFeaturesVec; - bool HasEVEX512 = true; + std::vector UpdatedAVX10FeaturesVec; + int HasEVEX512 = -1; phoebewang wrote: I think it's better to use enum. It's a 3-status flag. std::optional isn't much useful here. https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
https://github.com/phoebewang updated
https://github.com/llvm/llvm-project/pull/71318
>From d9ee6309924e7f248695cbd488afe98273432e84 Mon Sep 17 00:00:00 2001
From: Phoebe Wang
Date: Sun, 5 Nov 2023 21:15:53 +0800
Subject: [PATCH 1/3] [X86][AVX10] Permit AVX512 options/features used together
with AVX10
This patch relaxes the driver logic to permit combinations between
AVX512 and AVX10 options and makes sure we have a unified behavior
between options and features combination.
Here are rules we are following when handle these combinations:
1. evex512 can only be used for avx512xxx options/features. It will be
ignored if used without them;
2. avx512xxx and avx10.xxx are options in two worlds. Avoid to use them
together in any case. It will enable a common super set when they are
used together. E.g., "-mavx512f -mavx10.1-256" euqals "-mavx10.1-512".
Compiler emits warnings when user using combinations like
"-mavx512f -mavx10.1-256" in case they won't get unexpected result silently.
---
.../clang/Basic/DiagnosticCommonKinds.td | 2 +
clang/lib/Basic/Targets/X86.cpp | 57 ---
clang/lib/Driver/ToolChains/Arch/X86.cpp | 7 ---
clang/lib/Headers/avx2intrin.h| 4 +-
clang/lib/Headers/avx512bf16intrin.h | 3 +-
clang/lib/Headers/avx512bwintrin.h| 4 +-
clang/lib/Headers/avx512dqintrin.h| 4 +-
clang/lib/Headers/avx512fintrin.h | 8 ++-
clang/lib/Headers/avx512fp16intrin.h | 6 +-
clang/lib/Headers/avx512ifmavlintrin.h| 10 +++-
clang/lib/Headers/avx512pfintrin.h| 5 --
clang/lib/Headers/avx512vbmivlintrin.h| 11 +++-
clang/lib/Headers/avx512vlbf16intrin.h| 14 +++--
clang/lib/Headers/avx512vlbitalgintrin.h | 10 +++-
clang/lib/Headers/avx512vlbwintrin.h | 10 +++-
clang/lib/Headers/avx512vlcdintrin.h | 11 +++-
clang/lib/Headers/avx512vldqintrin.h | 10 +++-
clang/lib/Headers/avx512vlfp16intrin.h| 4 +-
clang/lib/Headers/avx512vlintrin.h| 10 +++-
clang/lib/Headers/avx512vlvbmi2intrin.h | 10 +++-
clang/lib/Headers/avx512vlvnniintrin.h| 10 +++-
.../lib/Headers/avx512vlvp2intersectintrin.h | 10 ++--
clang/lib/Headers/avx512vpopcntdqvlintrin.h | 8 ++-
clang/lib/Headers/avxintrin.h | 4 +-
clang/lib/Headers/emmintrin.h | 4 +-
clang/lib/Headers/gfniintrin.h| 14 +++--
clang/lib/Headers/pmmintrin.h | 2 +-
clang/lib/Headers/smmintrin.h | 2 +-
clang/lib/Headers/tmmintrin.h | 4 +-
clang/lib/Headers/xmmintrin.h | 4 +-
clang/test/CodeGen/X86/avx512-error.c | 13 +
clang/test/CodeGen/target-avx-abi-diag.c | 28 -
clang/test/Driver/x86-target-features.c | 6 +-
33 files changed, 214 insertions(+), 95 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 9f0ccd255a32148..8084a4ce0d1751b 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -346,6 +346,8 @@ def err_opt_not_valid_on_target : Error<
"option '%0' cannot be specified on this target">;
def err_invalid_feature_combination : Error<
"invalid feature combination: %0">;
+def warn_invalid_feature_combination : Warning<
+ "invalid feature combination: %0">,
InGroup>;
def warn_target_unrecognized_env : Warning<
"mismatch between architecture and environment in target triple '%0'; did
you mean '%1'?">,
InGroup;
diff --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index eec3cd558435e2a..9cfda95f385d627 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -119,9 +119,13 @@ bool X86TargetInfo::initFeatureMap(
setFeatureEnabled(Features, F, true);
std::vector UpdatedFeaturesVec;
- bool HasEVEX512 = true;
+ std::vector UpdatedAVX10FeaturesVec;
+ int HasEVEX512 = -1;
bool HasAVX512F = false;
bool HasAVX10 = false;
+ bool HasAVX10_512 = false;
+ std::string LastAVX10;
+ std::string LastAVX512;
for (const auto : FeaturesVec) {
// Expand general-regs-only to -x86, -mmx and -sse
if (Feature == "+general-regs-only") {
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature ==
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
e-kud wrote: I'm a little bit confused, What's the expected behavior of `+avx10.1-512 -avx10.1-256` in codegen aspect. Should we generate only instructions in the difference of sets? Or do we consider `avx10.1-256` as a base of `avx10.1-512` and if it is disabled `avx10.1-512` can't be enabled? https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature == "-avx10.1-256") {
+HasAVX10 = false;
+HasAVX10_512 = false;
+ } else if (HasAVX10_512 && Feature == "-avx10.1-512") {
+HasAVX10_512 = false;
}
+ // Postpone AVX10 features handling after AVX512 settled.
+ UpdatedAVX10FeaturesVec.push_back(Feature);
+ continue;
} else if (!HasAVX512F && Feature.substr(0, 7) == "+avx512") {
HasAVX512F = true;
+ LastAVX512 = Feature;
} else if (HasAVX512F && Feature == "-avx512f") {
HasAVX512F = false;
-} else if (HasAVX10 && Feature == "-avx10.1-256") {
- HasAVX10 = false;
- HasAVX512F = false;
-} else if (!HasEVEX512 && Feature == "+evex512") {
+} else if (HasEVEX512 != true && Feature == "+evex512") {
HasEVEX512 = true;
-} else if (HasEVEX512 && Feature == "-avx10.1-512") {
- HasEVEX512 = false;
-} else if (HasEVEX512 && Feature == "-evex512") {
+ continue;
+} else if (HasEVEX512 != false && Feature == "-evex512") {
HasEVEX512 = false;
+ continue;
}
UpdatedFeaturesVec.push_back(Feature);
}
- if (HasAVX512F && HasEVEX512)
-UpdatedFeaturesVec.push_back("+evex512");
- else if (HasAVX10)
-UpdatedFeaturesVec.push_back("-evex512");
+ llvm::append_range(UpdatedFeaturesVec, UpdatedAVX10FeaturesVec);
KanRobert wrote:
I see.
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature == "-avx10.1-256") {
+HasAVX10 = false;
+HasAVX10_512 = false;
+ } else if (HasAVX10_512 && Feature == "-avx10.1-512") {
+HasAVX10_512 = false;
}
+ // Postpone AVX10 features handling after AVX512 settled.
+ UpdatedAVX10FeaturesVec.push_back(Feature);
+ continue;
} else if (!HasAVX512F && Feature.substr(0, 7) == "+avx512") {
HasAVX512F = true;
+ LastAVX512 = Feature;
} else if (HasAVX512F && Feature == "-avx512f") {
HasAVX512F = false;
-} else if (HasAVX10 && Feature == "-avx10.1-256") {
- HasAVX10 = false;
- HasAVX512F = false;
-} else if (!HasEVEX512 && Feature == "+evex512") {
+} else if (HasEVEX512 != true && Feature == "+evex512") {
HasEVEX512 = true;
-} else if (HasEVEX512 && Feature == "-avx10.1-512") {
- HasEVEX512 = false;
-} else if (HasEVEX512 && Feature == "-evex512") {
+ continue;
+} else if (HasEVEX512 != false && Feature == "-evex512") {
HasEVEX512 = false;
+ continue;
}
UpdatedFeaturesVec.push_back(Feature);
}
- if (HasAVX512F && HasEVEX512)
-UpdatedFeaturesVec.push_back("+evex512");
- else if (HasAVX10)
-UpdatedFeaturesVec.push_back("-evex512");
+ llvm::append_range(UpdatedFeaturesVec, UpdatedAVX10FeaturesVec);
e-kud wrote:
Nope, there is a `continue` in handling `avx10*` features
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature == "-avx10.1-256") {
+HasAVX10 = false;
+HasAVX10_512 = false;
+ } else if (HasAVX10_512 && Feature == "-avx10.1-512") {
+HasAVX10_512 = false;
}
+ // Postpone AVX10 features handling after AVX512 settled.
+ UpdatedAVX10FeaturesVec.push_back(Feature);
+ continue;
} else if (!HasAVX512F && Feature.substr(0, 7) == "+avx512") {
HasAVX512F = true;
+ LastAVX512 = Feature;
} else if (HasAVX512F && Feature == "-avx512f") {
HasAVX512F = false;
-} else if (HasAVX10 && Feature == "-avx10.1-256") {
- HasAVX10 = false;
- HasAVX512F = false;
-} else if (!HasEVEX512 && Feature == "+evex512") {
+} else if (HasEVEX512 != true && Feature == "+evex512") {
HasEVEX512 = true;
-} else if (HasEVEX512 && Feature == "-avx10.1-512") {
- HasEVEX512 = false;
-} else if (HasEVEX512 && Feature == "-evex512") {
+ continue;
+} else if (HasEVEX512 != false && Feature == "-evex512") {
HasEVEX512 = false;
+ continue;
}
UpdatedFeaturesVec.push_back(Feature);
}
- if (HasAVX512F && HasEVEX512)
-UpdatedFeaturesVec.push_back("+evex512");
- else if (HasAVX10)
-UpdatedFeaturesVec.push_back("-evex512");
+ llvm::append_range(UpdatedFeaturesVec, UpdatedAVX10FeaturesVec);
KanRobert wrote:
Does it mean the flags for AVX10 Features will be in the vector
`UpdatedFeaturesVec` 2 times?
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature == "-avx10.1-256") {
+HasAVX10 = false;
+HasAVX10_512 = false;
+ } else if (HasAVX10_512 && Feature == "-avx10.1-512") {
+HasAVX10_512 = false;
}
+ // Postpone AVX10 features handling after AVX512 settled.
+ UpdatedAVX10FeaturesVec.push_back(Feature);
+ continue;
} else if (!HasAVX512F && Feature.substr(0, 7) == "+avx512") {
HasAVX512F = true;
+ LastAVX512 = Feature;
} else if (HasAVX512F && Feature == "-avx512f") {
HasAVX512F = false;
-} else if (HasAVX10 && Feature == "-avx10.1-256") {
- HasAVX10 = false;
- HasAVX512F = false;
-} else if (!HasEVEX512 && Feature == "+evex512") {
+} else if (HasEVEX512 != true && Feature == "+evex512") {
KanRobert wrote:
Comparing a int value with `true` and `false` may be confusing. I suggest
"std::optional"
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -50,11 +50,11 @@ typedef __bf16 __m128bh __attribute__((__vector_size__(16),
__aligned__(16)));
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS
\
- __attribute__((__always_inline__, __nodebug__, __target__("sse2"),
\
- __min_vector_width__(128)))
+ __attribute__((__always_inline__, __nodebug__,
\
+ __target__("sse2,no-evex512"), __min_vector_width__(128)))
#define __DEFAULT_FN_ATTRS_MMX
\
- __attribute__((__always_inline__, __nodebug__, __target__("mmx,sse2"),
\
- __min_vector_width__(64)))
+ __attribute__((__always_inline__, __nodebug__,
\
KanRobert wrote:
Why does the function targeted at sse2 need no-evex512?
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -15,8 +15,12 @@
#define __AVX2INTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS256 __attribute__((__always_inline__, __nodebug__,
__target__("avx2"), __min_vector_width__(256)))
-#define __DEFAULT_FN_ATTRS128 __attribute__((__always_inline__, __nodebug__,
__target__("avx2"), __min_vector_width__(128)))
+#define __DEFAULT_FN_ATTRS256
\
+ __attribute__((__always_inline__, __nodebug__,
\
+ __target__("avx2,no-evex512"), __min_vector_width__(256)))
KanRobert wrote:
Why does the function targeted at avx2 need `no-evex512`?
https://github.com/llvm/llvm-project/pull/71318
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
@@ -119,9 +119,13 @@ bool X86TargetInfo::initFeatureMap( setFeatureEnabled(Features, F, true); std::vector UpdatedFeaturesVec; - bool HasEVEX512 = true; + std::vector UpdatedAVX10FeaturesVec; + int HasEVEX512 = -1; KanRobert wrote: Use std::optional ? https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
phoebewang wrote: Ping~ https://github.com/llvm/llvm-project/pull/71318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
https://github.com/phoebewang updated
https://github.com/llvm/llvm-project/pull/71318
>From d9ee6309924e7f248695cbd488afe98273432e84 Mon Sep 17 00:00:00 2001
From: Phoebe Wang
Date: Sun, 5 Nov 2023 21:15:53 +0800
Subject: [PATCH 1/2] [X86][AVX10] Permit AVX512 options/features used together
with AVX10
This patch relaxes the driver logic to permit combinations between
AVX512 and AVX10 options and makes sure we have a unified behavior
between options and features combination.
Here are rules we are following when handle these combinations:
1. evex512 can only be used for avx512xxx options/features. It will be
ignored if used without them;
2. avx512xxx and avx10.xxx are options in two worlds. Avoid to use them
together in any case. It will enable a common super set when they are
used together. E.g., "-mavx512f -mavx10.1-256" euqals "-mavx10.1-512".
Compiler emits warnings when user using combinations like
"-mavx512f -mavx10.1-256" in case they won't get unexpected result silently.
---
.../clang/Basic/DiagnosticCommonKinds.td | 2 +
clang/lib/Basic/Targets/X86.cpp | 57 ---
clang/lib/Driver/ToolChains/Arch/X86.cpp | 7 ---
clang/lib/Headers/avx2intrin.h| 4 +-
clang/lib/Headers/avx512bf16intrin.h | 3 +-
clang/lib/Headers/avx512bwintrin.h| 4 +-
clang/lib/Headers/avx512dqintrin.h| 4 +-
clang/lib/Headers/avx512fintrin.h | 8 ++-
clang/lib/Headers/avx512fp16intrin.h | 6 +-
clang/lib/Headers/avx512ifmavlintrin.h| 10 +++-
clang/lib/Headers/avx512pfintrin.h| 5 --
clang/lib/Headers/avx512vbmivlintrin.h| 11 +++-
clang/lib/Headers/avx512vlbf16intrin.h| 14 +++--
clang/lib/Headers/avx512vlbitalgintrin.h | 10 +++-
clang/lib/Headers/avx512vlbwintrin.h | 10 +++-
clang/lib/Headers/avx512vlcdintrin.h | 11 +++-
clang/lib/Headers/avx512vldqintrin.h | 10 +++-
clang/lib/Headers/avx512vlfp16intrin.h| 4 +-
clang/lib/Headers/avx512vlintrin.h| 10 +++-
clang/lib/Headers/avx512vlvbmi2intrin.h | 10 +++-
clang/lib/Headers/avx512vlvnniintrin.h| 10 +++-
.../lib/Headers/avx512vlvp2intersectintrin.h | 10 ++--
clang/lib/Headers/avx512vpopcntdqvlintrin.h | 8 ++-
clang/lib/Headers/avxintrin.h | 4 +-
clang/lib/Headers/emmintrin.h | 4 +-
clang/lib/Headers/gfniintrin.h| 14 +++--
clang/lib/Headers/pmmintrin.h | 2 +-
clang/lib/Headers/smmintrin.h | 2 +-
clang/lib/Headers/tmmintrin.h | 4 +-
clang/lib/Headers/xmmintrin.h | 4 +-
clang/test/CodeGen/X86/avx512-error.c | 13 +
clang/test/CodeGen/target-avx-abi-diag.c | 28 -
clang/test/Driver/x86-target-features.c | 6 +-
33 files changed, 214 insertions(+), 95 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 9f0ccd255a32148..8084a4ce0d1751b 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -346,6 +346,8 @@ def err_opt_not_valid_on_target : Error<
"option '%0' cannot be specified on this target">;
def err_invalid_feature_combination : Error<
"invalid feature combination: %0">;
+def warn_invalid_feature_combination : Warning<
+ "invalid feature combination: %0">,
InGroup>;
def warn_target_unrecognized_env : Warning<
"mismatch between architecture and environment in target triple '%0'; did
you mean '%1'?">,
InGroup;
diff --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index eec3cd558435e2a..9cfda95f385d627 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -119,9 +119,13 @@ bool X86TargetInfo::initFeatureMap(
setFeatureEnabled(Features, F, true);
std::vector UpdatedFeaturesVec;
- bool HasEVEX512 = true;
+ std::vector UpdatedAVX10FeaturesVec;
+ int HasEVEX512 = -1;
bool HasAVX512F = false;
bool HasAVX10 = false;
+ bool HasAVX10_512 = false;
+ std::string LastAVX10;
+ std::string LastAVX512;
for (const auto : FeaturesVec) {
// Expand general-regs-only to -x86, -mmx and -sse
if (Feature == "+general-regs-only") {
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature ==
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 8a7846fe86f95e82c6bd5f4f45b8dd311320e903
d9ee6309924e7f248695cbd488afe98273432e84 -- clang/lib/Basic/Targets/X86.cpp
clang/lib/Driver/ToolChains/Arch/X86.cpp clang/lib/Headers/avx2intrin.h
clang/lib/Headers/avx512bf16intrin.h clang/lib/Headers/avx512bwintrin.h
clang/lib/Headers/avx512dqintrin.h clang/lib/Headers/avx512fintrin.h
clang/lib/Headers/avx512fp16intrin.h clang/lib/Headers/avx512ifmavlintrin.h
clang/lib/Headers/avx512pfintrin.h clang/lib/Headers/avx512vbmivlintrin.h
clang/lib/Headers/avx512vlbf16intrin.h clang/lib/Headers/avx512vlbitalgintrin.h
clang/lib/Headers/avx512vlbwintrin.h clang/lib/Headers/avx512vlcdintrin.h
clang/lib/Headers/avx512vldqintrin.h clang/lib/Headers/avx512vlfp16intrin.h
clang/lib/Headers/avx512vlintrin.h clang/lib/Headers/avx512vlvbmi2intrin.h
clang/lib/Headers/avx512vlvnniintrin.h
clang/lib/Headers/avx512vlvp2intersectintrin.h
clang/lib/Headers/avx512vpopcntdqvlintrin.h clang/lib/Headers/avxintrin.h
clang/lib/Headers/emmintrin.h clang/lib/Headers/gfniintrin.h
clang/lib/Headers/pmmintrin.h clang/lib/Headers/smmintrin.h
clang/lib/Headers/tmmintrin.h clang/lib/Headers/xmmintrin.h
clang/test/CodeGen/X86/avx512-error.c clang/test/CodeGen/target-avx-abi-diag.c
clang/test/Driver/x86-target-features.c
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/Headers/avx2intrin.h b/clang/lib/Headers/avx2intrin.h
index 2bb0fa39c..096cae01b 100644
--- a/clang/lib/Headers/avx2intrin.h
+++ b/clang/lib/Headers/avx2intrin.h
@@ -15,8 +15,12 @@
#define __AVX2INTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS256 __attribute__((__always_inline__, __nodebug__,
__target__("avx2,no-evex512"), __min_vector_width__(256)))
-#define __DEFAULT_FN_ATTRS128 __attribute__((__always_inline__, __nodebug__,
__target__("avx2,no-evex512"), __min_vector_width__(128)))
+#define __DEFAULT_FN_ATTRS256
\
+ __attribute__((__always_inline__, __nodebug__,
\
+ __target__("avx2,no-evex512"), __min_vector_width__(256)))
+#define __DEFAULT_FN_ATTRS128
\
+ __attribute__((__always_inline__, __nodebug__,
\
+ __target__("avx2,no-evex512"), __min_vector_width__(128)))
/* SSE4 Multiple Packed Sums of Absolute Difference. */
/// Computes sixteen sum of absolute difference (SAD) operations on sets of
diff --git a/clang/lib/Headers/avxintrin.h b/clang/lib/Headers/avxintrin.h
index b74701a8c..f116d8bc3 100644
--- a/clang/lib/Headers/avxintrin.h
+++ b/clang/lib/Headers/avxintrin.h
@@ -50,8 +50,12 @@ typedef __bf16 __m256bh __attribute__((__vector_size__(32),
__aligned__(32)));
#endif
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__,
__target__("avx,no-evex512"), __min_vector_width__(256)))
-#define __DEFAULT_FN_ATTRS128 __attribute__((__always_inline__, __nodebug__,
__target__("avx,no-evex512"), __min_vector_width__(128)))
+#define __DEFAULT_FN_ATTRS
\
+ __attribute__((__always_inline__, __nodebug__, __target__("avx,no-evex512"),
\
+ __min_vector_width__(256)))
+#define __DEFAULT_FN_ATTRS128
\
+ __attribute__((__always_inline__, __nodebug__, __target__("avx,no-evex512"),
\
+ __min_vector_width__(128)))
/* Arithmetic */
/// Adds two 256-bit vectors of [4 x double].
diff --git a/clang/lib/Headers/emmintrin.h b/clang/lib/Headers/emmintrin.h
index 3ff13e6b0..96e3ebdec 100644
--- a/clang/lib/Headers/emmintrin.h
+++ b/clang/lib/Headers/emmintrin.h
@@ -50,11 +50,11 @@ typedef __bf16 __m128bh __attribute__((__vector_size__(16),
__aligned__(16)));
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS
\
- __attribute__((__always_inline__, __nodebug__,
__target__("sse2,no-evex512"), \
- __min_vector_width__(128)))
+ __attribute__((__always_inline__, __nodebug__,
\
+ __target__("sse2,no-evex512"), __min_vector_width__(128)))
#define __DEFAULT_FN_ATTRS_MMX
\
- __attribute__((__always_inline__, __nodebug__,
__target__("mmx,sse2,no-evex512"), \
- __min_vector_width__(64)))
+ __attribute__((__always_inline__, __nodebug__,
\
+ __target__("mmx,sse2,no-evex512"), __min_vector_width__(64)))
/// Adds
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
llvmbot wrote:
@llvm/pr-subscribers-backend-x86
Author: Phoebe Wang (phoebewang)
Changes
This patch relaxes the driver logic to permit combinations between AVX512 and
AVX10 options and makes sure we have a unified behavior between options and
features combination.
Here are rules we are following when handle these combinations:
1. evex512 can only be used for avx512xxx options/features. It will be ignored
if used without them;
2. avx512xxx and avx10.xxx are options in two worlds. Avoid to use them
together in any case. It will enable a common super set when they are used
together. E.g., "-mavx512f -mavx10.1-256" euqals "-mavx10.1-512".
Compiler emits warnings when user using combinations like "-mavx512f
-mavx10.1-256" in case they won't get unexpected result silently.
---
Patch is 42.99 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/71318.diff
33 Files Affected:
- (modified) clang/include/clang/Basic/DiagnosticCommonKinds.td (+2)
- (modified) clang/lib/Basic/Targets/X86.cpp (+38-19)
- (modified) clang/lib/Driver/ToolChains/Arch/X86.cpp (-7)
- (modified) clang/lib/Headers/avx2intrin.h (+2-2)
- (modified) clang/lib/Headers/avx512bf16intrin.h (+2-1)
- (modified) clang/lib/Headers/avx512bwintrin.h (+3-1)
- (modified) clang/lib/Headers/avx512dqintrin.h (+3-1)
- (modified) clang/lib/Headers/avx512fintrin.h (+6-2)
- (modified) clang/lib/Headers/avx512fp16intrin.h (+4-2)
- (modified) clang/lib/Headers/avx512ifmavlintrin.h (+8-2)
- (modified) clang/lib/Headers/avx512pfintrin.h (-5)
- (modified) clang/lib/Headers/avx512vbmivlintrin.h (+8-3)
- (modified) clang/lib/Headers/avx512vlbf16intrin.h (+8-6)
- (modified) clang/lib/Headers/avx512vlbitalgintrin.h (+8-2)
- (modified) clang/lib/Headers/avx512vlbwintrin.h (+8-2)
- (modified) clang/lib/Headers/avx512vlcdintrin.h (+8-3)
- (modified) clang/lib/Headers/avx512vldqintrin.h (+8-2)
- (modified) clang/lib/Headers/avx512vlfp16intrin.h (+2-2)
- (modified) clang/lib/Headers/avx512vlintrin.h (+8-2)
- (modified) clang/lib/Headers/avx512vlvbmi2intrin.h (+8-2)
- (modified) clang/lib/Headers/avx512vlvnniintrin.h (+8-2)
- (modified) clang/lib/Headers/avx512vlvp2intersectintrin.h (+6-4)
- (modified) clang/lib/Headers/avx512vpopcntdqvlintrin.h (+6-2)
- (modified) clang/lib/Headers/avxintrin.h (+2-2)
- (modified) clang/lib/Headers/emmintrin.h (+2-2)
- (modified) clang/lib/Headers/gfniintrin.h (+10-4)
- (modified) clang/lib/Headers/pmmintrin.h (+1-1)
- (modified) clang/lib/Headers/smmintrin.h (+1-1)
- (modified) clang/lib/Headers/tmmintrin.h (+2-2)
- (modified) clang/lib/Headers/xmmintrin.h (+2-2)
- (modified) clang/test/CodeGen/X86/avx512-error.c (+13)
- (modified) clang/test/CodeGen/target-avx-abi-diag.c (+25-3)
- (modified) clang/test/Driver/x86-target-features.c (+2-4)
``diff
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 9f0ccd255a32148..8084a4ce0d1751b 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -346,6 +346,8 @@ def err_opt_not_valid_on_target : Error<
"option '%0' cannot be specified on this target">;
def err_invalid_feature_combination : Error<
"invalid feature combination: %0">;
+def warn_invalid_feature_combination : Warning<
+ "invalid feature combination: %0">,
InGroup>;
def warn_target_unrecognized_env : Warning<
"mismatch between architecture and environment in target triple '%0'; did
you mean '%1'?">,
InGroup;
diff --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index eec3cd558435e2a..9cfda95f385d627 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -119,9 +119,13 @@ bool X86TargetInfo::initFeatureMap(
setFeatureEnabled(Features, F, true);
std::vector UpdatedFeaturesVec;
- bool HasEVEX512 = true;
+ std::vector UpdatedAVX10FeaturesVec;
+ int HasEVEX512 = -1;
bool HasAVX512F = false;
bool HasAVX10 = false;
+ bool HasAVX10_512 = false;
+ std::string LastAVX10;
+ std::string LastAVX512;
for (const auto : FeaturesVec) {
// Expand general-regs-only to -x86, -mmx and -sse
if (Feature == "+general-regs-only") {
@@ -131,35 +135,50 @@ bool X86TargetInfo::initFeatureMap(
continue;
}
-if (Feature.substr(0, 7) == "+avx10.") {
- HasAVX10 = true;
- HasAVX512F = true;
- if (Feature.substr(Feature.size() - 3, 3) == "512") {
-HasEVEX512 = true;
- } else if (Feature.substr(7, 2) == "1-") {
-HasEVEX512 = false;
+if (Feature.substr(1, 6) == "avx10.") {
+ if (Feature[0] == '+') {
+HasAVX10 = true;
+if (Feature.substr(Feature.size() - 3, 3) == "512")
+ HasAVX10_512 = true;
+LastAVX10 = Feature;
+ } else if (HasAVX10 && Feature == "-avx10.1-256") {
+HasAVX10 = false;
+HasAVX10_512 = false;
+ } else
[clang] [X86][AVX10] Permit AVX512 options/features used together with AVX10 (PR #71318)
https://github.com/phoebewang created
https://github.com/llvm/llvm-project/pull/71318
This patch relaxes the driver logic to permit combinations between AVX512 and
AVX10 options and makes sure we have a unified behavior between options and
features combination.
Here are rules we are following when handle these combinations:
1. evex512 can only be used for avx512xxx options/features. It will be ignored
if used without them;
2. avx512xxx and avx10.xxx are options in two worlds. Avoid to use them
together in any case. It will enable a common super set when they are used
together. E.g., "-mavx512f -mavx10.1-256" euqals "-mavx10.1-512".
Compiler emits warnings when user using combinations like "-mavx512f
-mavx10.1-256" in case they won't get unexpected result silently.
>From d9ee6309924e7f248695cbd488afe98273432e84 Mon Sep 17 00:00:00 2001
From: Phoebe Wang
Date: Sun, 5 Nov 2023 21:15:53 +0800
Subject: [PATCH] [X86][AVX10] Permit AVX512 options/features used together
with AVX10
This patch relaxes the driver logic to permit combinations between
AVX512 and AVX10 options and makes sure we have a unified behavior
between options and features combination.
Here are rules we are following when handle these combinations:
1. evex512 can only be used for avx512xxx options/features. It will be
ignored if used without them;
2. avx512xxx and avx10.xxx are options in two worlds. Avoid to use them
together in any case. It will enable a common super set when they are
used together. E.g., "-mavx512f -mavx10.1-256" euqals "-mavx10.1-512".
Compiler emits warnings when user using combinations like
"-mavx512f -mavx10.1-256" in case they won't get unexpected result silently.
---
.../clang/Basic/DiagnosticCommonKinds.td | 2 +
clang/lib/Basic/Targets/X86.cpp | 57 ---
clang/lib/Driver/ToolChains/Arch/X86.cpp | 7 ---
clang/lib/Headers/avx2intrin.h| 4 +-
clang/lib/Headers/avx512bf16intrin.h | 3 +-
clang/lib/Headers/avx512bwintrin.h| 4 +-
clang/lib/Headers/avx512dqintrin.h| 4 +-
clang/lib/Headers/avx512fintrin.h | 8 ++-
clang/lib/Headers/avx512fp16intrin.h | 6 +-
clang/lib/Headers/avx512ifmavlintrin.h| 10 +++-
clang/lib/Headers/avx512pfintrin.h| 5 --
clang/lib/Headers/avx512vbmivlintrin.h| 11 +++-
clang/lib/Headers/avx512vlbf16intrin.h| 14 +++--
clang/lib/Headers/avx512vlbitalgintrin.h | 10 +++-
clang/lib/Headers/avx512vlbwintrin.h | 10 +++-
clang/lib/Headers/avx512vlcdintrin.h | 11 +++-
clang/lib/Headers/avx512vldqintrin.h | 10 +++-
clang/lib/Headers/avx512vlfp16intrin.h| 4 +-
clang/lib/Headers/avx512vlintrin.h| 10 +++-
clang/lib/Headers/avx512vlvbmi2intrin.h | 10 +++-
clang/lib/Headers/avx512vlvnniintrin.h| 10 +++-
.../lib/Headers/avx512vlvp2intersectintrin.h | 10 ++--
clang/lib/Headers/avx512vpopcntdqvlintrin.h | 8 ++-
clang/lib/Headers/avxintrin.h | 4 +-
clang/lib/Headers/emmintrin.h | 4 +-
clang/lib/Headers/gfniintrin.h| 14 +++--
clang/lib/Headers/pmmintrin.h | 2 +-
clang/lib/Headers/smmintrin.h | 2 +-
clang/lib/Headers/tmmintrin.h | 4 +-
clang/lib/Headers/xmmintrin.h | 4 +-
clang/test/CodeGen/X86/avx512-error.c | 13 +
clang/test/CodeGen/target-avx-abi-diag.c | 28 -
clang/test/Driver/x86-target-features.c | 6 +-
33 files changed, 214 insertions(+), 95 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td
b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index 9f0ccd255a32148..8084a4ce0d1751b 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -346,6 +346,8 @@ def err_opt_not_valid_on_target : Error<
"option '%0' cannot be specified on this target">;
def err_invalid_feature_combination : Error<
"invalid feature combination: %0">;
+def warn_invalid_feature_combination : Warning<
+ "invalid feature combination: %0">,
InGroup>;
def warn_target_unrecognized_env : Warning<
"mismatch between architecture and environment in target triple '%0'; did
you mean '%1'?">,
InGroup;
diff --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index eec3cd558435e2a..9cfda95f385d627 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -119,9 +119,13 @@ bool X86TargetInfo::initFeatureMap(
setFeatureEnabled(Features, F, true);
std::vector UpdatedFeaturesVec;
- bool HasEVEX512 = true;
+ std::vector UpdatedAVX10FeaturesVec;
+ int HasEVEX512 = -1;
bool HasAVX512F = false;
bool HasAVX10 = false;
+ bool HasAVX10_512 = false;
+ std::string LastAVX10;
+ std::string LastAVX512;
for (const auto : FeaturesVec) {
// Expand general-regs-only
