[AArch64][AsmParser] Fix .arch_extension directive parsing

[c-rhodes]

Summary

This patch fixes .arch_extension directive parsing to handle a wider
range of architecture extension options. The existing parser was parsing
extensions as an identifier which breaks for extensions containing a
"-", such as the "tlb-rmi" extension.

The extension is now parsed as a string. This is consistent with the
extension parsing in the .arch and .cpu directive parsing.

[mstorsjo]

The change looks good and straightforward to me, but I guess it could be good for someone else to have a look as well as I'm no authority here. I can mark it as approved in a few days if nobody else comments.

[SjoerdMeijer]

This change looks also fine to me.
From a quick grep in the test directory, looks like we don't have many tests for the .arch_extension directive. I.e., I see only one, for the simd directive. I think that confirms my suspicion that we tend to forget to add tests/support for the .arch_extension directive when we upstream new architecture support. So, sorry to be a pain here, but while you're at it, my request would be to add tests for some other architecture extensions that we are missing. For example, rasv8_4 might be an interesting one because it has an underscore in it. Looks like the refactoring in D54633 introduced new extensions that we could test.

[c-rhodes]

From a quick grep in the test directory, looks like we don't have many tests for the .arch_extension directive. I.e., I see only one, for the simd directive. I think that confirms my suspicion that we tend to forget to add tests/support for the .arch_extension directive when we upstream new architecture support. So, sorry to be a pain here, but while you're at it, my request would be to add tests for some other architecture extensions that we are missing. For example, rasv8_4 might be an interesting one because it has an underscore in it. Looks like the refactoring in D54633 introduced new extensions that we could test.

Hi Sjoerd, yes the tests are lacking for the .arch_extension directive, thanks for pointing that patch out I'll add a few tests for features with interesting characters in the name that are not currently covered.

[c-rhodes]

@SjoerdMeijer It appears the .cpu, .arch and .arch_extension directives can only enable features defined in ExtensionMap in the AArch64AsmParser.cpp and ignores them otherwise. ExtensionMap seems incomplete, in the patch you mentioned only tlb-rmi, pan-rwv and ccpp features were added. There's also a comment:

2834     // FIXME: Unsupported extensions
2835     {"pan", {}},
2836     {"lor", {}},
2837     {"rdma", {}},
2838     {"profile", {}},

Which includes pan and lor which I don't understand why they are unsupported, perhaps they were added before the full feature support. I think additional work is required to improve ExtensionMap, I could add further tests for features currently available such as SVE in this patch, or is it also worth addressing the missing features?

[SjoerdMeijer]

It appears the .cpu, .arch and .arch_extension directives can only enable features defined in ExtensionMap in the AArch64AsmParser.cpp and ignores them otherwise. ExtensionMap seems incomplete, in the patch you mentioned only tlb-rmi, pan-rwv and ccpp features were added.

This whole ExtensionMap is a bit of a hack. It's a hack because it duplicates information already described in TargetParser (AArch64TargetParser.def). But this isn't an easy thing to fix I think.

I don't want to hold up this patch, but any low hanging fruit to make this more complete would be a really big bonus. Perhaps an approach is to first complete the tests with extensions already in ExtensionMap, that should hopefully just work. And if you feel strong about this, you can then add missing extension in a follow up patch.

[c-rhodes]

Improved tests to cover all features supported by .arch_extension

[c-rhodes]

@SjoerdMeijer I've added tests for all features currently in ExtensionMap.

[SjoerdMeijer]

Nice one, thanks for fixing this!

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

