[AArch64] Asm: Fix parsing of register aliases that have a name starting with 'z'

[sdesmalen]

Summary

This fixes an issue as identified by @rnk in https://reviews.llvm.org/rL321029.

[fhahn]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

2010

I don't think we need this early exit here. If it's a vector register, matchRegisterNameAlias should return 0 and we return -1 later. We do not bail out for NEON early either.

[fhahn]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

2010

Ok, I was wrong about matchRegisterNameAlias. But it seems the early exit is not needed, the MC test pass for me without it and we do not have a similar case for NEON.

[sdesmalen]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

2010

This was added to enable the other test, where the alias is a vector register (that needs custom parsing), e.g.:

peter .req z6
add peter.s, z0.s, z0.s

However, in that case it seems wrong for 'matchRegisterNameAlias(..., RegKind::Scalar) to return the register number of an SVE vector. This is caused by MatchRegisterName() being generic to work for non-scalar registers as well, so I'll fix this.

[sdesmalen]

Fixed issue where matchRegisterNameAlias(..., RegKind::Scalar) also matched non-scalar identifiers.

[rnk]

Thanks!

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

2005–2012

This is unused now.

[fhahn]

LGTM. The problem this solves is that MatchRegisterName parses GPRs, NEON and SVE registers, which leads to surprising behaviour, i.e. matchRegisterNameAlias with RegKind == Scalar would return > 0 for any (valid) Neon & SVE register as well. From the comments and the behaviour, it seems like tryParseRegister should only parse scalar registers, which this patch implements. I think it's worth adding a comment making that clear.

Adding Eric, in case he has some more thoughts!

[fhahn]

One more thing for completeness (there's no need to address this in this patch): currently we try to parse the register before looking up aliases so the following will fail

	z1 .req x6
	add z1, x0, x0
	.unreq z1
// CHECK: add x6, x0, x0

I think GAS tries to apply aliases first, with the following reasoning: Before the SVE registers were added, the snippet above was valid. Now it is not valid any more, as it will parse z1 as a SVE register (the same problem also exists with NEON vector registers)

[sdesmalen]

@rnk Thanks for providing the reduced test-case!
@fhahn Interesting, it seems the assembler had this behaviour even before the SVE patches changed the matchRegisterNameAlias() method.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

return StringSwitch<bool>(Name.lower())
       .Case(".b", true)
       .Case(".h", true)
       .Case(".s", true)
       .Case(".d", true)
       .Case(".q", true)
       .Default(false);
 }
 
-static bool isSVEDataVectorRegister(StringRef Name) {
-  return Name[0] == 'z';
-}
-
 static void parseValidVectorKind(StringRef Name, unsigned &NumElements,
                                  char &ElementKind) {
   assert(isValidVectorKind(Name));
 
   ElementKind = Name.lower()[Name.size() - 1];
   NumElements = 0;
 
   if (Name.size() == 2)

 /// Identifier when called, and if it is a register name the token is eaten and
 /// the register is added to the operand list.
 int AArch64AsmParser::tryParseRegister() {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
   if (Tok.isNot(AsmToken::Identifier))
     return -1;
 
   std::string lowerCase = Tok.getString().lower();
-  if (isSVEDataVectorRegister(lowerCase))
+  unsigned RegNum = matchRegisterNameAlias(lowerCase, RegKind::Scalar);
+
+  // Leave the parsing of ZPR/PPR registers (or aliases) to the explicit
+  // tryParse methods for SVE data and predicate vectors.
+  if (AArch64MCRegisterClasses[AArch64::ZPRRegClassID].contains(RegNum))
     return -1;
 
-  unsigned RegNum = matchRegisterNameAlias(lowerCase, RegKind::Scalar);
   // Also handle a few aliases of registers.
   if (RegNum == 0)
     RegNum = StringSwitch<unsigned>(lowerCase)
                  .Case("fp",  AArch64::FP)
                  .Case("lr",  AArch64::LR)
                  .Case("x31", AArch64::XZR)
                  .Case("w31", AArch64::WZR)
                  .Default(0);

test/MC/AArch64/dot-req.s

 // CHECK: fmov    d2, d3                  // encoding: [0x62,0x40,0x60,0x1e]
 // CHECK: ldr      q2, [sp]               // encoding: [0xe2,0x03,0xc0,0x3d]
 // CHECK: mov             v0.8b, v1.8b    // encoding: [0x20,0x1c,0xa1,0x0e]
 
   peter .req x6
   add peter, x0, x0
   .unreq peter
 // CHECK: add x6, x0, x0
+
+  zoe .req x6
+  add zoe, x0, x0
+  .unreq zoe
+// CHECK: add x6, x0, x0