[llvm] [AArch64][AsmParser] Directives should clear transitively implied features (PR #106625)
Alexandros Lamprineas via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 30 04:39:39 PDT 2024
https://github.com/labrinea updated https://github.com/llvm/llvm-project/pull/106625
>From da0400f2ae18aee9e7372599515a62a9b2a44720 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Thu, 29 Aug 2024 21:22:17 +0100
Subject: [PATCH 1/2] [AArch64][AsmParser] Directives should clear transitively
implied features.
The commit ff3f3a54e2d1 made it possible to enable transitively implied
features when parsing assembler directives. For example enabling sve2
also enables sve.
This patch allows disabling features which depend on each other. For
example disabling sve also disables sve2.
---
.../AArch64/AsmParser/AArch64AsmParser.cpp | 37 ++++++++++---------
.../MC/AArch64/SVE/directive-arch-negative.s | 8 ++++
.../SVE/directive-arch_extension-negative.s | 7 +++-
.../MC/AArch64/SVE/directive-cpu-negative.s | 7 +++-
.../directive-arch_extension-negative.s | 12 ++++++
5 files changed, 51 insertions(+), 20 deletions(-)
create mode 100644 llvm/test/MC/AArch64/SVE/directive-arch-negative.s
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 37add682b150e7..12241b6b3f4eb7 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -6991,11 +6991,12 @@ bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
if (Extension.Features.none())
report_fatal_error("unsupported architectural extension: " + Name);
- FeatureBitset ToggleFeatures =
- EnableFeature
- ? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
- : STI.ToggleFeature(Features & Extension.Features);
- setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
+ if (EnableFeature)
+ STI.SetFeatureBitsTransitively(Extension.Features);
+ else
+ STI.ClearFeatureBitsTransitively(Extension.Features);
+ FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
+ setAvailableFeatures(Features);
break;
}
}
@@ -7018,8 +7019,6 @@ bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) {
Name = Name.substr(2);
}
- MCSubtargetInfo &STI = copySTI();
- FeatureBitset Features = STI.getFeatureBits();
for (const auto &Extension : ExtensionMap) {
if (Extension.Name != Name)
continue;
@@ -7027,11 +7026,13 @@ bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) {
if (Extension.Features.none())
return Error(ExtLoc, "unsupported architectural extension: " + Name);
- FeatureBitset ToggleFeatures =
- EnableFeature
- ? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
- : STI.ToggleFeature(Features & Extension.Features);
- setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
+ MCSubtargetInfo &STI = copySTI();
+ if (EnableFeature)
+ STI.SetFeatureBitsTransitively(Extension.Features);
+ else
+ STI.ClearFeatureBitsTransitively(Extension.Features);
+ FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
+ setAvailableFeatures(Features);
return false;
}
@@ -7083,12 +7084,12 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) {
if (Extension.Features.none())
report_fatal_error("unsupported architectural extension: " + Name);
- FeatureBitset Features = STI.getFeatureBits();
- FeatureBitset ToggleFeatures =
- EnableFeature
- ? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
- : STI.ToggleFeature(Features & Extension.Features);
- setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
+ if (EnableFeature)
+ STI.SetFeatureBitsTransitively(Extension.Features);
+ else
+ STI.ClearFeatureBitsTransitively(Extension.Features);
+ FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
+ setAvailableFeatures(Features);
FoundExtension = true;
break;
diff --git a/llvm/test/MC/AArch64/SVE/directive-arch-negative.s b/llvm/test/MC/AArch64/SVE/directive-arch-negative.s
new file mode 100644
index 00000000000000..e3029c16ffc8a6
--- /dev/null
+++ b/llvm/test/MC/AArch64/SVE/directive-arch-negative.s
@@ -0,0 +1,8 @@
+// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
+
+// Check that setting +nosve implies +nosve2
+.arch armv9-a+nosve
+
+adclb z0.s, z1.s, z31.s
+// CHECK: error: instruction requires: sve2
+// CHECK-NEXT: adclb z0.s, z1.s, z31.s
diff --git a/llvm/test/MC/AArch64/SVE/directive-arch_extension-negative.s b/llvm/test/MC/AArch64/SVE/directive-arch_extension-negative.s
index 661f13974d0bc8..31118f7490d00d 100644
--- a/llvm/test/MC/AArch64/SVE/directive-arch_extension-negative.s
+++ b/llvm/test/MC/AArch64/SVE/directive-arch_extension-negative.s
@@ -1,7 +1,12 @@
// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
-.arch_extension nosve
+.arch_extension sve2+nosve
ptrue p0.b, pow2
// CHECK: error: instruction requires: sve or sme
// CHECK-NEXT: ptrue p0.b, pow2
+
+// Check that setting +nosve implies +nosve2
+adclb z0.s, z1.s, z31.s
+// CHECK: error: instruction requires: sve2
+// CHECK-NEXT: adclb z0.s, z1.s, z31.s
diff --git a/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s b/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s
index 82acc1b0b0be9b..6ba537ca70609e 100644
--- a/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s
+++ b/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s
@@ -1,6 +1,11 @@
// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
-.cpu generic+sve+nosve
+.cpu generic+sve2+nosve
ptrue p0.b, pow2
// CHECK: error: instruction requires: sve or sme
// CHECK-NEXT: ptrue p0.b, pow2
+
+// Check that setting +nosve implies +nosve2
+adclb z0.s, z1.s, z31.s
+// CHECK: error: instruction requires: sve2
+// CHECK-NEXT: adclb z0.s, z1.s, z31.s
diff --git a/llvm/test/MC/AArch64/directive-arch_extension-negative.s b/llvm/test/MC/AArch64/directive-arch_extension-negative.s
index 1c1cfc9d33e3ed..d8f7fa1d7b791b 100644
--- a/llvm/test/MC/AArch64/directive-arch_extension-negative.s
+++ b/llvm/test/MC/AArch64/directive-arch_extension-negative.s
@@ -49,6 +49,8 @@ fminnm d0, d0, d1
// CHECK: [[@LINE-1]]:1: error: instruction requires: fp
// CHECK-NEXT: fminnm d0, d0, d1
+// nofp implied nosimd, so reinstate it
+.arch_extension simd
addp v0.4s, v0.4s, v0.4s
// CHECK-NOT: [[@LINE-1]]:1: error: instruction requires: neon
.arch_extension nosimd
@@ -70,6 +72,8 @@ casa w5, w7, [x20]
// CHECK: [[@LINE-1]]:1: error: instruction requires: lse
// CHECK-NEXT: casa w5, w7, [x20]
+// nolse implied nolse128, so reinstate it
+.arch_extension lse128
swpp x0, x2, [x3]
// CHECK-NOT: [[@LINE-1]]:1: error: instruction requires: lse128
.arch_extension nolse128
@@ -84,6 +88,8 @@ cfp rctx, x0
// CHECK: [[@LINE-1]]:5: error: CFPRCTX requires: predres
// CHECK-NEXT: cfp rctx, x0
+// nopredres implied nopredres2, so reinstate it
+.arch_extension predres2
cosp rctx, x0
// CHECK-NOT: [[@LINE-1]]:6: error: COSP requires: predres2
.arch_extension nopredres2
@@ -133,6 +139,8 @@ ldapr x0, [x1]
// CHECK: [[@LINE-1]]:1: error: instruction requires: rcpc
// CHECK-NEXT: ldapr x0, [x1]
+// norcpc implied norcpc3, so reinstate it
+.arch_extension rcpc3
stilp w24, w0, [x16, #-8]!
// CHECK-NOT: [[@LINE-1]]:1: error: instruction requires: rcpc3
.arch_extension norcpc3
@@ -169,6 +177,8 @@ cpyfp [x0]!, [x1]!, x2!
// CHECK: [[@LINE-1]]:1: error: instruction requires: mops
// CHECK-NEXT: cpyfp [x0]!, [x1]!, x2!
+// nolse128 implied nod128, so reinstate it
+.arch_extension d128
// This needs to come before `.arch_extension nothe` as it uses an instruction
// that requires both the and d128
sysp #0, c2, c0, #0, x0, x1
@@ -204,6 +214,8 @@ umax x0, x1, x2
// CHECK: [[@LINE-1]]:1: error: instruction requires: cssc
// CHECK-NEXT: umax x0, x1, x2
+// noras implied norasv2, so reinstate it
+.arch_extension rasv2
mrs x0, ERXGSR_EL1
// CHECK-NOT: [[@LINE-1]]:9: error: expected readable system register
.arch_extension norasv2
>From daca0cf965f082076c724510f91e877825fc22e4 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Fri, 30 Aug 2024 12:33:44 +0100
Subject: [PATCH 2/2] Changes from last revision:
* replaced the nested loop over ExtensionMap with find_if
* replaced report_fatal_error with Error to properly diagnose
unsupported extensions
* updated the location info when diagnosing the arch directive
* adjusted two tests accordingly
---
.../AArch64/AsmParser/AArch64AsmParser.cpp | 100 ++++++++----------
.../test/MC/AArch64/directive-arch-negative.s | 5 +-
.../directive-arch_extension-negative.s | 2 +-
3 files changed, 49 insertions(+), 58 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
index 12241b6b3f4eb7..5b9c5b6c20e01c 100644
--- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
+++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -6947,10 +6947,14 @@ static void ExpandCryptoAEK(const AArch64::ArchInfo &ArchInfo,
}
}
+static SMLoc incrementLoc(SMLoc L, int Offset) {
+ return SMLoc::getFromPointer(L.getPointer() + Offset);
+}
+
/// parseDirectiveArch
/// ::= .arch token
bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
- SMLoc ArchLoc = getLoc();
+ SMLoc CurLoc = getLoc();
StringRef Arch, ExtensionString;
std::tie(Arch, ExtensionString) =
@@ -6958,7 +6962,7 @@ bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
const AArch64::ArchInfo *ArchInfo = AArch64::parseArch(Arch);
if (!ArchInfo)
- return Error(ArchLoc, "unknown arch name");
+ return Error(CurLoc, "unknown arch name");
if (parseToken(AsmToken::EndOfStatement))
return true;
@@ -6978,28 +6982,29 @@ bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
ExtensionString.split(RequestedExtensions, '+');
ExpandCryptoAEK(*ArchInfo, RequestedExtensions);
+ CurLoc = incrementLoc(CurLoc, Arch.size());
- FeatureBitset Features = STI.getFeatureBits();
- setAvailableFeatures(ComputeAvailableFeatures(Features));
for (auto Name : RequestedExtensions) {
+ // Advance source location past '+'.
+ CurLoc = incrementLoc(CurLoc, 1);
+
bool EnableFeature = !Name.consume_front_insensitive("no");
- for (const auto &Extension : ExtensionMap) {
- if (Extension.Name != Name)
- continue;
+ auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
+ return Extension.Name == Name; });
- if (Extension.Features.none())
- report_fatal_error("unsupported architectural extension: " + Name);
+ if (It == std::end(ExtensionMap))
+ Error(CurLoc, "unsupported architectural extension: " + Name);
- if (EnableFeature)
- STI.SetFeatureBitsTransitively(Extension.Features);
- else
- STI.ClearFeatureBitsTransitively(Extension.Features);
- FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
- setAvailableFeatures(Features);
- break;
- }
+ if (EnableFeature)
+ STI.SetFeatureBitsTransitively(It->Features);
+ else
+ STI.ClearFeatureBitsTransitively(It->Features);
+
+ CurLoc = incrementLoc(CurLoc, Name.size());
}
+ FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
+ setAvailableFeatures(Features);
return false;
}
@@ -7019,28 +7024,20 @@ bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) {
Name = Name.substr(2);
}
- for (const auto &Extension : ExtensionMap) {
- if (Extension.Name != Name)
- continue;
+ auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
+ return Extension.Name == Name; });
- if (Extension.Features.none())
- return Error(ExtLoc, "unsupported architectural extension: " + Name);
+ if (It == std::end(ExtensionMap))
+ return Error(ExtLoc, "unsupported architectural extension: " + Name);
- MCSubtargetInfo &STI = copySTI();
- if (EnableFeature)
- STI.SetFeatureBitsTransitively(Extension.Features);
- else
- STI.ClearFeatureBitsTransitively(Extension.Features);
- FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
- setAvailableFeatures(Features);
- return false;
- }
-
- return Error(ExtLoc, "unknown architectural extension: " + Name);
-}
-
-static SMLoc incrementLoc(SMLoc L, int Offset) {
- return SMLoc::getFromPointer(L.getPointer() + Offset);
+ MCSubtargetInfo &STI = copySTI();
+ if (EnableFeature)
+ STI.SetFeatureBitsTransitively(It->Features);
+ else
+ STI.ClearFeatureBitsTransitively(It->Features);
+ FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
+ setAvailableFeatures(Features);
+ return false;
}
/// parseDirectiveCPU
@@ -7076,30 +7073,21 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) {
bool EnableFeature = !Name.consume_front_insensitive("no");
- bool FoundExtension = false;
- for (const auto &Extension : ExtensionMap) {
- if (Extension.Name != Name)
- continue;
-
- if (Extension.Features.none())
- report_fatal_error("unsupported architectural extension: " + Name);
-
- if (EnableFeature)
- STI.SetFeatureBitsTransitively(Extension.Features);
- else
- STI.ClearFeatureBitsTransitively(Extension.Features);
- FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
- setAvailableFeatures(Features);
- FoundExtension = true;
+ auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
+ return Extension.Name == Name; });
- break;
- }
+ if (It == std::end(ExtensionMap))
+ Error(CurLoc, "unsupported architectural extension: " + Name);
- if (!FoundExtension)
- Error(CurLoc, "unsupported architectural extension");
+ if (EnableFeature)
+ STI.SetFeatureBitsTransitively(It->Features);
+ else
+ STI.ClearFeatureBitsTransitively(It->Features);
CurLoc = incrementLoc(CurLoc, Name.size());
}
+ FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
+ setAvailableFeatures(Features);
return false;
}
diff --git a/llvm/test/MC/AArch64/directive-arch-negative.s b/llvm/test/MC/AArch64/directive-arch-negative.s
index f60759899aa6c9..406507d5fc8f4d 100644
--- a/llvm/test/MC/AArch64/directive-arch-negative.s
+++ b/llvm/test/MC/AArch64/directive-arch-negative.s
@@ -12,10 +12,13 @@
# CHECK-NEXT: aese v0.8h, v1.8h
# CHECK-NEXT: ^
-// We silently ignore invalid features.
.arch armv8+foo
aese v0.8h, v1.8h
+# CHECK: error: unsupported architectural extension: foo
+# CHECK-NEXT: .arch armv8+foo
+# CHECK-NEXT: ^
+
# CHECK: error: invalid operand for instruction
# CHECK-NEXT: aese v0.8h, v1.8h
# CHECK-NEXT: ^
diff --git a/llvm/test/MC/AArch64/directive-arch_extension-negative.s b/llvm/test/MC/AArch64/directive-arch_extension-negative.s
index d8f7fa1d7b791b..1843af56555461 100644
--- a/llvm/test/MC/AArch64/directive-arch_extension-negative.s
+++ b/llvm/test/MC/AArch64/directive-arch_extension-negative.s
@@ -4,7 +4,7 @@
// RUN: -filetype asm -o - %s 2>&1 | FileCheck %s
.arch_extension axp64
-// CHECK: error: unknown architectural extension: axp64
+// CHECK: error: unsupported architectural extension: axp64
// CHECK-NEXT: .arch_extension axp64
crc32cx w0, w1, x3
More information about the llvm-commits
mailing list