[AMDGPU][MC] Corrected parsing of registers

[dp]

Summary

See bug 41280: https://bugs.llvm.org/show_bug.cgi?id=41280

[dp]

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

1768 ↗

(On Diff #194902)

This function is not strictly necessary for this change but it is required for D60624.

[artem.tamazov]

Looks good.

llvm/trunk/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

bool ParseToEndDirective(const char *AssemblerDirectiveBegin,
                            std::string &CollectString);
 
   bool AddNextRegisterToList(unsigned& Reg, unsigned& RegWidth,
                              RegisterKind RegKind, unsigned Reg1,
                              unsigned RegNum);
   bool ParseAMDGPURegister(RegisterKind& RegKind, unsigned& Reg,
                            unsigned& RegNum, unsigned& RegWidth,
                            unsigned *DwordRegIndex);
+  bool isRegister();
+  bool isRegister(const AsmToken &Token, const AsmToken &NextToken) const;
   Optional<StringRef> getGprCountSymbolName(RegisterKind RegKind);
   void initializeGprCountSymbol(RegisterKind RegKind);
   bool updateGprCountSymbols(RegisterKind RegKind, unsigned DwordRegIndex,
                              unsigned RegWidth);
   void cvtMubufImpl(MCInst &Inst, const OperandVector &Operands,
                     bool IsAtomic, bool IsAtomicReturn, bool IsLds = false);
   void cvtDSImpl(MCInst &Inst, const OperandVector &Operands,
                  bool IsGdsHardcoded);
private:
   bool validateMIMGGatherDMask(const MCInst &Inst);
   bool validateMIMGDataSize(const MCInst &Inst);
   bool validateMIMGD16(const MCInst &Inst);
   bool validateLdsDirect(const MCInst &Inst);
   bool usesConstantBus(const MCInst &Inst, unsigned OpIdx);
   bool isInlineConstant(const MCInst &Inst, unsigned OpIdx) const;
   unsigned findImplicitSGPRReadInVOP(const MCInst &Inst) const;
 
+  bool isId(const StringRef Id) const;
+  bool isId(const AsmToken &Token, const StringRef Id) const;
+  bool isToken(const AsmToken::TokenKind Kind) const;
   bool trySkipId(const StringRef Id);
   bool trySkipToken(const AsmToken::TokenKind Kind);
   bool skipToken(const AsmToken::TokenKind Kind, const StringRef ErrMsg);
   bool parseString(StringRef &Val, const StringRef ErrMsg = "expected a string");
+  AsmToken::TokenKind getTokenKind() const;
   bool parseExpr(int64_t &Imm);
+  AsmToken peekToken();
+  AsmToken getToken() const;
+  SMLoc getLoc() const;
+  void lex();
 
 public:
   OperandMatchResultTy parseOptionalOperand(OperandVector &Operands);
   OperandMatchResultTy parseOptionalOpr(OperandVector &Operands);
 
   OperandMatchResultTy parseExpTgt(OperandVector &Operands);
   OperandMatchResultTy parseSendMsgOp(OperandVector &Operands);
   OperandMatchResultTy parseInterpSlot(OperandVector &Operands);
case IS_TTMP:
     }
     RegWidth++;
     return true;
   default:
     llvm_unreachable("unexpected register kind");
   }
 }
 