for (auto Name : RequestedExtensions) {
     }
   }
   return false;
 }
 
 /// parseDirectiveArchExtension
 ///   ::= .arch_extension [no]feature
 bool AArch64AsmParser::parseDirectiveArchExtension(SMLoc L) {
-  MCAsmParser &Parser = getParser();
-
-  if (getLexer().isNot(AsmToken::Identifier))
-    return Error(getLexer().getLoc(), "expected architecture extension name");
+  SMLoc ExtLoc = getLoc();
 
-  const AsmToken &Tok = Parser.getTok();
-  StringRef Name = Tok.getString();
-  SMLoc ExtLoc = Tok.getLoc();
-  Lex();
+  StringRef Name = getParser().parseStringToEndOfStatement().trim();
 
   if (parseToken(AsmToken::EndOfStatement,
                  "unexpected token in '.arch_extension' directive"))
     return true;
 
   bool EnableFeature = true;
   if (Name.startswith_lower("no")) {
     EnableFeature = false;

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

+// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
+
+.arch_extension nosve
+
+ptrue   p0.b, pow2
+// CHECK: error: instruction requires: sve
+// CHECK-NEXT: ptrue   p0.b, pow2

llvm/trunk/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

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

+// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
+
+.arch_extension axp64
+// CHECK: error: unknown architectural extension: axp64
+// CHECK-NEXT: .arch_extension axp64
+
+.arch_extension nocrc
+crc32cx w0, w1, x3
+// CHECK: error: instruction requires: crc
+// CHECK-NEXT: crc32cx w0, w1, x3
+
+.arch_extension nosm4
+sm4e v2.4s, v15.4s
+// CHECK: error: instruction requires: sm4
+// CHECK-NEXT: sm4e v2.4s, v15.4s
+
+.arch_extension nosha3
+sha512h q0, q1, v2.2d
+// CHECK: error: instruction requires: sha3
+// CHECK-NEXT: sha512h q0, q1, v2.2d
+
+.arch_extension nosha2
+sha1h s0, s1
+// CHECK: error: instruction requires: sha2
+// CHECK-NEXT: sha1h s0, s1
+
+.arch_extension noaes
+aese v0.16b, v1.16b
+// CHECK: error: instruction requires: aes
+// CHECK-NEXT: aese v0.16b, v1.16b
+
+.arch_extension nofp
+fminnm d0, d0, d1
+// CHECK: error: instruction requires: fp
+// CHECK-NEXT: fminnm d0, d0, d1
+
+.arch_extension nosimd
+addp v0.4s, v0.4s, v0.4s
+// CHECK: error: instruction requires: neon
+// CHECK-NEXT: addp v0.4s, v0.4s, v0.4s
+
+.arch_extension noras
+esb
+// CHECK: error: instruction requires: ras
+// CHECK-NEXT: esb
+
+.arch_extension nolse
+casa w5, w7, [x20]
+// CHECK: error: instruction requires: lse
+// CHECK-NEXT: casa w5, w7, [x20]
+
+.arch_extension nopredres
+cfp rctx, x0
+// CHECK: error: CFPRCTX requires predres
+// CHECK-NEXT: cfp rctx, x0
+
+.arch_extension noccdp
+dc cvadp, x7
+// CHECK: error: DC CVADP requires ccdp
+// CHECK-NEXT: dc cvadp, x7
+
+.arch_extension nomte
+irg x0, x1
+// CHECK: error: instruction requires: mte
+// CHECK-NEXT: irg x0, x1
+
+.arch_extension notlb-rmi
+tlbi vmalle1os
+// CHECK: error: TLBI VMALLE1OS requires tlb-rmi
+// CHECK-NEXT: tlbi vmalle1os
+
+.arch_extension nopan-rwv
+at s1e1wp, x2
+// CHECK: error: AT S1E1WP requires pan-rwv
+// CHECK-NEXT: at s1e1wp, x2
+
+.arch_extension noccpp
+dc cvap, x7
+// CHECK: error: DC CVAP requires ccpp
+// CHECK-NEXT: dc cvap, x7

llvm/trunk/test/MC/AArch64/directive-arch_extension-nosimd.s

-// RUN: not llvm-mc -triple aarch64 -filetype asm -o - %s 2>&1 | FileCheck %s
-
-  .arch_extension nosimd
-
-  add v0.8b, v0.8b, v0.8b
-// CHECK: error: instruction requires: neon

llvm/trunk/test/MC/AArch64/directive-arch_extension-simd.s

-// RUN: llvm-mc -triple aarch64 -mattr=-simd -filetype asm -o - %s | FileCheck %s
-
-  .arch_extension simd
-
-  add v0.8b, v0.8b, v0.8b
-// CHECK: add v0.8b, v0.8b, v0.8b

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

+// RUN: llvm-mc -triple aarch64 -filetype asm -o - %s | FileCheck %s
+
+.arch_extension crc
+crc32cx w0, w1, x3
+// CHECK: crc32cx w0, w1, x3
+
+.arch_extension sm4
+sm4e v2.4s, v15.4s
+// CHECK: sm4e v2.4s, v15.4s
+
+.arch_extension sha3
+sha512h q0, q1, v2.2d
+// CHECK: sha512h q0, q1, v2.2d
+
+.arch_extension sha2
+sha1h s0, s1
+// CHECK: sha1h s0, s1
+
+.arch_extension aes
+aese v0.16b, v1.16b
+// CHECK: aese v0.16b, v1.16b
+
+.arch_extension fp
+fminnm d0, d0, d1
+// CHECK: fminnm d0, d0, d1
+
+.arch_extension simd
+addp v0.4s, v0.4s, v0.4s
+// CHECK: addp v0.4s, v0.4s, v0.4s
+
+.arch_extension ras
+esb
+// CHECK: esb
+
+.arch_extension lse
+casa w5, w7, [x20]
+// CHECK: casa w5, w7, [x20]
+
+.arch_extension predres
+cfp rctx, x0
+// CHECK: cfp rctx, x0
+
+.arch_extension ccdp
+dc cvadp, x7
+// CHECK: dc cvadp, x7
+
+.arch_extension mte
+irg x0, x1
+// CHECK: irg x0, x1
+
+.arch_extension tlb-rmi
+tlbi vmalle1os
+// CHECK: tlbi vmalle1os
+
+.arch_extension pan-rwv
+at s1e1wp, x2
+// CHECK: at s1e1wp, x2
+
+.arch_extension ccpp
+dc cvap, x7
+// CHECK: dc cvap, x7