[AArch64][SVE] Asm: Report SVE parsing diagnostics only once

[sdesmalen]

Summary

Prevent an issue where a diagnostic is reported multiple times by bailing out with a ParseFail if an invalid SVE register element qualifier/suffix is specified, for example:

<stdin>:10:18: error: invalid sve vector kind qualifier
add z20.h, z2.h, z31.x

^

<stdin>:10:18: error: invalid sve vector kind qualifier
add z20.h, z2.h, z31.x

...

<stdin>:10:18: error: invalid sve vector kind qualifier
add z20.h, z2.h, z31.x

^

[sdesmalen]

Reverted a change to the diagnostic for 'invalid vector kind qualifier'.

[rengolin]

Is the VEDataVectorRegister` to SVERegister change meaningful?

[sdesmalen]

In D39894#922644, @rengolin wrote:

Is the VEDataVectorRegister` to SVERegister change meaningful?

The change from 'tryParseSVEDataVectorRegister' to 'tryParseSVERegister' is there because when I applied this patch onto our downstream branch (to ensure this patch would work for our entire test set with more instructions than unpredicated add/sub), I found that tryParseSVEDataVectorRegister() and tryParsePredicateVectorRegister() were the practically same after backporting our changes to matchRegisterNameAlias() from the previous patch. tryParseSVERegister() will also handle Predicates when we extend matchRegisterNameAlias with SVE predicates in a later patch, similar to how we did it for data vectors and so there is no need for having the 'DataVector' part in the name.

[rengolin]

In D39894#923025, @sdesmalen wrote:

tryParseSVERegister() will also handle Predicates when we extend matchRegisterNameAlias with SVE predicates in a later patch, similar to how we did it for data vectors and so there is no need for having the 'DataVector' part in the name.

Ah, excellent! Normally, such a change would need to be done in a separate patch, but it should be fine for such a small and preparatory patch.

I'll review the changes soon. Sorry, I'm at SC17 now.

[rengolin]

Right, ok, you'll reuse the parser with a different kind later, that makes sense and the code is cleaner this way.

LGTM. Thanks!

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

private:
 
   bool parseSysAlias(StringRef Name, SMLoc NameLoc, OperandVector &Operands);
   void createSysAlias(uint16_t Encoding, OperandVector &Operands, SMLoc S);
   AArch64CC::CondCode parseCondCodeString(StringRef Cond);
   bool parseCondCode(OperandVector &Operands, bool invertCondCode);
   unsigned matchRegisterNameAlias(StringRef Name, RegKind Kind);
   int tryParseRegister();
   int tryMatchVectorRegister(StringRef &Kind, bool expected);
-  int tryParseSVEDataVectorRegister(const AsmToken &Tok, StringRef &Kind);
   bool parseRegister(OperandVector &Operands);
   bool parseSymbolicImmVal(const MCExpr *&ImmVal);
   bool parseVectorList(OperandVector &Operands);
   bool parseOperand(OperandVector &Operands, bool isCondCode,
                     bool invertCondCode);
 
   bool showMatchError(SMLoc Loc, unsigned ErrCode, OperandVector &Operands);
 

 /// @name Auto-generated Match Functions
 /// {
 
 #define GET_ASSEMBLER_HEADER
 #include "AArch64GenAsmMatcher.inc"
 
   /// }
 
+  OperandMatchResultTy tryParseSVERegister(int &Reg, StringRef &Kind,
+                                           RegKind MatchKind);
   OperandMatchResultTy tryParseOptionalShiftExtend(OperandVector &Operands);
   OperandMatchResultTy tryParseBarrierOperand(OperandVector &Operands);
   OperandMatchResultTy tryParseMRSSystemRegister(OperandVector &Operands);
   OperandMatchResultTy tryParseSysReg(OperandVector &Operands);
   OperandMatchResultTy tryParseSysCROperand(OperandVector &Operands);
   OperandMatchResultTy tryParsePrefetch(OperandVector &Operands);
   OperandMatchResultTy tryParsePSBHint(OperandVector &Operands);
   OperandMatchResultTy tryParseAdrpLabel(OperandVector &Operands);
bool AArch64AsmParser::tryParseNeonVectorRegister(OperandVector &Operands) {
   }
 
   return false;
 }
 
 // tryParseSVEDataVectorRegister - Try to parse a SVE vector register name with
 // optional kind specifier. If it is a register specifier, eat the token
 // and return it.
