[RISCV] Simplify matchRegisterNameHelper interface. NFC

[craig.topper]

Summary

This previously returned a bool to indicate success or failure and
returns a register through an output parameter.

Some callers used the bool to check for success. Some callers checked
for RISCV::NoRegister.

To make everything uniform, return the MCRegister directly and update
all callers to use MCRegister::isValid().

[VincentWu]

LGTM

[VincentWu]

plz update usage of matchRegisterNameHelper in function parseReglist at llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

tkx )

[barannikov88]

Just a few suggestions, not mandatory.

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1516

Stray //

1518

Since you are refactoring this, RegNo could be renamed to Reg.

1525–1530

Why change this? NoRegister was fine.

1546

1549

Just if (!RegNo)?

[craig.topper]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1549

Isn't that relying on the operator unsigned that's supposed to get removed from MCRegister eventually?

[barannikov88]

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

1549

It does currently, but I think adding explicit operator bool() const to MCRegister should be fine?

[craig.topper]

Rebase and address comments

[barannikov88]

LGTM
There is still // in the comment.

llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

bool RISCVAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
   }
   }
 
   llvm_unreachable("Unknown match type detected!");
 }
 
 // Attempts to match Name as a register (either using the default name or
 // alternative ABI names), setting RegNo to the matching register. Upon
