[clang] [llvm] [Driver,CodeGen] Report error when enabling 64-bit-only features on non-64-bit arch (PR #101151)
Shengchen Kan via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 30 19:31:49 PDT 2024
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.kan at intel.com>
Date: Tue, 30 Jul 2024 16:26:39 +0800
Subject: [PATCH 1/5] [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.kan at intel.com>
Date: Tue, 30 Jul 2024 16:47:25 +0800
Subject: [PATCH 2/5] 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.kan at intel.com>
Date: Tue, 30 Jul 2024 17:09:32 +0800
Subject: [PATCH 3/5] 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.kan at intel.com>
Date: Wed, 31 Jul 2024 10:11:20 +0800
Subject: [PATCH 4/5] 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"
>From 07966487a37d843ea6b004e7bc6090b2d3ba5c3b Mon Sep 17 00:00:00 2001
From: Shengchen Kan <shengchen.kan at intel.com>
Date: Wed, 31 Jul 2024 10:31:33 +0800
Subject: [PATCH 5/5] simplify
---
clang/lib/Driver/ToolChains/Arch/X86.cpp | 33 ++++++++++--------------
clang/test/Driver/x86-target-features.c | 2 ++
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp b/clang/lib/Driver/ToolChains/Arch/X86.cpp
index ef2c748694f09..dc6c8695488bb 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -248,10 +248,6 @@ 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,
@@ -272,28 +268,27 @@ 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))
+ if (Not64Bit && Name == "uintr")
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)) {
+ if (Not64Bit && !IsNegative)
+ D.Diag(diag::err_drv_unsupported_opt_for_target)
+ << StringRef(A->getSpelling().str() + "|-mapxf")
+ << Triple.getTriple();
+
for (StringRef Value : A->getValues()) {
- if (SubFeaturesOfAPX.contains(Value)) {
- if (Not64Bit && FeaturesIn64BitOnly.contains(Value)) {
- std::string ArgOrAlias = A->getSpelling().str() + "|-mapxf";
- D.Diag(diag::err_drv_unsupported_opt_for_target)
- << ArgOrAlias << Triple.getTriple();
- break;
- }
-
- Features.push_back(
- Args.MakeArgString((IsNegative ? "-" : "+") + Value));
- continue;
- }
- D.Diag(clang::diag::err_drv_unsupported_option_argument)
- << A->getSpelling() << Value;
+ if (Value != "egpr" && Value != "push2pop2" && Value != "ppx" &&
+ Value != "ndd" && Value != "ccmp" && Value != "nf" &&
+ Value != "cf" && Value != "zu")
+ D.Diag(clang::diag::err_drv_unsupported_option_argument)
+ << A->getSpelling() << Value;
+
+ 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 99f81a869f0ee..7d77ae75f8c47 100644
--- a/clang/test/Driver/x86-target-features.c
+++ b/clang/test/Driver/x86-target-features.c
@@ -410,8 +410,10 @@
// 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: %clang -### --target=i386 -mno-uintr %s 2>&1 > /dev/null
// 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
+// RUN: %clang -### --target=i386 -mno-apxf %s 2>&1 > /dev/null
// NON-UINTR: error: unsupported option '-muintr' for target 'i386'
// NON-APX: error: unsupported option '-mapx-features=|-mapxf' for target 'i386'
// NON-APX-NOT: error: {{.*}} -mapx-features=
More information about the llvm-commits
mailing list