-int AArch64AsmParser::tryParseSVEDataVectorRegister(const AsmToken &Tok,
-                                                StringRef &Kind) {
+OperandMatchResultTy
+AArch64AsmParser::tryParseSVERegister(int &Reg, StringRef &Kind,
+                                      RegKind MatchKind) {
+  MCAsmParser &Parser = getParser();
+  const AsmToken &Tok = Parser.getTok();
+
   if (Tok.isNot(AsmToken::Identifier))
-    return -1;
+    return MatchOperand_NoMatch;
 
   StringRef Name = Tok.getString();
   // If there is a kind specifier, it's separated from the register name by
   // a '.'.
   size_t Start = 0, Next = Name.find('.');
   StringRef Head = Name.slice(Start, Next);
-  unsigned RegNum = matchRegisterNameAlias(Head, RegKind::SVEDataVector);
+  unsigned RegNum = matchRegisterNameAlias(Head, MatchKind);
 
   if (RegNum) {
     if (Next != StringRef::npos) {
       Kind = Name.slice(Next, StringRef::npos);
       if (!isValidSVEKind(Kind)) {
         TokError("invalid sve vector kind qualifier");
-        return -1;
+        return MatchOperand_ParseFail;
       }
     }
-    return RegNum;
+    Parser.Lex(); // Eat the register token.
+
+    Reg = RegNum;
+    return MatchOperand_Success;
   }
 
-  return -1;
+  return MatchOperand_NoMatch;
 }
 
 /// parseRegister - Parse a non-vector register operand.
 bool AArch64AsmParser::parseRegister(OperandVector &Operands) {
   SMLoc S = getLoc();
   // Try for a vector (neon) register.
   if (!tryParseNeonVectorRegister(Operands))
     return false;

 }
 
 /// parseDirectiveReq
 ///  ::= name .req registername
 bool AArch64AsmParser::parseDirectiveReq(StringRef Name, SMLoc L) {
   MCAsmParser &Parser = getParser();
   Parser.Lex(); // Eat the '.req' token.
   SMLoc SRegLoc = getLoc();
-  unsigned RegNum = tryParseRegister();
+  int RegNum = tryParseRegister();
   RegKind RegisterKind = RegKind::Scalar;
 
-  if (RegNum == static_cast<unsigned>(-1)) {
+  if (RegNum == -1) {
     StringRef Kind;
     RegisterKind = RegKind::NeonVector;
     RegNum = tryMatchVectorRegister(Kind, false);
     if (!Kind.empty())
       return Error(SRegLoc, "vector register without type specifier expected");
   }
 
-  if (RegNum == static_cast<unsigned>(-1)) {
+  if (RegNum == -1) {
     StringRef Kind;
     RegisterKind = RegKind::SVEDataVector;
-    int RegNumTmp = tryParseSVEDataVectorRegister(Parser.getTok(), Kind);
-    if (RegNumTmp != -1)
-      Parser.Lex();
-    RegNum = RegNumTmp;
-    if (!Kind.empty())
-      return Error(SRegLoc, "sve vector register without type specifier expected");
+    OperandMatchResultTy Res =
+        tryParseSVERegister(RegNum, Kind, RegKind::SVEDataVector);
+
+    if (Res == MatchOperand_ParseFail)
+      return true;
+
+    if (Res == MatchOperand_Success && !Kind.empty())
+      return Error(SRegLoc,
+                   "sve vector register without type specifier expected");
   }
 
-  if (RegNum == static_cast<unsigned>(-1))
+  if (RegNum == -1)
     return Error(SRegLoc, "register name or alias expected");
 
   // Shouldn't be anything else.
   if (parseToken(AsmToken::EndOfStatement,
                  "unexpected input in .req directive"))
     return true;
 
-  auto pair = std::make_pair(RegisterKind, RegNum);
+  auto pair = std::make_pair(RegisterKind, (unsigned) RegNum);
   if (RegisterReqs.insert(std::make_pair(Name, pair)).first->second != pair)
     Warning(L, "ignoring redefinition of register alias '" + Name + "'");
 
   return false;
 }
 
 /// parseDirectiveUneq
 ///  ::= .unreq registername
Operands.push_back(AArch64Operand::CreateReg(Pair, RegKind::Scalar, S,
       getLoc(), getContext()));
 
   return MatchOperand_Success;
 }
 
 template <bool ParseSuffix>
 OperandMatchResultTy
 AArch64AsmParser::tryParseSVEDataVector(OperandVector &Operands) {
-  MCAsmParser &Parser = getParser();
   const SMLoc S = getLoc();
   // Check for a SVE vector register specifier first.
+  int RegNum = -1;
   StringRef Kind;
-  int RegNum = tryParseSVEDataVectorRegister(Parser.getTok(), Kind);
-  if (RegNum == -1)
-    return MatchOperand_NoMatch;
 
-  // Eat the SVE Register Token
-  Parser.Lex();
+  OperandMatchResultTy Res =
+      tryParseSVERegister(RegNum, Kind, RegKind::SVEDataVector);
+
+  if (Res != MatchOperand_Success)
+    return Res;
 
   if (ParseSuffix && Kind.empty())
     return MatchOperand_NoMatch;
 
   unsigned ElementWidth = StringSwitch<unsigned>(Kind.lower())
                         .Case("", -1)
                         .Case(".b", 8)
                         .Case(".h", 16)

test/MC/AArch64/SVE/add-diagnostics.s

 // RUN: not llvm-mc -triple=aarch64 -show-encoding -mattr=+sve  2>&1 < %s| FileCheck %s
 
 // Register z32 does not exist.
 add z22.h, z10.h, z32.h
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand
 // CHECK-NEXT: add z22.h, z10.h, z32.h
+// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
 
 // Invalid element kind.
 add z20.h, z2.h, z31.x
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid sve vector kind qualifier
 // CHECK-NEXT: add z20.h, z2.h, z31.x
+// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
 
 // Element size specifiers should match.
 add z27.h, z11.h, z27.b
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand
 // CHECK-NEXT: add z27.h, z11.h, z27.b
+// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:

test/MC/AArch64/SVE/sub-diagnostics.s

 // RUN: not llvm-mc -triple=aarch64 -show-encoding -mattr=+sve  2>&1 < %s| FileCheck %s
 
 // Register z32 does not exist.
 sub z3.h, z26.h, z32.h
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand
 // CHECK-NEXT: sub z3.h, z26.h, z32.h
+// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
 
 // Invalid element kind.
 sub z4.h, z27.h, z31.x
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid sve vector kind qualifier
 // CHECK-NEXT: sub z4.h, z27.h, z31.x
+// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
 
 // Element size specifiers should match.
 sub z0.h, z8.h, z8.b
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand
 // CHECK-NEXT: sub z0.h, z8.h, z8.b
+// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}: