[AMDGPU][MC] Corrected parsing of NAME:VALUE modifiers

[dp]

Summary

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

Note that this change also enables arbitrary constant expressions for VALUE (bug 40820).

[dp]

ping

[arsenm]

LGTM

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

3708 ↗

(On Diff #196221)

Missing space before ?

[dp]

Thanks!

[artem.tamazov]

Looks good.

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

private:
   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 trySkipId(const StringRef Id, const AsmToken::TokenKind Kind);
   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");
   void peekTokens(MutableArrayRef<AsmToken> Tokens);
   AsmToken::TokenKind getTokenKind() const;
   bool parseExpr(int64_t &Imm);
   StringRef getTokenStr() const;
   AsmToken peekToken();
bool AMDGPUAsmParser::ParseInstruction(ParseInstructionInfo &Info,
   return false;
 }
 
 //===----------------------------------------------------------------------===//
 // Utility functions
 //===----------------------------------------------------------------------===//
 
 OperandMatchResultTy
-AMDGPUAsmParser::parseIntWithPrefix(const char *Prefix, int64_t &Int) {
-  switch(getLexer().getKind()) {
-    default: return MatchOperand_NoMatch;
-    case AsmToken::Identifier: {
-      StringRef Name = Parser.getTok().getString();
-      if (!Name.equals(Prefix)) {
-        return MatchOperand_NoMatch;
-      }
-
-      Parser.Lex();
-      if (getLexer().isNot(AsmToken::Colon))
-        return MatchOperand_ParseFail;
-
-      Parser.Lex();
-
-      bool IsMinus = false;
-      if (getLexer().getKind() == AsmToken::Minus) {
-        Parser.Lex();
-        IsMinus = true;
-      }
-
-      if (getLexer().isNot(AsmToken::Integer))
-        return MatchOperand_ParseFail;
+AMDGPUAsmParser::parseIntWithPrefix(const char *Prefix, int64_t &IntVal) {
 
-      if (getParser().parseAbsoluteExpression(Int))
-        return MatchOperand_ParseFail;
+  if (!trySkipId(Prefix, AsmToken::Colon))
+    return MatchOperand_NoMatch;
 
-      if (IsMinus)
-        Int = -Int;
-      break;
-    }
-  }
-  return MatchOperand_Success;
+  return parseExpr(IntVal) ? MatchOperand_Success : MatchOperand_ParseFail;
 }
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseIntWithPrefix(const char *Prefix, OperandVector &Operands,
                                     AMDGPUOperand::ImmTy ImmTy,
                                     bool (*ConvertResult)(int64_t&)) {
-  SMLoc S = Parser.getTok().getLoc();
+  SMLoc S = getLoc();
   int64_t Value = 0;
 
   OperandMatchResultTy Res = parseIntWithPrefix(Prefix, Value);
   if (Res != MatchOperand_Success)
     return Res;
 
   if (ConvertResult && !ConvertResult(Value)) {
-    return MatchOperand_ParseFail;
+    Error(S, "invalid " + StringRef(Prefix) + " value.");
   }
 
   Operands.push_back(AMDGPUOperand::CreateImm(this, Value, S, ImmTy));
   return MatchOperand_Success;
 }
 
 OperandMatchResultTy AMDGPUAsmParser::parseOperandArrayWithPrefix(
   const char *Prefix,
AMDGPUAsmParser::trySkipId(const StringRef Id) {
   if (isId(Id)) {
     lex();
     return true;
   }
   return false;
 }
 
 bool
+AMDGPUAsmParser::trySkipId(const StringRef Id, const AsmToken::TokenKind Kind) {
+  if (isId(Id) && peekToken().is(Kind)) {
+    lex();
+    lex();
+    return true;
+  }
+  return false;
+}
+
+bool
 AMDGPUAsmParser::trySkipToken(const AsmToken::TokenKind Kind) {
   if (isToken(Kind)) {
     lex();
     return true;
   }
   return false;
 }
 

llvm/trunk/test/MC/AMDGPU/expressions.s

 
 v_and_b32 v0, -i1+102, v0
 // VI: v_and_b32_e32 v0, 0x65, v0      ; encoding: [0xff,0x00,0x00,0x26,0x65,0x00,0x00,0x00]
 
 v_add_u16 v0, (i1+100)*2, v0
 // VI: v_add_u16_e32 v0, 0xca, v0      ; encoding: [0xff,0x00,0x00,0x4c,0xca,0x00,0x00,0x00]
 
 //===----------------------------------------------------------------------===//
+// Constant expressions may be used with Name:Value modifiers.
+//===----------------------------------------------------------------------===//
+
+buffer_load_dword v1, off, s[4:7], s1 offset:-1+1
+// VI: buffer_load_dword v1, off, s[4:7], s1 ; encoding: [0x00,0x00,0x50,0xe0,0x00,0x01,0x01,0x01]
+
+buffer_load_dword v1, off, s[4:7], s1 offset:i1+4
+// VI: buffer_load_dword v1, off, s[4:7], s1 offset:5 ; encoding: [0x05,0x00,0x50,0xe0,0x00,0x01,0x01,0x01]
+
+buffer_load_dword v1, off, s[4:7], s1 offset:4+i1
+// VI: buffer_load_dword v1, off, s[4:7], s1 offset:5 ; encoding: [0x05,0x00,0x50,0xe0,0x00,0x01,0x01,0x01]
+
+buffer_load_dword v1, off, s[4:7], s1 offset:-i1+4
+// VI: buffer_load_dword v1, off, s[4:7], s1 offset:3 ; encoding: [0x03,0x00,0x50,0xe0,0x00,0x01,0x01,0x01]
+
+//===----------------------------------------------------------------------===//
 // Relocatable expressions can be used with 32-bit instructions.
 //===----------------------------------------------------------------------===//
 
 v_ceil_f32 v1, -u
 // VI: v_ceil_f32_e32 v1, -u           ; encoding: [0xff,0x3a,0x02,0x7e,A,A,A,A]
 // VI-NEXT: ; fixup A - offset: 4, value: -u, kind: FK_PCRel_4
 
 v_and_b32 v0, u+1, v0

llvm/trunk/test/MC/AMDGPU/vop3-errs.s

 
 v_cmp_le_f64_e64 vcc, v0, v1 mul:4
 // GCN: error: invalid operand for instruction
 
 v_cvt_u32_f32_e64 v0, v1 div:2
 // GCN: error: invalid operand for instruction
 
 //
+// mul
+//
+
+v_cvt_f64_i32 v[5:6], s1 mul:3
+// GCN: error: invalid mul value.
+
+//
 // v_interp*
 //
 
 v_interp_mov_f32_e64 v5, p10, attr0.x high
 // GCN: error: invalid operand for instruction
 
 v_interp_mov_f32_e64 v5, p10, attr0.x v0
 // GCN: error: invalid operand for instruction