[AArch64][AsmParser] NFC: Cleanup parsing of scalar registers.

[sdesmalen]

Summary

• Renamed tryParseRegister to tryParseScalarRegister, which now returns an OperandMatchResultTy.
• Moved matching of certain aliases into matchRegisterNameAlias.
• Changed type of most 'Reg' variables to 'unsigned'.

This is patch [1/4] in a series to add assembler/disassembler support for
SVE's contiguous LD1 (scalar+scalar) instructions:

• Patch [1/4]: https://reviews.llvm.org/D45687
• Patch [2/4]: https://reviews.llvm.org/D45688
• Patch [3/4]: https://reviews.llvm.org/D45689
• Patch [4/4]: https://reviews.llvm.org/D45690

[sdesmalen]

• Fixed 'typo' where a condition expression tested the wrong variable in tryParseScalarRegister ('RegNum' vs 'Reg').

[samparker]

Hi Sander,

This is a nice cleanup. LGTM with the one inline nit.

cheers,
sam

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

4537 ↗

(On Diff #142741)

RegisterKind variable can be removed right? Same below too.

[sdesmalen]

In D45687#1070654, @samparker wrote:

This is a nice cleanup. LGTM with the one inline nit.

Thanks!

lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp

4537 ↗

(On Diff #142741)

RegisterKind is still used for the pair that is inserted into RegisterReqs, so that it can be looked up for that register kind in matchRegisterNameAlias.

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();
   bool parseRegister(OperandVector &Operands);
   bool parseSymbolicImmVal(const MCExpr *&ImmVal);
   bool parseNeonVectorList(OperandVector &Operands);
   bool parseOptionalMulVl(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 tryParseVectorRegister(int &Reg, StringRef &Kind,
+  OperandMatchResultTy tryParseScalarRegister(unsigned &Reg);
+  OperandMatchResultTy tryParseVectorRegister(unsigned &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);
return StringSwitch<unsigned>(Name.lower())
       .Case("p14", AArch64::P14)
       .Case("p15", AArch64::P15)
       .Default(0);
 }
 
 bool AArch64AsmParser::ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
                                      SMLoc &EndLoc) {
   StartLoc = getLoc();
-  RegNo = tryParseRegister();
+  auto Res = tryParseScalarRegister(RegNo);
   EndLoc = SMLoc::getFromPointer(getLoc().getPointer() - 1);
-  return (RegNo == (unsigned)-1);
+  return Res != MatchOperand_Success;
 }
 
 // Matches a register name or register alias previously defined by '.req'
 unsigned AArch64AsmParser::matchRegisterNameAlias(StringRef Name,
                                                   RegKind Kind) {
   unsigned RegNum = 0;
   if ((RegNum = matchSVEDataVectorRegName(Name)))
     return Kind == RegKind::SVEDataVector ? RegNum : 0;
 
   if ((RegNum = matchSVEPredicateVectorRegName(Name)))
     return Kind == RegKind::SVEPredicateVector ? RegNum : 0;
 
   if ((RegNum = MatchNeonVectorRegName(Name)))
     return Kind == RegKind::NeonVector ? RegNum : 0;
 
   // The parsed register must be of RegKind Scalar
   if ((RegNum = MatchRegisterName(Name)))
     return Kind == RegKind::Scalar ? RegNum : 0;
 
   if (!RegNum) {
+    // Handle a few common aliases of registers.
+    if (auto RegNum = StringSwitch<unsigned>(Name.lower())
+                    .Case("fp", AArch64::FP)
+                    .Case("lr",  AArch64::LR)
+                    .Case("x31", AArch64::XZR)
+                    .Case("w31", AArch64::WZR)
+                    .Default(0))
+      return Kind == RegKind::Scalar ? RegNum : 0;
+
     // 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 (Kind == Entry->getValue().first)
       RegNum = Entry->getValue().second;
   }
   return RegNum;
 }
 
-/// tryParseRegister - Try to parse a register name. The token must be an
+/// tryParseScalarRegister - 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() {
+OperandMatchResultTy
+AArch64AsmParser::tryParseScalarRegister(unsigned &RegNum) {
   MCAsmParser &Parser = getParser();
   const AsmToken &Tok = Parser.getTok();
   if (Tok.isNot(AsmToken::Identifier))
-    return -1;
+    return MatchOperand_NoMatch;
 
   std::string lowerCase = Tok.getString().lower();
-  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 (RegNum == 0)
-    return -1;
+  unsigned Reg = matchRegisterNameAlias(lowerCase, RegKind::Scalar);
+  if (Reg == 0)
+    return MatchOperand_NoMatch;
 
+  RegNum = Reg;
   Parser.Lex(); // Eat identifier token.
-  return RegNum;
+  return MatchOperand_Success;
 }
 
 /// tryParseSysCROperand - Try to parse a system instruction CR operand name.
 OperandMatchResultTy
 AArch64AsmParser::tryParseSysCROperand(OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
   SMLoc S = getLoc();
 

 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;
-  int Reg = -1;
+  unsigned Reg;
   OperandMatchResultTy Res =
       tryParseVectorRegister(Reg, Kind, RegKind::NeonVector);
   if (Res != MatchOperand_Success)
     return true;
 
   const auto &KindRes = parseVectorKind(Kind, RegKind::NeonVector);
   if (!KindRes)
     return true;
AArch64AsmParser::tryParseVectorIndex(OperandVector &Operands) {
 
   return MatchOperand_NoMatch;
 }
 
 // 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::tryParseVectorRegister(int &Reg, StringRef &Kind,
+AArch64AsmParser::tryParseVectorRegister(unsigned &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();

 }
 
 /// 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;
+  unsigned RegNum;
   auto Res = tryParseVectorRegister(RegNum, Kind, RegKind::SVEPredicateVector);
   if (Res != MatchOperand_Success)
     return Res;
 
   const auto &KindRes = parseVectorKind(Kind, RegKind::SVEPredicateVector);
   if (!KindRes)
     return MatchOperand_NoMatch;
 
AArch64AsmParser::tryParseSVEPredicateVector(OperandVector &Operands) {
   const char *ZM = Pred == "z" ? "z" : "m";
   Operands.push_back(
     AArch64Operand::CreateToken(ZM, false, getLoc(), getContext()));
 
   Parser.Lex(); // Eat zero/merge token.
   return MatchOperand_Success;
 }
 
-/// parseRegister - Parse a non-vector register operand.
+/// parseRegister - Parse a register operand.
 bool AArch64AsmParser::parseRegister(OperandVector &Operands) {
   SMLoc S = getLoc();
-  // Try for a vector (neon) register.
+  // Try for a Neon vector register.
   if (!tryParseNeonVectorRegister(Operands))
     return false;
 
   // Try for a scalar register.
-  int64_t Reg = tryParseRegister();
-  if (Reg == -1)
+  unsigned Reg;
+  if (tryParseScalarRegister(Reg) != MatchOperand_Success)
     return true;
   Operands.push_back(AArch64Operand::CreateReg(Reg, RegKind::Scalar, S,
       getLoc(), getContext()));
 
   return false;
 }
 
 bool AArch64AsmParser::parseSymbolicImmVal(const MCExpr *&ImmVal) {

 OperandMatchResultTy
 AArch64AsmParser::tryParseVectorList(OperandVector &Operands,
                                      bool ExpectMatch) {
   MCAsmParser &Parser = getParser();
   if (!Parser.getTok().is(AsmToken::LCurly))
     return MatchOperand_NoMatch;
 
   // Wrapper around parse function
-  auto ParseVector = [this, &Parser](int &Reg, StringRef &Kind, SMLoc Loc,
+  auto ParseVector = [this, &Parser](unsigned &Reg, StringRef &Kind, SMLoc Loc,
                                      bool NoMatchIsError) {
     auto RegTok = Parser.getTok();
     auto ParseRes = tryParseVectorRegister(Reg, Kind, VectorKind);
     if (ParseRes == MatchOperand_Success) {
-      if (parseVectorKind(Kind, RegKind::NeonVector))
+      if (parseVectorKind(Kind, VectorKind))
         return ParseRes;
       llvm_unreachable("Expected a valid vector kind");
     }
 
     if (RegTok.isNot(AsmToken::Identifier) ||
         ParseRes == MatchOperand_ParseFail ||
         (ParseRes == MatchOperand_NoMatch && NoMatchIsError)) {
       Error(Loc, "vector register expected");
       return MatchOperand_ParseFail;
     }
 
     return MatchOperand_NoMatch;
   };
 
   SMLoc S = getLoc();
   auto LCurly = Parser.getTok();
   Parser.Lex(); // Eat left bracket token.
 
   StringRef Kind;
-  int FirstReg = -1;
+  unsigned FirstReg;
   auto ParseRes = ParseVector(FirstReg, Kind, getLoc(), ExpectMatch);
 
   // Put back the original left bracket if there was no match, so that
   // different types of list-operands can be matched (e.g. SVE, Neon).
   if (ParseRes == MatchOperand_NoMatch)
     Parser.getLexer().UnLex(LCurly);
 
   if (ParseRes != MatchOperand_Success)
     return ParseRes;
 
   int64_t PrevReg = FirstReg;
   unsigned Count = 1;
 
   if (parseOptionalToken(AsmToken::Minus)) {
     SMLoc Loc = getLoc();
     StringRef NextKind;
 
-    int Reg;
+    unsigned Reg;
     ParseRes = ParseVector(Reg, NextKind, getLoc(), true);
     if (ParseRes != MatchOperand_Success)
       return ParseRes;
 
     // Any Kind suffices must match on all regs in the list.
     if (Kind != NextKind) {
       Error(Loc, "mismatched register size suffix");
       return MatchOperand_ParseFail;
     }
 
     unsigned Space = (PrevReg < Reg) ? (Reg - PrevReg) : (Reg + 32 - PrevReg);
 
     if (Space == 0 || Space > 3) {
       Error(Loc, "invalid number of vectors");
       return MatchOperand_ParseFail;
     }
 
     Count += Space;
   }
   else {
     while (parseOptionalToken(AsmToken::Comma)) {
       SMLoc Loc = getLoc();
       StringRef NextKind;
-      int Reg;
+      unsigned Reg;
       ParseRes = ParseVector(Reg, NextKind, getLoc(), true);
       if (ParseRes != MatchOperand_Success)
         return ParseRes;
 
       // Any Kind suffices must match on all regs in the list.
       if (Kind != NextKind) {
         Error(Loc, "mismatched register size suffix");
         return MatchOperand_ParseFail;
bool AArch64AsmParser::parseNeonVectorList(OperandVector &Operands) {
   if (ParseRes != MatchOperand_Success)
     return true;
 
   return tryParseVectorIndex(Operands) == MatchOperand_ParseFail;
 }
 
 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(), RegKind::Scalar);
+  SMLoc StartLoc = getLoc();
 
-  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
+  unsigned RegNum;
+  OperandMatchResultTy Res = tryParseScalarRegister(RegNum);
+  if (Res != MatchOperand_Success)
+    return Res;
 
   if (!parseOptionalToken(AsmToken::Comma)) {
-    Operands.push_back(
-        AArch64Operand::CreateReg(RegNum, RegKind::Scalar, S, getLoc(), Ctx));
+    Operands.push_back(AArch64Operand::CreateReg(
+        RegNum, RegKind::Scalar, StartLoc, getLoc(), getContext()));
     return MatchOperand_Success;
   }
 
   parseOptionalToken(AsmToken::Hash);
 
-  if (Parser.getTok().isNot(AsmToken::Integer)) {
+  if (getParser().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) ||
+  if (getParser().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, RegKind::Scalar, S, getLoc(), Ctx));
+  Operands.push_back(AArch64Operand::CreateReg(
+      RegNum, RegKind::Scalar, StartLoc, getLoc(), getContext()));
   return MatchOperand_Success;
 }
 
 bool AArch64AsmParser::parseOptionalMulVl(OperandVector &Operands) {
   MCAsmParser &Parser = getParser();
 
   // Some SVE instructions have a decoration after the immediate, i.e.
   // "mul vl". We parse them here and add tokens, which must be present in the

 }
 
 /// parseDirectiveReq
 ///  ::= name .req registername
 bool AArch64AsmParser::parseDirectiveReq(StringRef Name, SMLoc L) {
   MCAsmParser &Parser = getParser();
   Parser.Lex(); // Eat the '.req' token.
   SMLoc SRegLoc = getLoc();
-  int RegNum = tryParseRegister();
   RegKind RegisterKind = RegKind::Scalar;
+  unsigned RegNum;
+  OperandMatchResultTy ParseRes = tryParseScalarRegister(RegNum);
 
-  if (RegNum == -1) {
+  if (ParseRes != MatchOperand_Success) {
     StringRef Kind;
     RegisterKind = RegKind::NeonVector;
-    OperandMatchResultTy Res =
-      tryParseVectorRegister(RegNum, Kind, RegKind::NeonVector);
+    ParseRes = tryParseVectorRegister(RegNum, Kind, RegKind::NeonVector);
 
-    if (Res == MatchOperand_ParseFail)
+    if (ParseRes == MatchOperand_ParseFail)
       return true;
 
-    if (Res == MatchOperand_Success && !Kind.empty())
+    if (ParseRes == MatchOperand_Success && !Kind.empty())
       return Error(SRegLoc, "vector register without type specifier expected");
   }
 
-  if (RegNum == -1) {
+  if (ParseRes != MatchOperand_Success) {
     StringRef Kind;
     RegisterKind = RegKind::SVEDataVector;
-    OperandMatchResultTy Res =
+    ParseRes =
         tryParseVectorRegister(RegNum, Kind, RegKind::SVEDataVector);
 
-    if (Res == MatchOperand_ParseFail)
+    if (ParseRes == MatchOperand_ParseFail)
       return true;
 
-    if (Res == MatchOperand_Success && !Kind.empty())
+    if (ParseRes == MatchOperand_Success && !Kind.empty())
       return Error(SRegLoc,
                    "sve vector register without type specifier expected");
   }
 
-  if (RegNum == -1) {
+  if (ParseRes != MatchOperand_Success) {
     StringRef Kind;
     RegisterKind = RegKind::SVEPredicateVector;
-    OperandMatchResultTy Res =
-        tryParseVectorRegister(RegNum, Kind, RegKind::SVEPredicateVector);
+    ParseRes = tryParseVectorRegister(RegNum, Kind, RegKind::SVEPredicateVector);
 
-    if (Res == MatchOperand_ParseFail)
+    if (ParseRes == MatchOperand_ParseFail)
       return true;
 
-    if (Res == MatchOperand_Success && !Kind.empty())
+    if (ParseRes == MatchOperand_Success && !Kind.empty())
       return Error(SRegLoc,
                    "sve predicate register without type specifier expected");
   }
 
-  if (RegNum == -1)
+  if (ParseRes != MatchOperand_Success)
     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, (unsigned) RegNum);
AArch64AsmParser::tryParseGPRSeqPair(OperandVector &Operands) {
 
   SMLoc S = getLoc();
 
   if (getParser().getTok().isNot(AsmToken::Identifier)) {
     Error(S, "expected register");
     return MatchOperand_ParseFail;
   }
 
-  int FirstReg = tryParseRegister();
-  if (FirstReg == -1) {
+  unsigned FirstReg;
+  OperandMatchResultTy Res = tryParseScalarRegister(FirstReg);
+  if (Res != MatchOperand_Success)
     return MatchOperand_ParseFail;
-  }
+
   const MCRegisterClass &WRegClass =
       AArch64MCRegisterClasses[AArch64::GPR32RegClassID];
   const MCRegisterClass &XRegClass =
       AArch64MCRegisterClasses[AArch64::GPR64RegClassID];
 
   bool isXReg = XRegClass.contains(FirstReg),
        isWReg = WRegClass.contains(FirstReg);
   if (!isXReg && !isWReg) {
     Error(S, "expected first even register of a "
              "consecutive same-size even/odd register pair");
     return MatchOperand_ParseFail;
   }
 
   const MCRegisterInfo *RI = getContext().getRegisterInfo();
   unsigned FirstEncoding = RI->getEncodingValue(FirstReg);
 
   if (FirstEncoding & 0x1) {
     Error(S, "expected first even register of a "
              "consecutive same-size even/odd register pair");
     return MatchOperand_ParseFail;
   }
 
-  SMLoc M = getLoc();
   if (getParser().getTok().isNot(AsmToken::Comma)) {
-    Error(M, "expected comma");
+    Error(getLoc(), "expected comma");
     return MatchOperand_ParseFail;
   }
   // Eat the comma
   getParser().Lex();
 
   SMLoc E = getLoc();
-  int SecondReg = tryParseRegister();
-  if (SecondReg ==-1) {
+  unsigned SecondReg;
+  Res = tryParseScalarRegister(SecondReg);
+  if (Res != MatchOperand_Success)
     return MatchOperand_ParseFail;
-  }
 
   if (RI->getEncodingValue(SecondReg) != FirstEncoding + 1 ||
       (isXReg && !XRegClass.contains(SecondReg)) ||
       (isWReg && !WRegClass.contains(SecondReg))) {
     Error(E,"expected second odd register of a "
              "consecutive same-size even/odd register pair");
     return MatchOperand_ParseFail;
   }
AArch64AsmParser::tryParseGPRSeqPair(OperandVector &Operands) {
   return MatchOperand_Success;
 }
 
 template <bool ParseSuffix>
 OperandMatchResultTy
 AArch64AsmParser::tryParseSVEDataVector(OperandVector &Operands) {
   const SMLoc S = getLoc();
   // Check for a SVE vector register specifier first.
-  int RegNum = -1;
+  unsigned RegNum;
   StringRef Kind;
 
   OperandMatchResultTy Res =
       tryParseVectorRegister(RegNum, Kind, RegKind::SVEDataVector);
 
   if (Res != MatchOperand_Success)
     return Res;