Revert "[AArch64][AsmParser] Directives should clear transitively implied features (#106625)" (#106813)

vitalybuka · web-flow · commit d8bffa9018c8 · 2024-08-30T17:37:48.000-07:00
Revert #106625 and fix attempt #106804" There is another issue in https://lab.llvm.org/buildbot/#/builders/169/builds/2690 directive-cpu-err.s and the fix like #106804 fixed the overflow but fails CHECKs. This reverts commit 10affaf. This reverts commit 2497739.
diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
@@ -6947,22 +6947,18 @@ 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 CurLoc = getLoc();
+  SMLoc ArchLoc = getLoc();
 
   StringRef Arch, ExtensionString;
   std::tie(Arch, ExtensionString) =
       getParser().parseStringToEndOfStatement().trim().split('+');
 
   const AArch64::ArchInfo *ArchInfo = AArch64::parseArch(Arch);
   if (!ArchInfo)
-    return Error(CurLoc, "unknown arch name");
+    return Error(ArchLoc, "unknown arch name");
 
   if (parseToken(AsmToken::EndOfStatement))
     return true;
@@ -6982,30 +6978,27 @@ 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");
 
-    auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
-      return Extension.Name == Name;
-    });
-
-    if (It == std::end(ExtensionMap))
-      return Error(CurLoc, "unsupported architectural extension: " + Name);
+    for (const auto &Extension : ExtensionMap) {
+      if (Extension.Name != Name)
+        continue;
 
-    if (EnableFeature)
-      STI.SetFeatureBitsTransitively(It->Features);
-    else
-      STI.ClearFeatureBitsTransitively(It->Features);
+      if (Extension.Features.none())
+        report_fatal_error("unsupported architectural extension: " + Name);
 
-    CurLoc = incrementLoc(CurLoc, Name.size());
+      FeatureBitset ToggleFeatures =
+          EnableFeature
+              ? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
+              : STI.ToggleFeature(Features & Extension.Features);
+      setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
+      break;
+    }
   }
-  FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
-  setAvailableFeatures(Features);
   return false;
 }
 
@@ -7025,21 +7018,28 @@ bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) {
     Name = Name.substr(2);
   }
 
-  auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
-    return Extension.Name == Name;
-  });
+  MCSubtargetInfo &STI = copySTI();
+  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
+            ? STI.SetFeatureBitsTransitively(~Features & Extension.Features)
+            : STI.ToggleFeature(Features & Extension.Features);
+    setAvailableFeatures(ComputeAvailableFeatures(ToggleFeatures));
+    return false;
+  }
 
-  if (It == std::end(ExtensionMap))
-    return Error(ExtLoc, "unsupported architectural extension: " + Name);
+  return Error(ExtLoc, "unknown architectural extension: " + Name);
+}
 
-  MCSubtargetInfo &STI = copySTI();
-  if (EnableFeature)
-    STI.SetFeatureBitsTransitively(It->Features);
-  else
-    STI.ClearFeatureBitsTransitively(It->Features);
-  FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
-  setAvailableFeatures(Features);
-  return false;
+static SMLoc incrementLoc(SMLoc L, int Offset) {
+  return SMLoc::getFromPointer(L.getPointer() + Offset);
 }
 
 /// parseDirectiveCPU
@@ -7075,22 +7075,30 @@ bool AArch64AsmParser::parseDirectiveCPU(SMLoc L) {
 
     bool EnableFeature = !Name.consume_front_insensitive("no");
 
-    auto It = llvm::find_if(ExtensionMap, [&Name](const auto &Extension) {
-      return Extension.Name == Name;
-    });
+    bool FoundExtension = false;
+    for (const auto &Extension : ExtensionMap) {
+      if (Extension.Name != Name)
+        continue;
 
-    if (It == std::end(ExtensionMap))
-      Error(CurLoc, "unsupported architectural extension: " + Name);
+      if (Extension.Features.none())
+        report_fatal_error("unsupported architectural extension: " + Name);
 
-    if (EnableFeature)
-      STI.SetFeatureBitsTransitively(It->Features);
-    else
-      STI.ClearFeatureBitsTransitively(It->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");
 
     CurLoc = incrementLoc(CurLoc, Name.size());
   }
-  FeatureBitset Features = ComputeAvailableFeatures(STI.getFeatureBits());
-  setAvailableFeatures(Features);
   return false;
 }
 
diff --git a/llvm/test/MC/AArch64/SVE/directive-arch-negative.s b/llvm/test/MC/AArch64/SVE/directive-arch-negative.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
@@ -1,12 +1,7 @@
 // RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
 
-.arch_extension sve2+nosve
+.arch_extension 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
@@ -1,11 +1,6 @@
 // RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
 
-.cpu generic+sve2+nosve
+.cpu generic+sve+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-negative.s b/llvm/test/MC/AArch64/directive-arch-negative.s
@@ -12,13 +12,10 @@
 # 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
@@ -4,7 +4,7 @@
 // RUN: -filetype asm -o - %s 2>&1 | FileCheck %s
 
 .arch_extension axp64
-// CHECK: error: unsupported architectural extension: axp64
+// CHECK: error: unknown architectural extension: axp64
 // CHECK-NEXT: .arch_extension axp64
 
 crc32cx w0, w1, x3
@@ -49,8 +49,6 @@ 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
@@ -72,8 +70,6 @@ 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
@@ -88,8 +84,6 @@ 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
@@ -139,8 +133,6 @@ 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
@@ -177,8 +169,6 @@ 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
@@ -214,8 +204,6 @@ 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
