[AArch64][AsmParser] Arch directives should set implied features.

[sdesmalen]

Summary

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.

[c-rhodes]

llvm/test/MC/AArch64/SVE/directive-arch_extension-negative.s

12 ↗

(On Diff #410327)

SVE is never enabled so I don't think this is doing what you intended, plus this instruction will be in the output for both pass/fail? I'm not sure it makes sense to mix positive/negative tests like this. As an aside, I'm not sure what my original intention was with this test but it would make sense to enable the feature before disabling it to verify nosve is doing what it's supposed to.

llvm/test/MC/AArch64/SVE/directive-arch_extension.s

7–12

unlike .arch, .arch_extension doesn't clear any previously selected architecture extensions, probably best to disable SVE with .arch_extension nosve first.

llvm/test/MC/AArch64/SVE/directive-cpu-negative.s

3

enable SVE before disabling it?

9–13

I think this belongs in the positive test.

[sdesmalen]

Addressed @c-rhodes' comments.

llvm/test/MC/AArch64/SVE/directive-arch_extension-negative.s

12 ↗

(On Diff #410327)

Good point!

llvm/test/MC/AArch64/SVE/directive-arch_extension.s

7–12

Good point!

llvm/test/MC/AArch64/SVE/directive-cpu-negative.s

3

Good suggestion, this actually exposed a bug in the existing implementation of .cpu directive!
https://godbolt.org/z/MndP97soh

llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

for (auto Name : RequestedExtensions) {
 
     for (const auto &Extension : ExtensionMap) {
       if (Extension.Name != Name)
         continue;
 
       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;
     }
   }
   return false;
 }
 
 /// parseDirectiveArchExtension
 ///   ::= .arch_extension [no]feature
bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) {
   FeatureBitset Features = STI.getFeatureBits();
   for (const auto &Extension : ExtensionMap) {
     if (Extension.Name != Name)
       continue;
 
     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;
   }
 
   return Error(ExtLoc, "unknown architectural extension: " + Name);
 }
 
 static SMLoc incrementLoc(SMLoc L, int Offset) {
   return SMLoc::getFromPointer(L.getPointer() + Offset);
bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) {
   }
 
   MCSubtargetInfo &STI = copySTI();
   STI.setDefaultFeatures(CPU, /*TuneCPU*/ CPU, "");
   CurLoc = incrementLoc(CurLoc, CPU.size());
 
   ExpandCryptoAEK(llvm::AArch64::getCPUArchKind(CPU), RequestedExtensions);
 
-  FeatureBitset Features = STI.getFeatureBits();
   for (auto Name : RequestedExtensions) {
     // Advance source location past '+'.
     CurLoc = incrementLoc(CurLoc, 1);
 
     bool EnableFeature = true;
 
     if (Name.startswith_insensitive("no")) {
       EnableFeature = false;
       Name = Name.substr(2);
     }
 
     bool FoundExtension = false;
     for (const auto &Extension : ExtensionMap) {
       if (Extension.Name != Name)
         continue;
 
       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;
     }
 
     if (!FoundExtension)
       Error(CurLoc, "unsupported architectural extension");
 

llvm/test/MC/AArch64/SVE/directive-arch.s

 // RUN: llvm-mc -triple=aarch64 < %s | FileCheck %s
 
 .arch armv8-a+sve
 
 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

llvm/test/MC/AArch64/SVE/directive-arch_extension.s

 // RUN: llvm-mc -triple=aarch64 < %s | FileCheck %s
 
 .arch_extension sve
 
 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

llvm/test/MC/AArch64/SVE/directive-cpu-negative.s

+// 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

llvm/test/MC/AArch64/SVE/directive-cpu.s

+// 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