[AArch64][SVE] Asm: Replace 'IsVector' by 'RegKind' in AArch64AsmParser (NFC)

[sdesmalen]

Summary

Patch [2/5] in a series to add assembler/disassembler support for AArch64 SVE unpredicated ADD/SUB instructions.

This change is a non functional change that adds RegKind as an alternative to 'isVector' to prepare it for newer types (SVE data vectors and predicate vectors) that will be added in next patches (where the SVE data vector is added as part of this patch set)

[fhahn]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

62 ↗

(On Diff #119568)

Would it make sense to use something more generic for the names here, like ScalableVector?

811 ↗

(On Diff #119568)

Personally I think having predicate functions like Reg.isScalar(), Reg.isNeonVector() would be slightly more compact, but I think either way is fine.

812 ↗

(On Diff #119568)

From looking at the functions around here, it seems like a space would be good here.

1889 ↗

(On Diff #119568)

What is the reason for moving this code out to a separate function? Is it to not change the interface of matchRegisterNameAlias, which still takes bool isVector as an argument?

1905 ↗

(On Diff #119568)

I think keeping the boolean isVector argument has potential to confuse people. Any reason for not being explicit here and pass RegKind here? Because there are 2 vector register types now, isVector seems ambiguous.

1911 ↗

(On Diff #119568)

space between matchRegisterNameAlias and ()?

2589 ↗

(On Diff #119568)

Any reason for not passing RegKind::NeonVector directly here? It seems like RegisterKind is not used anywhere else around here.

2590 ↗

(On Diff #119568)

stray new line?

2771 ↗

(On Diff #119568)

unrelated change?

[javed.absar]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

62 ↗

(On Diff #119568)

It seems to be it would be more maintainable to split it

• RegKind { Scalar, Vector }; and then

enum class VectorKind { NeonVector, SVEVector};

This might simplify the checking logic further down.

[sdesmalen]

Refactored code to reflect some of the suggestions.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

62 ↗

(On Diff #119568)

@fhahn A subsequent patch will also add a SVE Predicate Vector.

@javed.absar I had a look at splitting it into RegKind { Scalar, Vector }; and subsequently into VectorKind {NeonVector, SVEVector}, but I don't easily see how this makes the checking logic any simpler. Did you have a specific check that you think would be simplified by this?

811 ↗

(On Diff #119568)

I think that is basically what we're implementing here, but hen for 'isReg()'. See for example 'isVectorReg()', which is the opposite of 'isScalar()'. I see now that isVectorReg() is only checking for a NeonVector, which seems inconsistent with its name. I'll see if I can fix that.

1889 ↗

(On Diff #119568)

We initially moved it into a separate function to reuse it for 'matchSVEVectorNameAlias()' (as added in patch 3/5)... But, looking at it again I see it makes more sense to add to the same function and use RegKind for each (which eliminates the purpose of moving this into a separate function). Please check if you're happy with the next iteration of this patch.

1905 ↗

(On Diff #119568)

should be addressed by the next iteration of this patch (see previous comment)

[rengolin]

So, overall, this is a sensible change. We must be able to separate Neon vectors from SVE vectors. I'm not sure how the predicates will fare here and why they wouldn't be SVE vectors (surely, they're scalable, too).

I would prefer to keep isVector and add isScalable, and move the distinction between Neon and SVE to helper functions. But I'm not holding this opinion firmly, as I haven't seen the rest of the code.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

813 ↗

(On Diff #119661)

Is there a situation (in the future) that we might want?

bool isVector() const { return Kind == k_Register && !Reg.isScalar; }

1898 ↗

(On Diff #119661)

function naming consistency.

1912 ↗

(On Diff #119661)

not that it matters much but the return change here is pointless. :)

62 ↗

(On Diff #119568)

Why won't SVE predicate not be an "SVEVector"?

2771 ↗

(On Diff #119568)

just checked, this is not violating 80-columns. :)

[sdesmalen]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

813 ↗

(On Diff #119661)

At the moment I haven't found such a situation yet (even when considering other SVE assembler/disassembler patches) and I would rather not add code that isn't used anywhere.
So I suggest adding that only when needed.

62 ↗

(On Diff #119568)

Fair point, it should probably distinguish Data and Predicate in the name. I'll change this.
(also, since this is not yet used until the next patch in this series, I will move this change to that patch)

[rengolin]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

813 ↗

(On Diff #119661)

Makes sense

[rengolin]

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

62 ↗

(On Diff #120089)

nit: space alignment.

[rengolin]

Adding a few AArch64 folks, to make sure we get enough eyes on this, thus avoiding silly problems in the future, if we miss anything.

[sdesmalen]

Removed whitespace.

[fhahn]

A few minor nits, but this looks good to me now. @rengolin what do you think?

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

1909 ↗

(On Diff #120243)

nit: unrelated whitespace change

1914 ↗

(On Diff #120243)

nit: unrelated whitespace change.

2589 ↗

(On Diff #119568)

Do we need the RegisterKind variable? Doesn't look like it is used somewhere else.

[sdesmalen]

• Propagated 'RegKind::NeonVector' for variable RegisterKind with single use.
• Removed newline.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

1909 ↗

(On Diff #120243)

I don't feel strongly about it, but I think it improves readability.

2589 ↗

(On Diff #119568)

I thought I had addressed it in my previous patch, but I must have missed it. Please see updated patch.

[rengolin]

Small nit, but overall, looks good to me.

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

2627 ↗

(On Diff #121001)

nit: shouldn't this function be called tryParseNeonVectorRegister, then?

[fhahn]

Thanks Sander, looks good to me too, with the nit @rengolin pointed out.

[sdesmalen]

Renamed tryParseVectorRegister into tryParseNeonVectorRegister.

[rengolin]

This looks good to me, thanks!

Let's wait for a final round of comments.

[fhahn]

Looks good to me now.

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

def QQQQ : RegisterClass<"AArch64", [untyped], 128, (add QSeqQuads)> {
   let Size = 512;
 }
 
 
 // Vector operand versions of the FP registers. Alternate name printing and
 // assmebler matching.
 def VectorReg64AsmOperand : AsmOperandClass {
   let Name = "VectorReg64";
-  let PredicateMethod = "isVectorReg";
+  let PredicateMethod = "isNeonVectorReg";
 }
 def VectorReg128AsmOperand : AsmOperandClass {
   let Name = "VectorReg128";
-  let PredicateMethod = "isVectorReg";
+  let PredicateMethod = "isNeonVectorReg";
 }
 
 def V64  : RegisterOperand<FPR64, "printVRegOperand"> {
   let ParserMatchClass = VectorReg64AsmOperand;
 }
 
 def V128 : RegisterOperand<FPR128, "printVRegOperand"> {
   let ParserMatchClass = VectorReg128AsmOperand;
 }
 
-def VectorRegLoAsmOperand : AsmOperandClass { let Name = "VectorRegLo"; }
+def VectorRegLoAsmOperand : AsmOperandClass {
+  let Name = "VectorRegLo";
+  let PredicateMethod = "isNeonVectorRegLo";
+}
 def V128_lo : RegisterOperand<FPR128_lo, "printVRegOperand"> {
   let ParserMatchClass = VectorRegLoAsmOperand;
 }
 
 class TypedVecListAsmOperand<int count, int regsize, int lanes, string kind>
     : AsmOperandClass {
   let Name = "TypedVectorList" # count # "_" # lanes # kind;
 

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

 #include <tuple>
 #include <utility>
 #include <vector>
 
 using namespace llvm;
 
 namespace {
 
+enum class RegKind {Scalar, NeonVector};
+
 class AArch64AsmParser : public MCTargetAsmParser {
 private:
   StringRef Mnemonic; ///< Instruction mnemonic.
 
   // Map of register aliases registers via the .req directive.
-  StringMap<std::pair<bool, unsigned>> RegisterReqs;
+  StringMap<std::pair<RegKind, unsigned>> RegisterReqs;
 
   AArch64TargetStreamer &getTargetStreamer() {
     MCTargetStreamer &TS = *getParser().getStreamer().getTargetStreamer();
     return static_cast<AArch64TargetStreamer &>(TS);
   }
 
   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, bool isVector);
+  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);
 
#include "AArch64GenAsmMatcher.inc"
   OperandMatchResultTy tryParseSysCROperand(OperandVector &Operands);
   OperandMatchResultTy tryParsePrefetch(OperandVector &Operands);
   OperandMatchResultTy tryParsePSBHint(OperandVector &Operands);
   OperandMatchResultTy tryParseAdrpLabel(OperandVector &Operands);
   OperandMatchResultTy tryParseAdrLabel(OperandVector &Operands);
   OperandMatchResultTy tryParseFPImm(OperandVector &Operands);
   OperandMatchResultTy tryParseAddSubImm(OperandVector &Operands);
   OperandMatchResultTy tryParseGPR64sp0Operand(OperandVector &Operands);
-  bool tryParseVectorRegister(OperandVector &Operands);
+  bool tryParseNeonVectorRegister(OperandVector &Operands);
   OperandMatchResultTy tryParseGPRSeqPair(OperandVector &Operands);
 
 public:
   enum AArch64MatchResultTy {
     Match_InvalidSuffix = FIRST_TARGET_MATCH_RESULT_TY,
 #define GET_OPERAND_DIAGNOSTIC_TYPES
 #include "AArch64GenAsmMatcher.inc"
   };
private:
   struct TokOp {
     const char *Data;
     unsigned Length;
     bool IsSuffix; // Is the operand actually a suffix on the mnemonic.
   };
 
   struct RegOp {
     unsigned RegNum;
-    bool isVector;
+    RegKind Kind;
   };
 
   struct VectorListOp {
     unsigned RegNum;
     unsigned Count;
     unsigned NumElements;
     unsigned ElementKind;
   };
return (SysReg.PStateField == AArch64PState::PAN ||
             SysReg.PStateField == AArch64PState::UAO);
   }
 
   bool isSystemPStateFieldWithImm0_15() const {
     if (!isSysReg() || isSystemPStateFieldWithImm0_1()) return false;
     return SysReg.PStateField != -1U;
   }
 
-  bool isReg() const override { return Kind == k_Register && !Reg.isVector; }
-  bool isVectorReg() const { return Kind == k_Register && Reg.isVector; }
+  bool isReg() const override {
+    return Kind == k_Register && Reg.Kind == RegKind::Scalar;
+  }
+
+  bool isNeonVectorReg() const {
+    return Kind == k_Register && Reg.Kind == RegKind::NeonVector;
+  }
 
-  bool isVectorRegLo() const {
-    return Kind == k_Register && Reg.isVector &&
+  bool isNeonVectorRegLo() const {
+    return Kind == k_Register && Reg.Kind == RegKind::NeonVector &&
            AArch64MCRegisterClasses[AArch64::FPR128_loRegClassID].contains(
                Reg.RegNum);
   }
 
   bool isGPR32as64() const {
-    return Kind == k_Register && !Reg.isVector &&
+    return Kind == k_Register && Reg.Kind == RegKind::Scalar &&
       AArch64MCRegisterClasses[AArch64::GPR64RegClassID].contains(Reg.RegNum);
   }
 
   bool isWSeqPair() const {
-    return Kind == k_Register && !Reg.isVector &&
+    return Kind == k_Register && Reg.Kind == RegKind::Scalar &&
            AArch64MCRegisterClasses[AArch64::WSeqPairsClassRegClassID].contains(
                Reg.RegNum);
   }
 
   bool isXSeqPair() const {
-    return Kind == k_Register && !Reg.isVector &&
+    return Kind == k_Register && Reg.Kind == RegKind::Scalar &&
            AArch64MCRegisterClasses[AArch64::XSeqPairsClassRegClassID].contains(
                Reg.RegNum);
   }
 
   bool isGPR64sp0() const {
-    return Kind == k_Register && !Reg.isVector &&
+    return Kind == k_Register && Reg.Kind == RegKind::Scalar &&
       AArch64MCRegisterClasses[AArch64::GPR64spRegClassID].contains(Reg.RegNum);
   }
 
   template<int64_t Angle, int64_t Remainder>
   bool isComplexRotation() const {
     if (!isImm()) return false;
 
     const MCConstantExpr *CE = dyn_cast<MCConstantExpr>(getImm());
CreateToken(StringRef Str, bool IsSuffix, SMLoc S, MCContext &Ctx) {
     Op->Tok.Length = Str.size();
     Op->Tok.IsSuffix = IsSuffix;
     Op->StartLoc = S;
     Op->EndLoc = S;
     return Op;
   }
 
   static std::unique_ptr<AArch64Operand>
-  CreateReg(unsigned RegNum, bool isVector, SMLoc S, SMLoc E, MCContext &Ctx) {
+  CreateReg(unsigned RegNum, RegKind Kind, SMLoc S, SMLoc E, MCContext &Ctx) {
     auto Op = make_unique<AArch64Operand>(k_Register, Ctx);
     Op->Reg.RegNum = RegNum;
-    Op->Reg.isVector = isVector;
+    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) {

 
 /// @name Auto-generated Match Functions
 /// {
 
 static unsigned MatchRegisterName(StringRef Name);
 
 /// }
 
-static unsigned matchVectorRegName(StringRef Name) {
+static unsigned MatchNeonVectorRegName(StringRef Name) {
   return StringSwitch<unsigned>(Name.lower())
       .Case("v0", AArch64::Q0)
       .Case("v1", AArch64::Q1)
       .Case("v2", AArch64::Q2)
       .Case("v3", AArch64::Q3)
       .Case("v4", AArch64::Q4)
       .Case("v5", AArch64::Q5)
       .Case("v6", AArch64::Q6)
bool AArch64AsmParser::ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
   StartLoc = getLoc();
   RegNo = tryParseRegister();
   EndLoc = SMLoc::getFromPointer(getLoc().getPointer() - 1);
   return (RegNo == (unsigned)-1);
 }
 
 // Matches a register name or register alias previously defined by '.req'
 unsigned AArch64AsmParser::matchRegisterNameAlias(StringRef Name,
-                                                  bool isVector) {
-  unsigned RegNum = isVector ? matchVectorRegName(Name)
-                             : MatchRegisterName(Name);
+                                                  RegKind Kind) {
+  unsigned RegNum;
+  switch (Kind) {
+  case RegKind::Scalar:
+    RegNum = MatchRegisterName(Name);
+    break;
+  case RegKind::NeonVector:
+    RegNum = MatchNeonVectorRegName(Name);
+    break;
+  }
 
-  if (RegNum == 0) {
+  if (!RegNum) {
     // Check for aliases registered via .req. Canonicalize to lower case.
     // That's more consistent since register names are case insensitive, and
     // it's how the original entry was passed in from MC/MCParser/AsmParser.
     auto Entry = RegisterReqs.find(Name.lower());
     if (Entry == RegisterReqs.end())
       return 0;
+
     // set RegNum if the match is the right kind of register
-    if (isVector == Entry->getValue().first)
+    if (Kind == Entry->getValue().first)
       RegNum = Entry->getValue().second;
   }
   return RegNum;
 }
 
 /// tryParseRegister - Try to parse a register name. The token must be an
 /// 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();
-  unsigned RegNum = matchRegisterNameAlias(lowerCase, false);
+  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);
if (Parser.getTok().isNot(AsmToken::Identifier)) {
     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, true);
+  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;
       }
AArch64AsmParser::tryParseSysReg(OperandVector &Operands) {
   Operands.push_back(
       AArch64Operand::CreateSysReg(Tok.getString(), getLoc(), MRSReg, MSRReg,
                                    PStateImm, getContext()));
   Parser.Lex(); // Eat identifier
 
   return MatchOperand_Success;
 }
 
-/// tryParseVectorRegister - Parse a vector register operand.
-bool AArch64AsmParser::tryParseVectorRegister(OperandVector &Operands) {
+/// tryParseNeonVectorRegister - Parse a vector register operand.
+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)
     return true;
   Operands.push_back(
-      AArch64Operand::CreateReg(Reg, true, S, getLoc(), getContext()));
+      AArch64Operand::CreateReg(Reg, RegKind::NeonVector, 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.
   SMLoc SIdx = getLoc();
bool AArch64AsmParser::tryParseNeonVectorRegister(OperandVector &Operands) {
   }
 
   return false;
 }
 
 /// parseRegister - Parse a non-vector register operand.
 bool AArch64AsmParser::parseRegister(OperandVector &Operands) {
   SMLoc S = getLoc();
-  // Try for a vector register.
-  if (!tryParseVectorRegister(Operands))
+  // Try for a vector (neon) register.
+  if (!tryParseNeonVectorRegister(Operands))
     return false;
 
   // Try for a scalar register.
   int64_t Reg = tryParseRegister();
   if (Reg == -1)
     return true;
-  Operands.push_back(
-      AArch64Operand::CreateReg(Reg, false, S, getLoc(), getContext()));
+  Operands.push_back(AArch64Operand::CreateReg(Reg, RegKind::Scalar, S,
+      getLoc(), getContext()));
 
   return false;
 }
 
 bool AArch64AsmParser::parseSymbolicImmVal(const MCExpr *&ImmVal) {
   MCAsmParser &Parser = getParser();
   bool HasELFModifier = false;
   AArch64MCExpr::VariantKind RefKind;

 
 OperandMatchResultTy
 AArch64AsmParser::tryParseGPR64sp0Operand(OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
   if (!Tok.is(AsmToken::Identifier))
     return MatchOperand_NoMatch;
 
-  unsigned RegNum = matchRegisterNameAlias(Tok.getString().lower(), false);
+  unsigned RegNum = matchRegisterNameAlias(Tok.getString().lower(), RegKind::Scalar);
 
   MCContext &Ctx = getContext();
   const MCRegisterInfo *RI = Ctx.getRegisterInfo();
   if (!RI->getRegClass(AArch64::GPR64spRegClassID).contains(RegNum))
     return MatchOperand_NoMatch;
 
   SMLoc S = getLoc();
   Parser.Lex(); // Eat register
 
   if (!parseOptionalToken(AsmToken::Comma)) {
     Operands.push_back(
-        AArch64Operand::CreateReg(RegNum, false, S, getLoc(), Ctx));
+        AArch64Operand::CreateReg(RegNum, RegKind::Scalar, S, getLoc(), Ctx));
     return MatchOperand_Success;
   }
 
   parseOptionalToken(AsmToken::Hash);
 
   if (Parser.getTok().isNot(AsmToken::Integer)) {
     Error(getLoc(), "index must be absent or #0");
     return MatchOperand_ParseFail;
   }
 
   const MCExpr *ImmVal;
   if (Parser.parseExpression(ImmVal) || !isa<MCConstantExpr>(ImmVal) ||
       cast<MCConstantExpr>(ImmVal)->getValue() != 0) {
     Error(getLoc(), "index must be absent or #0");
     return MatchOperand_ParseFail;
   }
 
   Operands.push_back(
-      AArch64Operand::CreateReg(RegNum, false, S, getLoc(), Ctx));
+      AArch64Operand::CreateReg(RegNum, RegKind::Scalar, S, getLoc(), Ctx));
   return MatchOperand_Success;
 }
 
 /// parseOperand - Parse a arm instruction operand.  For now this parses the
 /// operand regardless of the mnemonic.
 bool AArch64AsmParser::parseOperand(OperandVector &Operands, bool isCondCode,
                                   bool invertCondCode) {
   MCAsmParser &Parser = getParser();
if (Op1.isReg() && LSBOp.isImm() && WidthOp.isImm()) {
           return Error(WidthOp.getStartLoc(),
                        "requested insert overflows register");
 
         const MCExpr *ImmRExpr = MCConstantExpr::create(ImmR, getContext());
         const MCExpr *ImmSExpr = MCConstantExpr::create(ImmS, getContext());
         Operands[0] = AArch64Operand::CreateToken(
               "bfm", false, Op.getStartLoc(), getContext());
         Operands[2] = AArch64Operand::CreateReg(
-            RegWidth == 32 ? AArch64::WZR : AArch64::XZR, false, SMLoc(),
-            SMLoc(), getContext());
+            RegWidth == 32 ? AArch64::WZR : AArch64::XZR, RegKind::Scalar,
+            SMLoc(), SMLoc(), getContext());
         Operands[3] = AArch64Operand::CreateImm(
             ImmRExpr, LSBOp.getStartLoc(), LSBOp.getEndLoc(), getContext());
         Operands.emplace_back(
             AArch64Operand::CreateImm(ImmSExpr, WidthOp.getStartLoc(),
                                       WidthOp.getEndLoc(), getContext()));
       }
     }
   } else if (NumOperands == 5) {
bool AArch64AsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
   //        InstAlias can't quite handle this since the reg classes aren't
   //        subclasses.
   if (NumOperands == 3 && (Tok == "sxtw" || Tok == "uxtw")) {
     // The source register can be Wn here, but the matcher expects a
     // GPR64. Twiddle it here if necessary.
     AArch64Operand &Op = static_cast<AArch64Operand &>(*Operands[2]);
     if (Op.isReg()) {
       unsigned Reg = getXRegFromWReg(Op.getReg());
-      Operands[2] = AArch64Operand::CreateReg(Reg, false, Op.getStartLoc(),
-                                              Op.getEndLoc(), getContext());
+      Operands[2] = AArch64Operand::CreateReg(Reg, RegKind::Scalar,
+                                              Op.getStartLoc(), Op.getEndLoc(),
+                                              getContext());
     }
   }
   // FIXME: Likewise for sxt[bh] with a Xd dst operand
   else if (NumOperands == 3 && (Tok == "sxtb" || Tok == "sxth")) {
     AArch64Operand &Op = static_cast<AArch64Operand &>(*Operands[1]);
     if (Op.isReg() &&
         AArch64MCRegisterClasses[AArch64::GPR64allRegClassID].contains(
             Op.getReg())) {
       // The source register can be Wn here, but the matcher expects a
       // GPR64. Twiddle it here if necessary.
       AArch64Operand &Op = static_cast<AArch64Operand &>(*Operands[2]);
       if (Op.isReg()) {
         unsigned Reg = getXRegFromWReg(Op.getReg());
-        Operands[2] = AArch64Operand::CreateReg(Reg, false, Op.getStartLoc(),
+        Operands[2] = AArch64Operand::CreateReg(Reg, RegKind::Scalar,
+                                                Op.getStartLoc(),
                                                 Op.getEndLoc(), getContext());
       }
     }
   }
   // FIXME: Likewise for uxt[bh] with a Xd dst operand
   else if (NumOperands == 3 && (Tok == "uxtb" || Tok == "uxth")) {
     AArch64Operand &Op = static_cast<AArch64Operand &>(*Operands[1]);
     if (Op.isReg() &&
         AArch64MCRegisterClasses[AArch64::GPR64allRegClassID].contains(
             Op.getReg())) {
       // The source register can be Wn here, but the matcher expects a
       // GPR32. Twiddle it here if necessary.
       AArch64Operand &Op = static_cast<AArch64Operand &>(*Operands[1]);
       if (Op.isReg()) {
         unsigned Reg = getWRegFromXReg(Op.getReg());
-        Operands[1] = AArch64Operand::CreateReg(Reg, false, Op.getStartLoc(),
+        Operands[1] = AArch64Operand::CreateReg(Reg, RegKind::Scalar,
+                                                Op.getStartLoc(),
                                                 Op.getEndLoc(), getContext());
       }
     }
   }
 
   MCInst Inst;
   // First try to match against the secondary set of tables containing the
   // short-form NEON instructions (e.g. "fadd.2s v0, v1, v2").

 
 /// 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();
-  bool IsVector = false;
+  RegKind RegisterKind = RegKind::Scalar;
 
   if (RegNum == static_cast<unsigned>(-1)) {
     StringRef Kind;
+    RegisterKind = RegKind::NeonVector;
     RegNum = tryMatchVectorRegister(Kind, false);
     if (!Kind.empty())
       return Error(SRegLoc, "vector register without type specifier expected");
-    IsVector = true;
   }
 
   if (RegNum == static_cast<unsigned>(-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(IsVector, RegNum);
+  auto pair = std::make_pair(RegisterKind, 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
AArch64AsmParser::tryParseGPRSeqPair(OperandVector &Operands) {
   if (isXReg) {
     Pair = RI->getMatchingSuperReg(FirstReg, AArch64::sube64,
            &AArch64MCRegisterClasses[AArch64::XSeqPairsClassRegClassID]);
   } else {
     Pair = RI->getMatchingSuperReg(FirstReg, AArch64::sube32,
            &AArch64MCRegisterClasses[AArch64::WSeqPairsClassRegClassID]);
   }
 
-  Operands.push_back(AArch64Operand::CreateReg(Pair, false, S, getLoc(),
-      getContext()));
+  Operands.push_back(AArch64Operand::CreateReg(Pair, RegKind::Scalar, S,
+      getLoc(), getContext()));
 
   return MatchOperand_Success;
 }