[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
phoebewang wrote: > @phoebewang Can you add a release note for this on the PR for the release > branch and then add the release:note label. Done. https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
tstellar wrote: @phoebewang Can you add a release note for this on the PR for the release branch and then add the release:note label. https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
FireBurn wrote: No worries, it's fixed now, llvm, nodejs and libreoffice are all compiling fine again https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
phoebewang wrote: > You'll be probably building 18.1.6 with 18.1.5... and that's when you'll > notice the issue. I'm just about finished building 18.1.5 with your patch. So > hopefully all those issues will be gone. I'll report back Okay... This is a combined problem which I never thought before. Sorry for the inconvenience! https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
FireBurn wrote: You'll be probably building 18.1.6 with 18.1.5... and that's when you'll notice the issue. I'm just about finished building 18.1.5 with your patch. So hopefully all those issues will be gone. I'll report back https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
phoebewang wrote: @FireBurn Sorry, I just noticed #91719. The above comments were based on the issue #91076. I didn't look at the source, but the command line doesn't have an AVX512 option. So I'm assuming the code may related function multi-versioning with manually specified target attributes regarding to AVX512 features. The reason is the same as #91719, but the scenarios may be more common in practice. The defect in previous patch does expose a known issue with the design of EVEX512, which we have noted in https://clang.llvm.org/docs/UsersManual.html#x86 In a word, when user uses target attributes with AVX512 features, they should add an explicit `evex512` or `no-evex512` for robustness since LLVM 18. Missing it may result in some unexpected codegen or error in some corner case. https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
phoebewang wrote: > @tstellar Can a note be added somewhere about this? Folks upgrading to > llvm-18.1.6 will get errors unless they drop -march=native if they were on > 18.1.5 Although I agree we should add a note, I don't think this patch affects much. The problem only coccurs when combining `-march=native` with AVX512 options on a non AVX512 target. This actually is not a valid scenario. The compiled binary will contain AVX512 instruction which crashes it on the target. It violates the intention of using `-march=native`. I also don't see where the error from with 18.1.6. If there were errors, it should come from 18.1.5. https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
FireBurn wrote: @tstellar Can a note be added somewhere about this? Folks upgrading to llvm-18.1.6 will get errors unless they drop -march=native if they were on 18.1.5 https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
llvmbot wrote: /pull-request llvm/llvm-project#91705 https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
phoebewang wrote: /cherry-pick https://github.com/llvm/llvm-project/commit/87f3407856e61a73798af4e41b28bc33b5bf4ce6 https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
https://github.com/phoebewang milestoned https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
https://github.com/phoebewang closed https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
https://github.com/topperc approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
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 db78ee0cb82669302a5e0f18a15fd53346a73823 d50ae3b317839f5603b091aa029414b3c3c42259 -- llvm/lib/TargetParser/Host.cpp `` View the diff from clang-format here. ``diff diff --git a/llvm/lib/TargetParser/Host.cpp b/llvm/lib/TargetParser/Host.cpp index c5156c6cb8..ca053c8872 100644 --- a/llvm/lib/TargetParser/Host.cpp +++ b/llvm/lib/TargetParser/Host.cpp @@ -1803,7 +1803,7 @@ bool sys::getHostCPUFeatures(StringMap &Features) { // AVX512 is only supported if the OS supports the context save for it. Features["avx512f"]= HasLeaf7 && ((EBX >> 16) & 1) && HasAVX512Save; if (Features["avx512f"]) -Features["evex512"] = true; +Features["evex512"] = true; Features["avx512dq"] = HasLeaf7 && ((EBX >> 17) & 1) && HasAVX512Save; Features["rdseed"] = HasLeaf7 && ((EBX >> 18) & 1); Features["adx"]= HasLeaf7 && ((EBX >> 19) & 1); `` https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
phoebewang wrote: > Could you make getHostCPUFeatures only write the Features["evex512"] entry if > Features["avx512f"] is true? Instead of copying Features["avx512f"]? Good idea, done. https://github.com/llvm/llvm-project/pull/91694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 (PR #91694)
https://github.com/phoebewang updated https://github.com/llvm/llvm-project/pull/91694 >From 6ea15aa0ee6726cad8dccff10155f058afa0f013 Mon Sep 17 00:00:00 2001 From: Phoebe Wang Date: Fri, 10 May 2024 11:21:41 +0800 Subject: [PATCH 1/2] [X86][Driver] Do not add `-evex512` for `-march=native` when the target doesn't support AVX512 Users want `-march=sandybridge -mavx512f -mavx512vl` behaves the same as `-march=native -mavx512f -mavx512vl` on a sandybridge target. Fixes: #91076 --- clang/lib/Driver/ToolChains/Arch/X86.cpp | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp b/clang/lib/Driver/ToolChains/Arch/X86.cpp index 53e26a9f8e229..c6d97ef4e971a 100644 --- a/clang/lib/Driver/ToolChains/Arch/X86.cpp +++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp @@ -132,10 +132,14 @@ void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple, if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) { if (StringRef(A->getValue()) == "native") { llvm::StringMap HostFeatures; - if (llvm::sys::getHostCPUFeatures(HostFeatures)) -for (auto &F : HostFeatures) + if (llvm::sys::getHostCPUFeatures(HostFeatures)) { +for (auto &F : HostFeatures) { + if (!F.second && F.first() == "evex512") +continue; Features.push_back( Args.MakeArgString((F.second ? "+" : "-") + F.first())); +} + } } } >From d50ae3b317839f5603b091aa029414b3c3c42259 Mon Sep 17 00:00:00 2001 From: Phoebe Wang Date: Fri, 10 May 2024 12:02:09 +0800 Subject: [PATCH 2/2] Address review comment --- clang/lib/Driver/ToolChains/Arch/X86.cpp | 8 ++-- llvm/lib/TargetParser/Host.cpp | 3 ++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp b/clang/lib/Driver/ToolChains/Arch/X86.cpp index c6d97ef4e971a..53e26a9f8e229 100644 --- a/clang/lib/Driver/ToolChains/Arch/X86.cpp +++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp @@ -132,14 +132,10 @@ void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple, if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) { if (StringRef(A->getValue()) == "native") { llvm::StringMap HostFeatures; - if (llvm::sys::getHostCPUFeatures(HostFeatures)) { -for (auto &F : HostFeatures) { - if (!F.second && F.first() == "evex512") -continue; + if (llvm::sys::getHostCPUFeatures(HostFeatures)) +for (auto &F : HostFeatures) Features.push_back( Args.MakeArgString((F.second ? "+" : "-") + F.first())); -} - } } } diff --git a/llvm/lib/TargetParser/Host.cpp b/llvm/lib/TargetParser/Host.cpp index 834f4536f93ac..c5156c6cb802c 100644 --- a/llvm/lib/TargetParser/Host.cpp +++ b/llvm/lib/TargetParser/Host.cpp @@ -1802,7 +1802,8 @@ bool sys::getHostCPUFeatures(StringMap &Features) { Features["rtm"]= HasLeaf7 && ((EBX >> 11) & 1); // AVX512 is only supported if the OS supports the context save for it. Features["avx512f"]= HasLeaf7 && ((EBX >> 16) & 1) && HasAVX512Save; - Features["evex512"]= Features["avx512f"]; + if (Features["avx512f"]) +Features["evex512"] = true; Features["avx512dq"] = HasLeaf7 && ((EBX >> 17) & 1) && HasAVX512Save; Features["rdseed"] = HasLeaf7 && ((EBX >> 18) & 1); Features["adx"]= HasLeaf7 && ((EBX >> 19) & 1); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits