[AArch64][AsmParser] Unify code for parsing Neon/SVE vectors.

[sdesmalen]

Summary

Merged 'tryMatchVectorRegister' (specific to Neon) and
'tryParseSVERegister' into a single 'tryParseVectorRegister' function, and
created a generic 'parseVectorKind()' function that returns the #Elements
and ElementWidth of a vector suffix. This reduces the duplication of
this functionality between two the vector implementations.

This is patch [1/6] in a series to add assembler/disassembler support for
SVE's contiguous ST1 (scalar+imm) instructions.

[fhahn]

nice cleanup, thanks. Just a few minor comments.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

1853 ↗

(On Diff #141601)

Please keep those comments.

1859 ↗

(On Diff #141601)

Can you add a brief comment to document what the values in the returned pair mean?

1866 ↗

(On Diff #141601)

Why are we matching "" for neon here? Looks like we did not match that before (on a first look)?

2871 ↗

(On Diff #141601)

Would it be enough to capture this here? Or just pass the location explicitly?

2877 ↗

(On Diff #141601)

This will trigger an unused variable warning when assertions are enabled I think. Maybe just move parseVectorKind in the assert?

[rengolin]

Indeed, nice cleanup. I think we discussed this earlier and it's a much welcome change, to merge NEON and SVE parsing.

Apart from the error message text, NFC. With Florian's comments, LGTM, thanks!

I'll let @fhahn approve when he's ready.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

1575 ↗

(On Diff #141601)

Maybe an assert making sure the arguments are compatible with vector registers?

[sdesmalen]

• Added a new comment describing what 'parseVectorKind' does.
• Put back some of the original comments into 'parseVectorKind'.
• Fixed assert in lambda function and updated the capture-list.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

1866 ↗

(On Diff #141601)

That is because this function can also be called when the Suffix is an empty string, where previously 'isValidVectorKind()' was only called when Kind was non-empty. See for instance 'tryParseNeonVectorRegister()' which unconditionally calls 'parseVectorKind()' in order to pass ElementWidth to the CreateVectorReg() call.

2877 ↗

(On Diff #141601)

If I put the 'parseVectorKind' itself in an assert, it might not get executed (which is needed for 'Kind' to be set, which is passed by reference). I'll rewrite it into a condition with the 'else' being a llvm_unreachable.

[fhahn]

LGTM

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

1876 ↗

(On Diff #141805)

The indents here seem weird. Can you run clang-format-diff before committing?

1866 ↗

(On Diff #141601)

Got it, thanks.

2877 ↗

(On Diff #141601)

Got it, thanks.

[sdesmalen]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

1876 ↗

(On Diff #141805)

This is actually what clang-format gave me, but I see that I'm using the clang-format installed on my system which isn't very recent, where a newer version fixes this. I'll update it before committing.

llvm/trunk/lib/Target/AArch64/AArch64RegisterInfo.td

class PPRAsmOperand <string name, string RegClass, int Width>: AsmOperandClass {
   let Name = "SVE" # name # "Reg";
   let PredicateMethod = "isSVEVectorRegOfWidth<"
                             # Width # ", " # "AArch64::" # RegClass # "RegClassID>";
   let DiagnosticType = "InvalidSVE" # name # "Reg";
   let RenderMethod = "addRegOperands";
   let ParserMethod = "tryParseSVEPredicateVector";
 }
 
-def PPRAsmOpAny : PPRAsmOperand<"PredicateAny", "PPR", -1>;
+def PPRAsmOpAny : PPRAsmOperand<"PredicateAny", "PPR",  0>;
 def PPRAsmOp8   : PPRAsmOperand<"PredicateB",   "PPR",  8>;
 def PPRAsmOp16  : PPRAsmOperand<"PredicateH",   "PPR", 16>;
 def PPRAsmOp32  : PPRAsmOperand<"PredicateS",   "PPR", 32>;
 def PPRAsmOp64  : PPRAsmOperand<"PredicateD",   "PPR", 64>;
 
 def PPRAny : PPRRegOp<"",  PPRAsmOpAny, PPR>;
 def PPR8   : PPRRegOp<"b", PPRAsmOp8,   PPR>;
 def PPR16  : PPRRegOp<"h", PPRAsmOp16,  PPR>;
 def PPR32  : PPRRegOp<"s", PPRAsmOp32,  PPR>;
 def PPR64  : PPRRegOp<"d", PPRAsmOp64,  PPR>;
 
-def PPRAsmOp3bAny : PPRAsmOperand<"Predicate3bAny", "PPR_3b", -1>;
+def PPRAsmOp3bAny : PPRAsmOperand<"Predicate3bAny", "PPR_3b",  0>;
 def PPRAsmOp3b8   : PPRAsmOperand<"Predicate3bB",   "PPR_3b",  8>;
 def PPRAsmOp3b16  : PPRAsmOperand<"Predicate3bH",   "PPR_3b", 16>;
 def PPRAsmOp3b32  : PPRAsmOperand<"Predicate3bS",   "PPR_3b", 32>;
 def PPRAsmOp3b64  : PPRAsmOperand<"Predicate3bD",   "PPR_3b", 64>;
 
 def PPR3bAny : PPRRegOp<"",  PPRAsmOp3bAny, PPR_3b>;
 def PPR3b8   : PPRRegOp<"b", PPRAsmOp3b8,   PPR_3b>;
 def PPR3b16  : PPRRegOp<"h", PPRAsmOp3b16,  PPR_3b>;

 }
 
 class ZPRAsmOperand <string name, int Width>: AsmOperandClass {
   let Name = "SVE" # name # "Reg";
   let PredicateMethod = "isSVEVectorRegOfWidth<"
                             # Width # ", AArch64::ZPRRegClassID>";
   let RenderMethod = "addRegOperands";
   let ParserMethod = "tryParseSVEDataVector<"
-                               # !if(!eq(Width, -1), "false", "true") # ">";
+                               # !if(!eq(Width, 0), "false", "true") # ">";
 }
 
-def ZPRAsmOpAny : ZPRAsmOperand<"VectorAny", -1>;
+def ZPRAsmOpAny : ZPRAsmOperand<"VectorAny", 0>;
 def ZPRAsmOp8   : ZPRAsmOperand<"VectorB",   8>;
 def ZPRAsmOp16  : ZPRAsmOperand<"VectorH",   16>;
 def ZPRAsmOp32  : ZPRAsmOperand<"VectorS",   32>;
 def ZPRAsmOp64  : ZPRAsmOperand<"VectorD",   64>;
 def ZPRAsmOp128 : ZPRAsmOperand<"VectorQ",   128>;
 
 def ZPRAny  : ZPRRegOp<"",  ZPRAsmOpAny, ZPR>;
 def ZPR8    : ZPRRegOp<"b", ZPRAsmOp8,   ZPR>;
 def ZPR16   : ZPRRegOp<"h", ZPRAsmOp16,  ZPR>;
 def ZPR32   : ZPRRegOp<"s", ZPRAsmOp32,  ZPR>;
 def ZPR64   : ZPRRegOp<"d", ZPRAsmOp64,  ZPR>;
 def ZPR128  : ZPRRegOp<"q", ZPRAsmOp128, ZPR>;

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

private:
   SMLoc getLoc() const { return getParser().getTok().getLoc(); }
 
   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);
   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,
+  OperandMatchResultTy tryParseVectorRegister(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);
template <unsigned Class> bool isSVEVectorReg() const {
 
     return (Kind == k_Register && Reg.Kind == RK) &&
            AArch64MCRegisterClasses[Class].contains(getReg());
   }
 
   template <int ElementWidth, unsigned Class>
   bool isSVEVectorRegOfWidth() const {
     return isSVEVectorReg<Class>() &&
-           (ElementWidth == -1 || Reg.ElementWidth == ElementWidth);
+           (ElementWidth == 0 || Reg.ElementWidth == ElementWidth);
   }
 
   bool isGPR32as64() const {
     return Kind == k_Register && Reg.Kind == RegKind::Scalar &&
       AArch64MCRegisterClasses[AArch64::GPR64RegClassID].contains(Reg.RegNum);
   }
 
   bool isWSeqPair() const {
CreateReg(unsigned RegNum, RegKind Kind, SMLoc S, SMLoc E, MCContext &Ctx) {
     Op->Reg.RegNum = RegNum;
     Op->Reg.Kind = Kind;
     Op->StartLoc = S;
     Op->EndLoc = E;
     return Op;
   }
 
   static std::unique_ptr<AArch64Operand>
-  CreateReg(unsigned RegNum, RegKind Kind, unsigned ElementWidth,
+  CreateVectorReg(unsigned RegNum, RegKind Kind, unsigned ElementWidth,
                   SMLoc S, SMLoc E, MCContext &Ctx) {
+    assert((Kind == RegKind::NeonVector || Kind == RegKind::SVEDataVector ||
+            Kind == RegKind::SVEPredicateVector) &&
+           "Invalid vector kind");
     auto Op = make_unique<AArch64Operand>(k_Register, Ctx);
     Op->Reg.RegNum = RegNum;
     Op->Reg.ElementWidth = ElementWidth;
     Op->Reg.Kind = Kind;
     Op->StartLoc = S;
     Op->EndLoc = E;
     return Op;
   }
 
   static std::unique_ptr<AArch64Operand>
   CreateVectorList(unsigned RegNum, unsigned Count, unsigned NumElements,
-                   char ElementKind, SMLoc S, SMLoc E, MCContext &Ctx) {
+                   unsigned ElementWidth, SMLoc S, SMLoc E, MCContext &Ctx) {
     auto Op = make_unique<AArch64Operand>(k_VectorList, Ctx);
     Op->VectorList.RegNum = RegNum;
     Op->VectorList.Count = Count;
     Op->VectorList.NumElements = NumElements;
-    Op->VectorList.ElementKind = ElementKind;
+    switch (ElementWidth) {
+    case 0:
+      Op->VectorList.ElementKind = 0;
+      break;
+    case 8:
+      Op->VectorList.ElementKind = 'b';
+      break;
+    case 16:
+      Op->VectorList.ElementKind = 'h';
+      break;
+    case 32:
+      Op->VectorList.ElementKind = 's';
+      break;
+    case 64:
+      Op->VectorList.ElementKind = 'd';
+      break;
+    default:
+      llvm_unreachable("Unsupported elementwidth");
+    }
     Op->StartLoc = S;
     Op->EndLoc = E;
     return Op;
   }
 
   static std::unique_ptr<AArch64Operand>
   CreateVectorIndex(unsigned Idx, SMLoc S, SMLoc E, MCContext &Ctx) {
     auto Op = make_unique<AArch64Operand>(k_VectorIndex, Ctx);
return StringSwitch<unsigned>(Name.lower())
       .Case("v27", AArch64::Q27)
       .Case("v28", AArch64::Q28)
       .Case("v29", AArch64::Q29)
       .Case("v30", AArch64::Q30)
       .Case("v31", AArch64::Q31)
       .Default(0);
 }
 
-static bool isValidVectorKind(StringRef Name) {
-  return StringSwitch<bool>(Name.lower())
-      .Case(".8b", true)
-      .Case(".16b", true)
-      .Case(".4h", true)
-      .Case(".8h", true)
-      .Case(".2s", true)
-      .Case(".4s", true)
-      .Case(".1d", true)
-      .Case(".2d", true)
-      .Case(".1q", true)
+/// Returns an optional pair of (#elements, element-width) if Suffix
+/// is a valid vector kind. Where the number of elements in a vector
+/// or the vector width is implicit or explicitly unknown (but still a
+/// valid suffix kind), 0 is used.
+static Optional<std::pair<int, int>> parseVectorKind(StringRef Suffix,
+                                                     RegKind VectorKind) {
+  std::pair<int, int> Res = {-1, -1};
+
+  switch (VectorKind) {
+  case RegKind::NeonVector:
+    Res =
+        StringSwitch<std::pair<int, int>>(Suffix.lower())
+            .Case("", {0, 0})
+            .Case(".1d", {1, 64})
+            .Case(".1q", {1, 128})
+            // '.2h' needed for fp16 scalar pairwise reductions
+            .Case(".2h", {2, 16})
+            .Case(".2s", {2, 32})
+            .Case(".2d", {2, 64})
+            // '.4b' is another special case for the ARMv8.2a dot product
+            // operand
+            .Case(".4b", {4, 8})
+            .Case(".4h", {4, 16})
+            .Case(".4s", {4, 32})
+            .Case(".8b", {8, 8})
+            .Case(".8h", {8, 16})
+            .Case(".16b", {16, 8})
             // Accept the width neutral ones, too, for verbose syntax. If those
             // aren't used in the right places, the token operand won't match so
             // all will work out.
-      .Case(".b", true)
-      .Case(".h", true)
-      .Case(".s", true)
-      .Case(".d", true)
-      // Needed for fp16 scalar pairwise reductions
-      .Case(".2h", true)
-      // another special case for the ARMv8.2a dot product operand
-      .Case(".4b", true)
-      .Default(false);
+            .Case(".b", {0, 8})
+            .Case(".h", {0, 16})
+            .Case(".s", {0, 32})
+            .Case(".d", {0, 64})
+            .Default({-1, -1});
+    break;
+  case RegKind::SVEPredicateVector:
+  case RegKind::SVEDataVector:
+    Res = StringSwitch<std::pair<int, int>>(Suffix.lower())
+              .Case("", {0, 0})
+              .Case(".b", {0, 8})
+              .Case(".h", {0, 16})
+              .Case(".s", {0, 32})
+              .Case(".d", {0, 64})
+              .Case(".q", {0, 128})
+              .Default({-1, -1});
+    break;
+  default:
+    llvm_unreachable("Unsupported RegKind");
+  }
+
+  if (Res == std::make_pair(-1, -1))
+    return Optional<std::pair<int, int>>();
+
+  return Optional<std::pair<int, int>>(Res);
+}
+
+static bool isValidVectorKind(StringRef Suffix, RegKind VectorKind) {
+  return parseVectorKind(Suffix, VectorKind).hasValue();
 }
 
 static unsigned matchSVEDataVectorRegName(StringRef Name) {
   return StringSwitch<unsigned>(Name.lower())
       .Case("z0", AArch64::Z0)
       .Case("z1", AArch64::Z1)
       .Case("z2", AArch64::Z2)
       .Case("z3", AArch64::Z3)
return StringSwitch<unsigned>(Name.lower())
       .Case("p11", AArch64::P11)
       .Case("p12", AArch64::P12)
       .Case("p13", AArch64::P13)
       .Case("p14", AArch64::P14)
       .Case("p15", AArch64::P15)
       .Default(0);
 }
 
-static bool isValidSVEKind(StringRef Name) {
-  return StringSwitch<bool>(Name.lower())
-      .Case(".b", true)
-      .Case(".h", true)
-      .Case(".s", true)
-      .Case(".d", true)
-      .Case(".q", true)
-      .Default(false);
-}
-
-static void parseValidVectorKind(StringRef Name, unsigned &NumElements,
-                                 char &ElementKind) {
-  assert(isValidVectorKind(Name));
-
-  ElementKind = Name.lower()[Name.size() - 1];
-  NumElements = 0;
-
-  if (Name.size() == 2)
-    return;
-
-  // Parse the lane count
-  Name = Name.drop_front();
-  while (isdigit(Name.front())) {
-    NumElements = 10 * NumElements + (Name.front() - '0');
-    Name = Name.drop_front();
-  }
-}
-
 bool AArch64AsmParser::ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
                                      SMLoc &EndLoc) {
   StartLoc = getLoc();
   RegNo = tryParseRegister();
   EndLoc = SMLoc::getFromPointer(getLoc().getPointer() - 1);
   return (RegNo == (unsigned)-1);
 }
 
int AArch64AsmParser::tryParseRegister() {
 
   if (RegNum == 0)
     return -1;
 
   Parser.Lex(); // Eat identifier token.
   return RegNum;
 }
 
-/// tryMatchVectorRegister - Try to parse a vector register name with optional
-/// kind specifier. If it is a register specifier, eat the token and return it.
-int AArch64AsmParser::tryMatchVectorRegister(StringRef &Kind, bool expected) {
-  MCAsmParser &Parser = getParser();
-  if (Parser.getTok().isNot(AsmToken::Identifier)) {
-    TokError("vector register expected");
-    return -1;
-  }
-
-  StringRef Name = Parser.getTok().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::NeonVector);
-
-  if (RegNum) {
-    if (Next != StringRef::npos) {
-      Kind = Name.slice(Next, StringRef::npos);
-      if (!isValidVectorKind(Kind)) {
-        TokError("invalid vector kind qualifier");
-        return -1;
-      }
-    }
-    Parser.Lex(); // Eat the register token.
-    return RegNum;
-  }
-
-  if (expected)
-    TokError("vector register expected");
-  return -1;
-}
-
 /// tryParseSysCROperand - Try to parse a system instruction CR operand name.
 OperandMatchResultTy
 AArch64AsmParser::tryParseSysCROperand(OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
   SMLoc S = getLoc();
 
   if (Parser.getTok().isNot(AsmToken::Identifier)) {
     Error(S, "Expected cN operand where 0 <= N <= 15");

 bool AArch64AsmParser::tryParseNeonVectorRegister(OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
   if (Parser.getTok().isNot(AsmToken::Identifier))
     return true;
 
   SMLoc S = getLoc();
   // Check for a vector register specifier first.
   StringRef Kind;
-  int64_t Reg = tryMatchVectorRegister(Kind, false);
-  if (Reg == -1)
+  int Reg = -1;
+  OperandMatchResultTy Res =
+      tryParseVectorRegister(Reg, Kind, RegKind::NeonVector);
+  if (Res != MatchOperand_Success)
+    return true;
+
+  const auto &KindRes = parseVectorKind(Kind, RegKind::NeonVector);
+  if (!KindRes)
     return true;
+
+  unsigned ElementWidth = KindRes->second;
   Operands.push_back(
-      AArch64Operand::CreateReg(Reg, RegKind::NeonVector, S, getLoc(),
-                                getContext()));
+      AArch64Operand::CreateVectorReg(Reg, RegKind::NeonVector, ElementWidth,
+                                      S, getLoc(), getContext()));
 
   // If there was an explicit qualifier, that goes on as a literal text
   // operand.
   if (!Kind.empty())
     Operands.push_back(
         AArch64Operand::CreateToken(Kind, false, S, getContext()));
 
   // If there is an index specifier following the register, parse that too.
if (parseOptionalToken(AsmToken::LBrac)) {
 
     Operands.push_back(AArch64Operand::CreateVectorIndex(MCE->getValue(), SIdx,
                                                          E, getContext()));
   }
 
   return false;
 }
 
-// tryParseSVEDataVectorRegister - Try to parse a SVE vector register name with
+// tryParseVectorRegister - Try to parse a vector register name with
 // optional kind specifier. If it is a register specifier, eat the token
 // and return it.
 OperandMatchResultTy
-AArch64AsmParser::tryParseSVERegister(int &Reg, StringRef &Kind,
+AArch64AsmParser::tryParseVectorRegister(int &Reg, StringRef &Kind,
                                          RegKind MatchKind) {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
 
   if (Tok.isNot(AsmToken::Identifier))
     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, MatchKind);
 
   if (RegNum) {
     if (Next != StringRef::npos) {
       Kind = Name.slice(Next, StringRef::npos);
-      if (!isValidSVEKind(Kind)) {
-        TokError("invalid sve vector kind qualifier");
+      if (!isValidVectorKind(Kind, MatchKind)) {
+        TokError("invalid vector kind qualifier");
         return MatchOperand_ParseFail;
       }
     }
     Parser.Lex(); // Eat the register token.
 
     Reg = RegNum;
     return MatchOperand_Success;
   }
 
   return MatchOperand_NoMatch;
 }
 
 /// tryParseSVEPredicateVector - Parse a SVE predicate register operand.
 OperandMatchResultTy
 AArch64AsmParser::tryParseSVEPredicateVector(OperandVector &Operands) {
   // Check for a SVE predicate register specifier first.
   const SMLoc S = getLoc();
   StringRef Kind;
   int RegNum = -1;
-  auto Res = tryParseSVERegister(RegNum, Kind, RegKind::SVEPredicateVector);
+  auto Res = tryParseVectorRegister(RegNum, Kind, RegKind::SVEPredicateVector);
   if (Res != MatchOperand_Success)
     return Res;
 
-  unsigned ElementWidth = StringSwitch<unsigned>(Kind.lower())
-                              .Case("", -1)
-                              .Case(".b", 8)
-                              .Case(".h", 16)
-                              .Case(".s", 32)
-                              .Case(".d", 64)
-                              .Case(".q", 128)
-                              .Default(0);
-
-  if (!ElementWidth)
+  const auto &KindRes = parseVectorKind(Kind, RegKind::SVEPredicateVector);
+  if (!KindRes)
     return MatchOperand_NoMatch;
 
-  Operands.push_back(
-      AArch64Operand::CreateReg(RegNum, RegKind::SVEPredicateVector,
-                                ElementWidth, S, getLoc(), getContext()));
+  unsigned ElementWidth = KindRes->second;
+  Operands.push_back(AArch64Operand::CreateVectorReg(
+      RegNum, RegKind::SVEPredicateVector, ElementWidth, S,
+      getLoc(), getContext()));
 
   // Not all predicates are followed by a '/m' or '/z'.
   MCAsmParser &Parser = getParser();
   if (Parser.getTok().isNot(AsmToken::Slash))
     return MatchOperand_Success;
 
   // But when they do they shouldn't have an element type suffix.
   if (!Kind.empty()) {
bool AArch64AsmParser::parseSymbolicImmVal(const MCExpr *&ImmVal) {
 
   return false;
 }
 
 /// parseVectorList - Parse a vector list operand for AdvSIMD instructions.
 bool AArch64AsmParser::parseVectorList(OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
   assert(Parser.getTok().is(AsmToken::LCurly) && "Token is not a Left Bracket");
+
+  // Wrapper around parse function
+  auto ParseVector = [this](int &Reg, StringRef &Kind, SMLoc Loc) {
+    if (tryParseVectorRegister(Reg, Kind, RegKind::NeonVector) ==
+        MatchOperand_Success) {
+      if (parseVectorKind(Kind, RegKind::NeonVector))
+        return true;
+      llvm_unreachable("Expected a valid vector kind");
+    }
+
+    Error(Loc, "vector register expected");
+    return false;
+  };
+
   SMLoc S = getLoc();
   Parser.Lex(); // Eat left bracket token.
   StringRef Kind;
-  int64_t FirstReg = tryMatchVectorRegister(Kind, true);
-  if (FirstReg == -1)
+  int FirstReg = -1;
+  if (!ParseVector(FirstReg, Kind, getLoc()))
      return true;
+
   int64_t PrevReg = FirstReg;
   unsigned Count = 1;
 
   if (parseOptionalToken(AsmToken::Minus)) {
     SMLoc Loc = getLoc();
     StringRef NextKind;
-    int64_t Reg = tryMatchVectorRegister(NextKind, true);
-    if (Reg == -1)
+
+    int Reg;
+    if (!ParseVector(Reg, NextKind, getLoc()))
       return true;
+
     // Any Kind suffices must match on all regs in the list.
     if (Kind != NextKind)
       return Error(Loc, "mismatched register size suffix");
 
     unsigned Space = (PrevReg < Reg) ? (Reg - PrevReg) : (Reg + 32 - PrevReg);
 
     if (Space == 0 || Space > 3) {
       return Error(Loc, "invalid number of vectors");
     }
 
     Count += Space;
   }
   else {
     while (parseOptionalToken(AsmToken::Comma)) {
       SMLoc Loc = getLoc();
       StringRef NextKind;
-      int64_t Reg = tryMatchVectorRegister(NextKind, true);
-      if (Reg == -1)
+      int Reg;
+      if (!ParseVector(Reg, NextKind, getLoc()))
         return true;
       // Any Kind suffices must match on all regs in the list.
       if (Kind != NextKind)
         return Error(Loc, "mismatched register size suffix");
 
       // Registers must be incremental (with wraparound at 31)
       if (getContext().getRegisterInfo()->getEncodingValue(Reg) !=
           (getContext().getRegisterInfo()->getEncodingValue(PrevReg) + 1) % 32)
        return Error(Loc, "registers must be sequential");
 
       PrevReg = Reg;
       ++Count;
     }
   }
 
   if (parseToken(AsmToken::RCurly, "'}' expected"))
     return true;
 
   if (Count > 4)
     return Error(S, "invalid number of vectors");
 
   unsigned NumElements = 0;
-  char ElementKind = 0;
-  if (!Kind.empty())
-    parseValidVectorKind(Kind, NumElements, ElementKind);
+  unsigned ElementWidth = 0;
+  if (!Kind.empty()) {
+    if (const auto &VK = parseVectorKind(Kind, RegKind::NeonVector))
+      std::tie(NumElements, ElementWidth) = *VK;
+  }
 
   Operands.push_back(AArch64Operand::CreateVectorList(
-      FirstReg, Count, NumElements, ElementKind, S, getLoc(), getContext()));
+      FirstReg, Count, NumElements, ElementWidth, S, getLoc(), getContext()));
 
   // If there is an index specifier following the list, parse that too.
   SMLoc SIdx = getLoc();
   if (parseOptionalToken(AsmToken::LBrac)) { // Eat left bracket token.
     const MCExpr *ImmVal;
     if (getParser().parseExpression(ImmVal))
       return false;
     const MCConstantExpr *MCE = dyn_cast<MCConstantExpr>(ImmVal);
bool AArch64AsmParser::parseDirectiveReq(StringRef Name, SMLoc L) {
   Parser.Lex(); // Eat the '.req' token.
   SMLoc SRegLoc = getLoc();
   int RegNum = tryParseRegister();
   RegKind RegisterKind = RegKind::Scalar;
 
   if (RegNum == -1) {
     StringRef Kind;
     RegisterKind = RegKind::NeonVector;
-    RegNum = tryMatchVectorRegister(Kind, false);
-    if (!Kind.empty())
+    OperandMatchResultTy Res =
+      tryParseVectorRegister(RegNum, Kind, RegKind::NeonVector);
+
+    if (Res == MatchOperand_ParseFail)
+      return true;
+
+    if (Res == MatchOperand_Success && !Kind.empty())
       return Error(SRegLoc, "vector register without type specifier expected");
   }
 
   if (RegNum == -1) {
     StringRef Kind;
     RegisterKind = RegKind::SVEDataVector;
     OperandMatchResultTy Res =
-        tryParseSVERegister(RegNum, Kind, RegKind::SVEDataVector);
+        tryParseVectorRegister(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 == -1) {
     StringRef Kind;
     RegisterKind = RegKind::SVEPredicateVector;
     OperandMatchResultTy Res =
-        tryParseSVERegister(RegNum, Kind, RegKind::SVEPredicateVector);
+        tryParseVectorRegister(RegNum, Kind, RegKind::SVEPredicateVector);
 
     if (Res == MatchOperand_ParseFail)
       return true;
 
     if (Res == MatchOperand_Success && !Kind.empty())
       return Error(SRegLoc,
                    "sve predicate register without type specifier expected");
   }

 OperandMatchResultTy
 AArch64AsmParser::tryParseSVEDataVector(OperandVector &Operands) {
   const SMLoc S = getLoc();
   // Check for a SVE vector register specifier first.
   int RegNum = -1;
   StringRef Kind;
 
   OperandMatchResultTy Res =
-      tryParseSVERegister(RegNum, Kind, RegKind::SVEDataVector);
+      tryParseVectorRegister(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)
-                        .Case(".s", 32)
-                        .Case(".d", 64)
-                        .Case(".q", 128)
-                        .Default(0);
-  if (!ElementWidth)
+  const auto &KindRes = parseVectorKind(Kind, RegKind::SVEDataVector);
+  if (!KindRes)
     return MatchOperand_NoMatch;
 
-  Operands.push_back(
-    AArch64Operand::CreateReg(RegNum, RegKind::SVEDataVector, ElementWidth,
-                              S, S, getContext()));
+  unsigned ElementWidth = KindRes->second;
+  Operands.push_back(AArch64Operand::CreateVectorReg(
+      RegNum, RegKind::SVEDataVector, ElementWidth, S, S,
+      getContext()));
 
   return MatchOperand_Success;
 }
 
 OperandMatchResultTy
 AArch64AsmParser::tryParseSVEPattern(OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
 

llvm/trunk/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: [[@LINE-1]]:{{[0-9]+}}: error: invalid 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]+}}:

llvm/trunk/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: [[@LINE-1]]:{{[0-9]+}}: error: invalid 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]+}}:

llvm/trunk/test/MC/AArch64/SVE/zip1-diagnostics.s

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

llvm/trunk/test/MC/AArch64/SVE/zip2-diagnostics.s

 // RUN: not llvm-mc -triple=aarch64 -show-encoding -mattr=+sve  2>&1 < %s| FileCheck %s
 
 // Invalid element kind.
 zip2 z6.h, z23.h, z31.x
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid sve vector kind qualifier
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid vector kind qualifier
 // CHECK-NEXT: zip2 z6.h, z23.h, z31.x
 // CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
 
 // Element size specifiers should match.
 zip2 z0.h, z30.h, z24.b
 // CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand
 // CHECK-NEXT: zip2 z0.h, z30.h, z24.b
 // CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}: