[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 &Feature : 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 == "-avx
[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 &Feature : 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 == "-avx
[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 l
[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 &Feature : 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; + }
[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 &Feature : FeaturesVec) { // Expand general-reg