+static const StringRef Registers[] = {
+  { "v" },
+  { "s" },
+  { "ttmp" },
+};
+
+bool
+AMDGPUAsmParser::isRegister(const AsmToken &Token,
+                            const AsmToken &NextToken) const {
+
+  // A list of consecutive registers: [s0,s1,s2,s3]
+  if (Token.is(AsmToken::LBrac))
+    return true;
+
+  if (!Token.is(AsmToken::Identifier))
+    return false;
+
+  // A single register like s0 or a range of registers like s[0:1]
+
+  StringRef RegName = Token.getString();
+
+  for (StringRef Reg : Registers) {
+    if (RegName.startswith(Reg)) {
+      if (Reg.size() < RegName.size()) {
+        unsigned RegNum;
+        // A single register with an index: rXX
+        if (!RegName.substr(Reg.size()).getAsInteger(10, RegNum))
+          return true;
+      } else {
+        // A range of registers: r[XX:YY].
+        if (NextToken.is(AsmToken::LBrac))
+          return true;
+      }
+    }
+  }
+
+  return getSpecialRegForName(RegName);
+}
+
+bool
+AMDGPUAsmParser::isRegister()
+{
+  return isRegister(getToken(), peekToken());
+}
+
 bool AMDGPUAsmParser::ParseAMDGPURegister(RegisterKind &RegKind, unsigned &Reg,
                                           unsigned &RegNum, unsigned &RegWidth,
                                           unsigned *DwordRegIndex) {
   if (DwordRegIndex) { *DwordRegIndex = 0; }
   const MCRegisterInfo *TRI = getContext().getRegisterInfo();
   if (getLexer().is(AsmToken::Identifier)) {
     StringRef RegName = Parser.getTok().getString();
     if ((Reg = getSpecialRegForName(RegName))) {

 std::unique_ptr<AMDGPUOperand> AMDGPUAsmParser::parseRegister() {
   const auto &Tok = Parser.getTok();
   SMLoc StartLoc = Tok.getLoc();
   SMLoc EndLoc = Tok.getEndLoc();
   RegisterKind RegKind;
   unsigned Reg, RegNum, RegWidth, DwordRegIndex;
 
   if (!ParseAMDGPURegister(RegKind, Reg, RegNum, RegWidth, &DwordRegIndex)) {
+    //FIXME: improve error messages (bug 41303).
+    Error(StartLoc, "not a valid operand.");
     return nullptr;
   }
   if (AMDGPU::IsaInfo::hasCodeObjectV3(&getSTI())) {
     if (!updateGprCountSymbols(RegKind, DwordRegIndex, RegWidth))
       return nullptr;
   } else
     KernelScope.usesRegister(RegKind, DwordRegIndex, RegWidth);
   return AMDGPUOperand::CreateReg(this, Reg, StartLoc, EndLoc, false);
AMDGPUAsmParser::parseImm(OperandVector &Operands, bool AbsMod) {
   }
   default:
     return MatchOperand_NoMatch;
   }
 }
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseReg(OperandVector &Operands) {
+  if (!isRegister())
+    return MatchOperand_NoMatch;
+
   if (auto R = parseRegister()) {
     assert(R->isReg());
     R->Reg.IsForcedVOP3 = isForcedVOP3();
     Operands.push_back(std::move(R));
     return MatchOperand_Success;
   }
-  return MatchOperand_NoMatch;
+  return MatchOperand_ParseFail;
 }
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseRegOrImm(OperandVector &Operands, bool AbsMod) {
-  auto res = parseImm(Operands, AbsMod);
-  if (res != MatchOperand_NoMatch) {
-    return res;
-  }
-
-  return parseReg(Operands);
+  auto res = parseReg(Operands);
+  return (res == MatchOperand_NoMatch)?
+         parseImm(Operands, AbsMod) :
+         res;
 }
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseRegOrImmWithFPInputMods(OperandVector &Operands,
                                               bool AllowImm) {
   bool Negate = false, Negate2 = false, Abs = false, Abs2 = false;
 
   if (getLexer().getKind()== AsmToken::Minus) {

 }
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseRegWithIntInputMods(OperandVector &Operands) {
   return parseRegOrImmWithIntInputMods(Operands, false);
 }
 
 OperandMatchResultTy AMDGPUAsmParser::parseVReg32OrOff(OperandVector &Operands) {
+  auto Loc = getLoc();
+  if (trySkipId("off")) {
+    Operands.push_back(AMDGPUOperand::CreateImm(this, 0, Loc,
+                                                AMDGPUOperand::ImmTyOff, false));
+    return MatchOperand_Success;
+  }
+
+  if (!isRegister())
+    return MatchOperand_NoMatch;
+
   std::unique_ptr<AMDGPUOperand> Reg = parseRegister();
   if (Reg) {
     Operands.push_back(std::move(Reg));
     return MatchOperand_Success;
   }
 
-  const AsmToken &Tok = Parser.getTok();
-  if (Tok.getString() == "off") {
-    Operands.push_back(AMDGPUOperand::CreateImm(this, 0, Tok.getLoc(),
-                                                AMDGPUOperand::ImmTyOff, false));
-    Parser.Lex();
-    return MatchOperand_Success;
-  }
+  return MatchOperand_ParseFail;
 
-  return MatchOperand_NoMatch;
 }
 
 unsigned AMDGPUAsmParser::checkTargetMatchPredicate(MCInst &Inst) {
   uint64_t TSFlags = MII.get(Inst.getOpcode()).TSFlags;
 
   if ((getForcedEncodingSize() == 32 && (TSFlags & SIInstrFlags::VOP3)) ||
       (getForcedEncodingSize() == 64 && !(TSFlags & SIInstrFlags::VOP3)) ||
       (isForcedDPP() && !(TSFlags & SIInstrFlags::DPP)) ||
AMDGPUAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
   // are appending default values to the Operands list.  This is only done
   // by custom parser, so we shouldn't continue on to the generic parsing.
   if (ResTy == MatchOperand_Success || ResTy == MatchOperand_ParseFail ||
       getLexer().is(AsmToken::EndOfStatement))
     return ResTy;
 
   ResTy = parseRegOrImm(Operands);
 
-  if (ResTy == MatchOperand_Success)
+  if (ResTy == MatchOperand_Success || ResTy == MatchOperand_ParseFail)
     return ResTy;
 
   const auto &Tok = Parser.getTok();
   SMLoc S = Tok.getLoc();
 
   const MCExpr *Expr = nullptr;
   if (!Parser.parseExpression(Expr)) {
     Operands.push_back(AMDGPUOperand::CreateExpr(this, Expr, S));
bool AMDGPUOperand::isSendMsg() const {
   return isImmTy(ImmTySendMsg);
 }
 
 //===----------------------------------------------------------------------===//
 // parser helpers
 //===----------------------------------------------------------------------===//
 
 bool
+AMDGPUAsmParser::isId(const AsmToken &Token, const StringRef Id) const {
+  return Token.is(AsmToken::Identifier) && Token.getString() == Id;
+}
+
+bool
+AMDGPUAsmParser::isId(const StringRef Id) const {
+  return isId(getToken(), Id);
+}
+
+bool
+AMDGPUAsmParser::isToken(const AsmToken::TokenKind Kind) const {
+  return getTokenKind() == Kind;
+}
+
+bool
 AMDGPUAsmParser::trySkipId(const StringRef Id) {
-  if (getLexer().getKind() == AsmToken::Identifier &&
-      Parser.getTok().getString() == Id) {
-    Parser.Lex();
+  if (isId(Id)) {
+    lex();
     return true;
   }
   return false;
 }
 
 bool
 AMDGPUAsmParser::trySkipToken(const AsmToken::TokenKind Kind) {
-  if (getLexer().getKind() == Kind) {
-    Parser.Lex();
+  if (isToken(Kind)) {
+    lex();
     return true;
   }
   return false;
 }
 
 bool
 AMDGPUAsmParser::skipToken(const AsmToken::TokenKind Kind,
                            const StringRef ErrMsg) {
   if (!trySkipToken(Kind)) {
-    Error(Parser.getTok().getLoc(), ErrMsg);
+    Error(getLoc(), ErrMsg);
     return false;
   }
   return true;
 }
 
 bool
 AMDGPUAsmParser::parseExpr(int64_t &Imm) {
   return !getParser().parseAbsoluteExpression(Imm);
 }
 
 bool
 AMDGPUAsmParser::parseString(StringRef &Val, const StringRef ErrMsg) {
-  SMLoc S = Parser.getTok().getLoc();
-  if (getLexer().getKind() == AsmToken::String) {
-    Val = Parser.getTok().getStringContents();
-    Parser.Lex();
+  if (isToken(AsmToken::String)) {
+    Val = getToken().getStringContents();
+    lex();
     return true;
   } else {
-    Error(S, ErrMsg);
+    Error(getLoc(), ErrMsg);
     return false;
   }
 }
 
+AsmToken
+AMDGPUAsmParser::getToken() const {
+  return Parser.getTok();
+}
+
+AsmToken
+AMDGPUAsmParser::peekToken() {
+  return getLexer().peekTok();
+}
+
+AsmToken::TokenKind
+AMDGPUAsmParser::getTokenKind() const {
+  return getLexer().getKind();
+}
+
+SMLoc
+AMDGPUAsmParser::getLoc() const {
+  return getToken().getLoc();
+}
+
+void
+AMDGPUAsmParser::lex() {
+  Parser.Lex();
+}
+
 //===----------------------------------------------------------------------===//
 // swizzle
 //===----------------------------------------------------------------------===//
 
 LLVM_READNONE
 static unsigned
 encodeBitmaskPerm(const unsigned AndMask,
                   const unsigned OrMask,

llvm/trunk/test/MC/AMDGPU/reg-syntax-err.s

+// RUN: not llvm-mc -arch=amdgcn -mcpu=tonga %s 2>&1 | FileCheck -check-prefix=NOVI %s
+
+s_mov_b32 s1, s 1
+// NOVI: error: invalid operand for instruction
+
+s_mov_b32 s1, s[0 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s[0:0 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, [s[0 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, [s[0:1] 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, [s0, 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s999 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s[1:2] 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s[0:2] 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, xnack_mask_lo 1
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s s0
+// NOVI: error: invalid operand for instruction
+
+s_mov_b32 s1, s[0 s0
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s[0:0 s0
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, [s[0 s0
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, [s[0:1] s0
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, [s0, s0
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s999 s0
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s[1:2] s0
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, s[0:2] vcc_lo
+// NOVI: error: failed parsing operand
+
+s_mov_b32 s1, xnack_mask_lo s1
+// NOVI: error: failed parsing operand
+
+exp mrt0 v1, v2, v3, v4000 off
+// NOVI: error: failed parsing operand

llvm/trunk/test/MC/AMDGPU/sop1-err.s

 s_mov_b64 s1, s[0:1]
 // GCN: error: invalid operand for instruction
 
 s_mov_b64 s[0:1], s1
 // GCN: error: invalid operand for instruction
 
 // FIXME: This shoudl probably say failed to parse.
 s_mov_b32 s
-// GCN: error: not a valid operand
+// GCN: error: invalid operand for instruction
 // Out of range register
 
 s_mov_b32 s102, 1
 // VI: error: not a valid operand
 // SI: s_mov_b32 s102, 1
 
 s_mov_b32 s103, 1
 // VI: error: not a valid operand
 // SI: s_mov_b32 s103, 1
 
 s_mov_b64 s[102:103], -1
 // VI: error: not a valid operand
 // SI: s_mov_b64 s[102:103], -1