https://github.com/KanRobert updated https://github.com/llvm/llvm-project/pull/101151
>From 650f29dd40714ebe52bf5d0a407bd45b9d248269 Mon Sep 17 00:00:00 2001 From: Shengchen Kan <shengchen....@intel.com> Date: Tue, 30 Jul 2024 16:26:39 +0800 Subject: [PATCH 1/4] [Driver,CodeGen] Report error when enabling 64-bit-only features on non-64-bit arch In front-end, now we detect for `-mapx-features=/-mapxf` and `-muintr`, which is aligned with GCC https://gcc.gnu.org/bugzilla/attachment.cgi?id=58698&action=diff In backend, we just disable these 64-bit-only features silently, so that there is no error for `-march=native -m32` on APX-supported arch. llvm-issue: https://github.com/llvm/llvm-project/issues/94810 GCC thread: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115978 --- clang/lib/Driver/ToolChains/Arch/X86.cpp | 18 ++++++++++++--- clang/test/Driver/x86-target-features.c | 9 ++++++-- llvm/lib/Target/X86/X86Subtarget.cpp | 7 ++++++ llvm/test/CodeGen/X86/apx/i386-ndd.ll | 28 ++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 llvm/test/CodeGen/X86/apx/i386-ndd.ll diff --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp b/clang/lib/Driver/ToolChains/Arch/X86.cpp index 2f63333b732f6..1031898a155cf 100644 --- a/clang/lib/Driver/ToolChains/Arch/X86.cpp +++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp @@ -248,6 +248,10 @@ void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple, Features.push_back(Args.MakeArgString((IsNegative ? "-" : "+") + Name)); } + llvm::StringSet<> SubFeaturesOfAPX = {"egpr", "push2pop2", "ppx", "ndd", + "ccmp", "nf", "cf", "zu"}; + llvm::StringSet<> FeaturesIn64BitOnly = {"uintr"}; + FeaturesIn64BitOnly.insert(SubFeaturesOfAPX.begin(), SubFeaturesOfAPX.end()); // Now add any that the user explicitly requested on the command line, // which may override the defaults. for (const Arg *A : Args.filtered(options::OPT_m_x86_Features_Group, @@ -266,13 +270,21 @@ void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple, } bool IsNegative = Name.starts_with("no-"); + + bool Not64Bit = ArchType != llvm::Triple::x86_64; + if (Not64Bit && FeaturesIn64BitOnly.contains(Name)) + D.Diag(diag::err_drv_unsupported_opt_for_target) + << A->getSpelling() << Triple.getTriple(); + if (A->getOption().matches(options::OPT_mapx_features_EQ) || A->getOption().matches(options::OPT_mno_apx_features_EQ)) { for (StringRef Value : A->getValues()) { - if (Value == "egpr" || Value == "push2pop2" || Value == "ppx" || - Value == "ndd" || Value == "ccmp" || Value == "nf" || - Value == "cf" || Value == "zu") { + if (SubFeaturesOfAPX.contains(Value)) { + if (Not64Bit && FeaturesIn64BitOnly.contains(Value)) + D.Diag(diag::err_drv_unsupported_opt_for_target) + << A->getSpelling() << Triple.getTriple(); + Features.push_back( Args.MakeArgString((IsNegative ? "-" : "+") + Value)); continue; diff --git a/clang/test/Driver/x86-target-features.c b/clang/test/Driver/x86-target-features.c index 63237cbc3e7ef..a577419f8148e 100644 --- a/clang/test/Driver/x86-target-features.c +++ b/clang/test/Driver/x86-target-features.c @@ -309,8 +309,8 @@ // HRESET: "-target-feature" "+hreset" // NO-HRESET: "-target-feature" "-hreset" -// RUN: %clang --target=i386 -march=i386 -muintr %s -### 2>&1 | FileCheck -check-prefix=UINTR %s -// RUN: %clang --target=i386 -march=i386 -mno-uintr %s -### 2>&1 | FileCheck -check-prefix=NO-UINTR %s +// RUN: %clang --target=x86_64 -march=i386 -muintr %s -### 2>&1 | FileCheck -check-prefix=UINTR %s +// RUN: %clang --target=x86_64 -march=i386 -mno-uintr %s -### 2>&1 | FileCheck -check-prefix=NO-UINTR %s // UINTR: "-target-feature" "+uintr" // NO-UINTR: "-target-feature" "-uintr" @@ -409,6 +409,11 @@ // NONX86-NEXT: warning: argument unused during compilation: '-msse4.2' [-Wunused-command-line-argument] // NONX86-NEXT: error: unsupported option '-mno-sgx' for target 'aarch64' +// RUN: not %clang -### --target=i386 -muintr %s 2>&1 | FileCheck --check-prefix=NON-UINTR %s +// RUN: not %clang -### --target=i386 -mapx-features=ndd %s 2>&1 | FileCheck --check-prefix=NON-APX %s +// NON-UINTR: error: unsupported option '-muintr' for target 'i386' +// NON-APX: error: unsupported option '-mapx-features=' for target 'i386' + // RUN: %clang --target=i386 -march=i386 -mharden-sls=return %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=SLS-RET,NO-SLS %s // RUN: %clang --target=i386 -march=i386 -mharden-sls=indirect-jmp %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=SLS-IJMP,NO-SLS %s // RUN: %clang --target=i386 -march=i386 -mharden-sls=none -mharden-sls=all %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=SLS-IJMP,SLS-RET %s diff --git a/llvm/lib/Target/X86/X86Subtarget.cpp b/llvm/lib/Target/X86/X86Subtarget.cpp index 4e8e04b1112c0..7d5af2ede99cf 100644 --- a/llvm/lib/Target/X86/X86Subtarget.cpp +++ b/llvm/lib/Target/X86/X86Subtarget.cpp @@ -279,6 +279,13 @@ void X86Subtarget::initSubtargetFeatures(StringRef CPU, StringRef TuneCPU, FullFS += ",+evex512"; } + // Disable 64-bit only features in non-64-bit mode. + SmallVector<StringRef, 9> FeaturesIn64BitOnly = { + "egpr", "push2pop2", "ppx", "ndd", "ccmp", "nf", "cf", "zu", "uintr"}; + if (FullFS.find("-64bit-mode") != std::string::npos) + llvm::for_each(FeaturesIn64BitOnly, + [&](StringRef F) { FullFS += ",-" + F.str(); }); + // Parse features string and set the CPU. ParseSubtargetFeatures(CPU, TuneCPU, FullFS); diff --git a/llvm/test/CodeGen/X86/apx/i386-ndd.ll b/llvm/test/CodeGen/X86/apx/i386-ndd.ll new file mode 100644 index 0000000000000..b229798183166 --- /dev/null +++ b/llvm/test/CodeGen/X86/apx/i386-ndd.ll @@ -0,0 +1,28 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5 +; RUN: llc -mtriple=i386-linux-gnu -mattr=+cmov,+ndd < %s | FileCheck %s +define i64 @test(i64 %x, ptr %random) { +; CHECK-LABEL: test: +; CHECK: # %bb.0: # %entry +; CHECK-NEXT: pushl %esi +; CHECK-NEXT: .cfi_def_cfa_offset 8 +; CHECK-NEXT: .cfi_offset %esi, -8 +; CHECK-NEXT: movl {{[0-9]+}}(%esp), %eax +; CHECK-NEXT: movl {{[0-9]+}}(%esp), %edx +; CHECK-NEXT: movl {{[0-9]+}}(%esp), %ecx +; CHECK-NEXT: movzbl (%ecx), %ecx +; CHECK-NEXT: movl %eax, %esi +; CHECK-NEXT: shll %cl, %esi +; CHECK-NEXT: shldl %cl, %eax, %edx +; CHECK-NEXT: xorl %eax, %eax +; CHECK-NEXT: testb $32, %cl +; CHECK-NEXT: cmovnel %esi, %edx +; CHECK-NEXT: cmovel %esi, %eax +; CHECK-NEXT: popl %esi +; CHECK-NEXT: .cfi_def_cfa_offset 4 +; CHECK-NEXT: retl +entry: + %0 = load i32, ptr %random + %sh_prom = zext nneg i32 %0 to i64 + %shl = shl i64 %x, %sh_prom + ret i64 %shl +} >From 6f4b2cde6445f2a9a0a618a8823df6f2bddccaa5 Mon Sep 17 00:00:00 2001 From: Shengchen Kan <shengchen....@intel.com> Date: Tue, 30 Jul 2024 16:47:25 +0800 Subject: [PATCH 2/4] simplify test --- llvm/test/CodeGen/X86/apx/i386-ndd.ll | 29 ++++++++------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/llvm/test/CodeGen/X86/apx/i386-ndd.ll b/llvm/test/CodeGen/X86/apx/i386-ndd.ll index b229798183166..146a99340eb66 100644 --- a/llvm/test/CodeGen/X86/apx/i386-ndd.ll +++ b/llvm/test/CodeGen/X86/apx/i386-ndd.ll @@ -1,28 +1,15 @@ ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5 ; RUN: llc -mtriple=i386-linux-gnu -mattr=+cmov,+ndd < %s | FileCheck %s -define i64 @test(i64 %x, ptr %random) { +define i32 @test(i1 %cmp, i32 %x, i32 %y) nounwind { ; CHECK-LABEL: test: ; CHECK: # %bb.0: # %entry -; CHECK-NEXT: pushl %esi -; CHECK-NEXT: .cfi_def_cfa_offset 8 -; CHECK-NEXT: .cfi_offset %esi, -8 -; CHECK-NEXT: movl {{[0-9]+}}(%esp), %eax -; CHECK-NEXT: movl {{[0-9]+}}(%esp), %edx -; CHECK-NEXT: movl {{[0-9]+}}(%esp), %ecx -; CHECK-NEXT: movzbl (%ecx), %ecx -; CHECK-NEXT: movl %eax, %esi -; CHECK-NEXT: shll %cl, %esi -; CHECK-NEXT: shldl %cl, %eax, %edx -; CHECK-NEXT: xorl %eax, %eax -; CHECK-NEXT: testb $32, %cl -; CHECK-NEXT: cmovnel %esi, %edx -; CHECK-NEXT: cmovel %esi, %eax -; CHECK-NEXT: popl %esi -; CHECK-NEXT: .cfi_def_cfa_offset 4 +; CHECK-NEXT: testb $1, {{[0-9]+}}(%esp) +; CHECK-NEXT: leal {{[0-9]+}}(%esp), %eax +; CHECK-NEXT: leal {{[0-9]+}}(%esp), %ecx +; CHECK-NEXT: cmovnel %eax, %ecx +; CHECK-NEXT: movl (%ecx), %eax ; CHECK-NEXT: retl entry: - %0 = load i32, ptr %random - %sh_prom = zext nneg i32 %0 to i64 - %shl = shl i64 %x, %sh_prom - ret i64 %shl + %cmov = select i1 %cmp, i32 %x, i32 %y + ret i32 %cmov } >From fd9a08bca9805ba8f1963c59dc2ff7b627c960f1 Mon Sep 17 00:00:00 2001 From: Shengchen Kan <shengchen....@intel.com> Date: Tue, 30 Jul 2024 17:09:32 +0800 Subject: [PATCH 3/4] improve error message --- clang/lib/Driver/ToolChains/Arch/X86.cpp | 7 +++++-- clang/test/Driver/x86-target-features.c | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp b/clang/lib/Driver/ToolChains/Arch/X86.cpp index 1031898a155cf..ef2c748694f09 100644 --- a/clang/lib/Driver/ToolChains/Arch/X86.cpp +++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp @@ -281,9 +281,12 @@ void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple, for (StringRef Value : A->getValues()) { if (SubFeaturesOfAPX.contains(Value)) { - if (Not64Bit && FeaturesIn64BitOnly.contains(Value)) + if (Not64Bit && FeaturesIn64BitOnly.contains(Value)) { + std::string ArgOrAlias = A->getSpelling().str() + "|-mapxf"; D.Diag(diag::err_drv_unsupported_opt_for_target) - << A->getSpelling() << Triple.getTriple(); + << ArgOrAlias << Triple.getTriple(); + break; + } Features.push_back( Args.MakeArgString((IsNegative ? "-" : "+") + Value)); diff --git a/clang/test/Driver/x86-target-features.c b/clang/test/Driver/x86-target-features.c index a577419f8148e..f2dd89349ac91 100644 --- a/clang/test/Driver/x86-target-features.c +++ b/clang/test/Driver/x86-target-features.c @@ -411,8 +411,10 @@ // RUN: not %clang -### --target=i386 -muintr %s 2>&1 | FileCheck --check-prefix=NON-UINTR %s // RUN: not %clang -### --target=i386 -mapx-features=ndd %s 2>&1 | FileCheck --check-prefix=NON-APX %s +// RUN: not %clang -### --target=i386 -mapxf %s 2>&1 | FileCheck --check-prefix=NON-APX %s // NON-UINTR: error: unsupported option '-muintr' for target 'i386' -// NON-APX: error: unsupported option '-mapx-features=' for target 'i386' +// NON-APX: error: unsupported option '-mapx-features=|-mapxf' for target 'i386' +// NON-APX-NOT: error: {{.*}} -mapx-features= // RUN: %clang --target=i386 -march=i386 -mharden-sls=return %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=SLS-RET,NO-SLS %s // RUN: %clang --target=i386 -march=i386 -mharden-sls=indirect-jmp %s -### -o %t.o 2>&1 | FileCheck -check-prefixes=SLS-IJMP,NO-SLS %s >From 1ff825bf582af1d1efccb0b214f1028069f67308 Mon Sep 17 00:00:00 2001 From: Shengchen Kan <shengchen....@intel.com> Date: Wed, 31 Jul 2024 10:11:20 +0800 Subject: [PATCH 4/4] address review comment --- clang/test/Driver/x86-target-features.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/Driver/x86-target-features.c b/clang/test/Driver/x86-target-features.c index f2dd89349ac91..99f81a869f0ee 100644 --- a/clang/test/Driver/x86-target-features.c +++ b/clang/test/Driver/x86-target-features.c @@ -309,8 +309,8 @@ // HRESET: "-target-feature" "+hreset" // NO-HRESET: "-target-feature" "-hreset" -// RUN: %clang --target=x86_64 -march=i386 -muintr %s -### 2>&1 | FileCheck -check-prefix=UINTR %s -// RUN: %clang --target=x86_64 -march=i386 -mno-uintr %s -### 2>&1 | FileCheck -check-prefix=NO-UINTR %s +// RUN: %clang --target=x86_64 -muintr %s -### 2>&1 | FileCheck -check-prefix=UINTR %s +// RUN: %clang --target=x86_64 -mno-uintr %s -### 2>&1 | FileCheck -check-prefix=NO-UINTR %s // UINTR: "-target-feature" "+uintr" // NO-UINTR: "-target-feature" "-uintr" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
