[AArch64][AsmParser] Arch directives should set implied features.
authorSander de Smalen <sander.desmalen@arm.com>
Thu, 24 Feb 2022 08:46:15 +0000 (08:46 +0000)
committerSander de Smalen <sander.desmalen@arm.com>
Thu, 24 Feb 2022 09:15:17 +0000 (09:15 +0000)
When assembling for example an SVE instruction with the `.arch +sve2` directive,
+sve should be implied by setting +sve2, similar to what would happen if
one would pass the mattr=+sve2 flag on the command-line.

The AsmParser doesn't set the implied features, meaning that the SVE
instruction does not assemble. This patch fixes that.

Note that the same does not hold when disabling a feature. For example,
+nosve2 does not imply +nosve.

Reviewed By: c-rhodes

Differential Revision: https://reviews.llvm.org/D120259

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
llvm/test/MC/AArch64/SVE/directive-arch.s
llvm/test/MC/AArch64/SVE/directive-arch_extension.s
llvm/test/MC/AArch64/SVE/directive-cpu-negative.s [new file with mode: 0644]
llvm/test/MC/AArch64/SVE/directive-cpu.s [new file with mode: 0644]

index a764e8c..04ab54c 100644 (file)
@@ -6214,12 +6214,11 @@ bool AArch64AsmParser::parseDirectiveArch(SMLoc L) {
       if (Extension.Features.none())
         report_fatal_error("unsupported architectural extension: " + Name);
 
-      FeatureBitset ToggleFeatures = EnableFeature
-                                         ? (~Features & Extension.Features)
-                                         : ( Features & Extension.Features);
-      FeatureBitset Features =
-          ComputeAvailableFeatures(STI.ToggleFeature(ToggleFeatures));
-      setAvailableFeatures(Features);
+      FeatureBitset ToggleFeatures =
+          EnableFeature
+              ? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
+              : STI.ToggleFeature(Features & Extension.Features);
+      setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
       break;
     }
   }
@@ -6252,12 +6251,11 @@ bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) {
     if (Extension.Features.none())
       return Error(ExtLoc, "unsupported architectural extension: " + Name);
 
-    FeatureBitset ToggleFeatures = EnableFeature
-                                       ? (~Features & Extension.Features)
-                                       : (Features & Extension.Features);
-    FeatureBitset Features =
-        ComputeAvailableFeatures(STI.ToggleFeature(ToggleFeatures));
-    setAvailableFeatures(Features);
+    FeatureBitset ToggleFeatures =
+        EnableFeature
+            ? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
+            : STI.ToggleFeature(Features & Extension.Features);
+    setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
     return false;
   }
 
@@ -6297,7 +6295,6 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) {
 
   ExpandCryptoAEK(llvm::AArch64::getCPUArchKind(CPU), RequestedExtensions);
 
-  FeatureBitset Features = STI.getFeatureBits();
   for (auto Name : RequestedExtensions) {
     // Advance source location past '+'.
     CurLoc = incrementLoc(CurLoc, 1);
@@ -6317,12 +6314,12 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) {
       if (Extension.Features.none())
         report_fatal_error("unsupported architectural extension: " + Name);
 
-      FeatureBitset ToggleFeatures = EnableFeature
-                                         ? (~Features & Extension.Features)
-                                         : ( Features & Extension.Features);
-      FeatureBitset Features =
-          ComputeAvailableFeatures(STI.ToggleFeature(ToggleFeatures));
-      setAvailableFeatures(Features);
+      FeatureBitset Features = STI.getFeatureBits();
+      FeatureBitset ToggleFeatures =
+          EnableFeature
+              ? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
+              : STI.ToggleFeature(Features & Extension.Features);
+      setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
       FoundExtension = true;
 
       break;
index 99ddf4e..81473fb 100644 (file)
@@ -4,3 +4,9 @@
 
 ptrue   p0.b, pow2
 // CHECK: ptrue   p0.b, pow2
+
+// Test that the implied +sve feature is also set from +sve2.
+.arch armv8-a+sve2
+
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2
index 72351b6..c516c7d 100644 (file)
@@ -4,3 +4,15 @@
 
 ptrue   p0.b, pow2
 // CHECK: ptrue   p0.b, pow2
+
+// Test that the implied +sve feature is also set from +sve2.
+.arch_extension nosve
+.arch_extension sve2
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2
+
+// Check that setting +nosve2 does not imply +nosve
+.arch_extension nosve2
+
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2
diff --git a/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s b/llvm/test/MC/AArch64/SVE/directive-cpu-negative.s
new file mode 100644 (file)
index 0000000..82acc1b
--- /dev/null
@@ -0,0 +1,6 @@
+// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
+
+.cpu generic+sve+nosve
+ptrue   p0.b, pow2
+// CHECK: error: instruction requires: sve or sme
+// CHECK-NEXT: ptrue   p0.b, pow2
diff --git a/llvm/test/MC/AArch64/SVE/directive-cpu.s b/llvm/test/MC/AArch64/SVE/directive-cpu.s
new file mode 100644 (file)
index 0000000..37d399a
--- /dev/null
@@ -0,0 +1,15 @@
+// RUN: llvm-mc -triple=aarch64 < %s | FileCheck %s
+
+.cpu generic+sve
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2
+
+// Test that the implied +sve feature is also set from +sve2.
+.cpu generic+sve2
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2
+
+// Check that setting +nosve2 does not imply +nosve
+.cpu generic+sve2+nosve2
+ptrue   p0.b, pow2
+// CHECK: ptrue   p0.b, pow2