[AMDGPU][MC] Corrected parsing of registers

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

Reviewers: artem.tamazov, arsenm

Differential Revision: https://reviews.llvm.org/D60621

llvm-svn: 358581

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

@@ -908,6 +908,8 @@ private:
   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,
@@ -1125,11 +1127,19 @@ private:
   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);
@@ -1748,6 +1758,51 @@ bool AMDGPUAsmParser::AddNextRegisterToList(unsigned &Reg, unsigned &RegWidth,
   }
 }
 
+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) {
@@ -1933,6 +1988,8 @@ std::unique_ptr<AMDGPUOperand> AMDGPUAsmParser::parseRegister() {
   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())) {
@@ -2012,23 +2069,24 @@ AMDGPUAsmParser::parseImm(OperandVector &Operands, bool AbsMod) {
 
 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
@@ -2195,21 +2253,24 @@ AMDGPUAsmParser::parseRegWithIntInputMods(OperandVector &Operands) {
 }
 
 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) {
@@ -3520,7 +3581,7 @@ AMDGPUAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) {
 
   ResTy = parseRegOrImm(Operands);
 
-  if (ResTy == MatchOperand_Success)
+  if (ResTy == MatchOperand_Success || ResTy == MatchOperand_ParseFail)
     return ResTy;
 
   const auto &Tok = Parser.getTok();
@@ -4491,11 +4552,25 @@ bool AMDGPUOperand::isSendMsg() const {
 // 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;
@@ -4503,8 +4578,8 @@ AMDGPUAsmParser::trySkipId(const StringRef Id) {
 
 bool
 AMDGPUAsmParser::trySkipToken(const AsmToken::TokenKind Kind) {
-  if (getLexer().getKind() == Kind) {
-    Parser.Lex();
+  if (isToken(Kind)) {
+    lex();
     return true;
   }
   return false;
@@ -4514,7 +4589,7 @@ 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;
@@ -4527,17 +4602,41 @@ AMDGPUAsmParser::parseExpr(int64_t &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
 //===----------------------------------------------------------------------===//

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

@@ -0,0 +1,64 @@
+// 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

test/MC/AMDGPU/sop1-err.s

@@ -28,7 +28,7 @@ s_mov_b64 s[0:1], s1
 
 // 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