-// failure, returns true and sets RegNo to 0. If IsRVE then registers
-// x16-x31 will be rejected.
-static bool matchRegisterNameHelper(bool IsRVE, MCRegister &RegNo,
-                                    StringRef Name) {
-  RegNo = MatchRegisterName(Name);
+// failure, returns a non-valid MCRegister. If IsRVE, then registers x16-x31
+// will be rejected.
+static MCRegister matchRegisterNameHelper(bool IsRVE, StringRef Name) {
+  MCRegister Reg = MatchRegisterName(Name);
   // The 16-/32- and 64-bit FPRs have the same asm name. Check that the initial
   // match always matches the 64-bit variant, and not the 16/32-bit one.
-  assert(!(RegNo >= RISCV::F0_H && RegNo <= RISCV::F31_H));
-  assert(!(RegNo >= RISCV::F0_F && RegNo <= RISCV::F31_F));
+  assert(!(Reg >= RISCV::F0_H && Reg <= RISCV::F31_H));
+  assert(!(Reg >= RISCV::F0_F && Reg <= RISCV::F31_F));
   // The default FPR register class is based on the tablegen enum ordering.
   static_assert(RISCV::F0_D < RISCV::F0_H, "FPR matching must be updated");
   static_assert(RISCV::F0_D < RISCV::F0_F, "FPR matching must be updated");
-  if (RegNo == RISCV::NoRegister)
-    RegNo = MatchRegisterAltName(Name);
-  if (IsRVE && RegNo >= RISCV::X16 && RegNo <= RISCV::X31)
-    RegNo = RISCV::NoRegister;
-  return RegNo == RISCV::NoRegister;
+  if (!Reg)
+    Reg = MatchRegisterAltName(Name);
+  if (IsRVE && Reg >= RISCV::X16 && Reg <= RISCV::X31)
+    Reg = RISCV::NoRegister;
+  return Reg;
 }
 
 bool RISCVAsmParser::parseRegister(MCRegister &RegNo, SMLoc &StartLoc,
                                    SMLoc &EndLoc) {
   if (tryParseRegister(RegNo, StartLoc, EndLoc) != MatchOperand_Success)
     return Error(StartLoc, "invalid register name");
   return false;
 }
 
 OperandMatchResultTy RISCVAsmParser::tryParseRegister(MCRegister &RegNo,
                                                       SMLoc &StartLoc,
                                                       SMLoc &EndLoc) {
   const AsmToken &Tok = getParser().getTok();
   StartLoc = Tok.getLoc();
   EndLoc = Tok.getEndLoc();
-  RegNo = 0;
   StringRef Name = getLexer().getTok().getIdentifier();
 
-  if (matchRegisterNameHelper(isRVE(), (MCRegister &)RegNo, Name))
+  RegNo = matchRegisterNameHelper(isRVE(), Name);
+  if (!RegNo)
     return MatchOperand_NoMatch;
 
   getParser().Lex(); // Eat identifier token.
   return MatchOperand_Success;
 }
 
 OperandMatchResultTy RISCVAsmParser::parseRegister(OperandVector &Operands,
                                                    bool AllowParens) {
OperandMatchResultTy RISCVAsmParser::parseRegister(OperandVector &Operands,
 
   switch (getLexer().getKind()) {
   default:
     if (HadParens)
       getLexer().UnLex(LParen);
     return MatchOperand_NoMatch;
   case AsmToken::Identifier:
     StringRef Name = getLexer().getTok().getIdentifier();
-    MCRegister RegNo;
-    matchRegisterNameHelper(isRVE(), RegNo, Name);
+    MCRegister RegNo = matchRegisterNameHelper(isRVE(), Name);
 
-    if (RegNo == RISCV::NoRegister) {
+    if (!RegNo) {
       if (HadParens)
         getLexer().UnLex(LParen);
       return MatchOperand_NoMatch;
     }
     if (HadParens)
       Operands.push_back(RISCVOperand::createToken("(", FirstS));
     SMLoc S = getLoc();
     SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
OperandMatchResultTy RISCVAsmParser::parseMaskReg(OperandVector &Operands) {
   if (getLexer().isNot(AsmToken::Identifier))
     return MatchOperand_NoMatch;
 
   StringRef Name = getLexer().getTok().getIdentifier();
   if (!Name.consume_back(".t")) {
     Error(getLoc(), "expected '.t' suffix");
     return MatchOperand_ParseFail;
   }
-  MCRegister RegNo;
-  matchRegisterNameHelper(isRVE(), RegNo, Name);
+  MCRegister RegNo = matchRegisterNameHelper(isRVE(), Name);
 
-  if (RegNo == RISCV::NoRegister)
+  if (!RegNo)
     return MatchOperand_NoMatch;
   if (RegNo != RISCV::V0)
     return MatchOperand_NoMatch;
   SMLoc S = getLoc();
   SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
   getLexer().Lex();
   Operands.push_back(RISCVOperand::createReg(RegNo, S, E));
   return MatchOperand_Success;
 }
 
 OperandMatchResultTy RISCVAsmParser::parseGPRAsFPR(OperandVector &Operands) {
   if (getLexer().isNot(AsmToken::Identifier))
     return MatchOperand_NoMatch;
 
   StringRef Name = getLexer().getTok().getIdentifier();
-  MCRegister RegNo;
-  matchRegisterNameHelper(isRVE(), RegNo, Name);
+  MCRegister RegNo = matchRegisterNameHelper(isRVE(), Name);
 
-  if (RegNo == RISCV::NoRegister)
+  if (!RegNo)
     return MatchOperand_NoMatch;
   SMLoc S = getLoc();
   SMLoc E = SMLoc::getFromPointer(S.getPointer() + Name.size());
   getLexer().Lex();
   Operands.push_back(RISCVOperand::createReg(
       RegNo, S, E, !getSTI().hasFeature(RISCV::FeatureStdExtF)));
   return MatchOperand_Success;
 }
OperandMatchResultTy RISCVAsmParser::parseReglist(OperandVector &Operands) {
   SMLoc S = getLoc();
   if (getLexer().isNot(AsmToken::LCurly)) {
     Error(getLoc(), "register list must start with '{'");
     return MatchOperand_ParseFail;
   }
   getLexer().Lex(); // eat '{'
   bool IsEABI = isRVE();
 
-  MCRegister RegStart = RISCV::NoRegister;
-  MCRegister RegEnd = RISCV::NoRegister;
   StringRef RegName = getLexer().getTok().getIdentifier();
-  matchRegisterNameHelper(IsEABI, RegStart, RegName);
+  MCRegister RegStart = matchRegisterNameHelper(IsEABI, RegName);
+  MCRegister RegEnd;
   if (RegStart != RISCV::X1) {
     Error(getLoc(), "register list must start from 'ra' or 'x1'");
     return MatchOperand_ParseFail;
   }
   getLexer().Lex();
 
   // parse case like ,s0
   if (getLexer().is(AsmToken::Comma)) {
     getLexer().Lex();
     if (getLexer().isNot(AsmToken::Identifier)) {
       Error(getLoc(), "invalid register");
       return MatchOperand_ParseFail;
     }
     StringRef RegName = getLexer().getTok().getIdentifier();
-    if (matchRegisterNameHelper(IsEABI, RegStart, RegName)) {
+    RegStart = matchRegisterNameHelper(IsEABI, RegName);
+    if (!RegStart) {
       Error(getLoc(), "invalid register");
       return MatchOperand_ParseFail;
     }
     if (RegStart != RISCV::X8) {
       Error(getLoc(), "continuous register list must start from 's0' or 'x8'");
       return MatchOperand_ParseFail;
     }
     getLexer().Lex(); // eat reg
   }
 
   // parse case like -s1
   if (getLexer().is(AsmToken::Minus)) {
     getLexer().Lex();
     StringRef EndName = getLexer().getTok().getIdentifier();
     // FIXME: the register mapping and checks of EABI is wrong
-    if (matchRegisterNameHelper(IsEABI, RegEnd, EndName)) {
+    RegEnd = matchRegisterNameHelper(IsEABI, EndName);
+    if (!RegEnd) {
       Error(getLoc(), "invalid register");
       return MatchOperand_ParseFail;
     }
     if (IsEABI && RegEnd != RISCV::X9) {
       Error(getLoc(), "contiguous register list of EABI can only be 's0-s1' or "
                       "'x8-x9' pair");
       return MatchOperand_ParseFail;
     }