[AMDGPU][MC] Improved diagnostics message for sym/expr operands

[dp]

Summary

Improved diagnostics message for operands which may be specified as either a 'macro' or an expression.

See bug 48295

[rampitec]

LGTM with a nit.

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

5746

"a hwreg macro".

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

private:
   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");
   bool parseId(StringRef &Val, const StringRef ErrMsg);
 
   void peekTokens(MutableArrayRef<AsmToken> Tokens);
   AsmToken::TokenKind getTokenKind() const;
-  bool parseExpr(int64_t &Imm);
+  bool parseExpr(int64_t &Imm, StringRef Expected = "");
   bool parseExpr(OperandVector &Operands);
   StringRef getTokenStr() const;
   AsmToken peekToken();
   AsmToken getToken() const;
   SMLoc getLoc() const;
   void lex();
 
 public:
AMDGPUAsmParser::parseHwregBody(OperandInfoTy &HwReg,
                                 OperandInfoTy &Width) {
   using namespace llvm::AMDGPU::Hwreg;
 
   // The register may be specified by name or using a numeric code
   HwReg.Loc = getLoc();
   if (isToken(AsmToken::Identifier) &&
       (HwReg.Id = getHwregId(getTokenStr())) >= 0) {
     HwReg.IsSymbolic = true;
-    lex(); // skip message name
-  } else if (!parseExpr(HwReg.Id)) {
+    lex(); // skip register name
+  } else if (!parseExpr(HwReg.Id, "a register name")) {
     return false;
   }
 
   if (trySkipToken(AsmToken::RParen))
     return true;
 
   // parse optional params
   if (!skipToken(AsmToken::Comma, "expected a comma or a closing parenthesis"))
if (trySkipId("hwreg", AsmToken::LParen)) {
     OperandInfoTy Offset(OFFSET_DEFAULT_);
     OperandInfoTy Width(WIDTH_DEFAULT_);
     if (parseHwregBody(HwReg, Offset, Width) &&
         validateHwreg(HwReg, Offset, Width)) {
       ImmVal = encodeHwreg(HwReg.Id, Offset.Id, Width.Id);
     } else {
       return MatchOperand_ParseFail;
     }
-  } else if (parseExpr(ImmVal)) {
+  } else if (parseExpr(ImmVal, "an hwreg macro")) {
     if (ImmVal < 0 || !isUInt<16>(ImmVal)) {
       Error(Loc, "invalid immediate: only 16-bit values are legal");
       return MatchOperand_ParseFail;
     }
   } else {
     return MatchOperand_ParseFail;
   }
 
AMDGPUAsmParser::parseSendMsgBody(OperandInfoTy &Msg,
                                   OperandInfoTy &Op,
                                   OperandInfoTy &Stream) {
   using namespace llvm::AMDGPU::SendMsg;
 
   Msg.Loc = getLoc();
   if (isToken(AsmToken::Identifier) && (Msg.Id = getMsgId(getTokenStr())) >= 0) {
     Msg.IsSymbolic = true;
     lex(); // skip message name
-  } else if (!parseExpr(Msg.Id)) {
+  } else if (!parseExpr(Msg.Id, "a message name")) {
     return false;
   }
 
   if (trySkipToken(AsmToken::Comma)) {
     Op.IsDefined = true;
     Op.Loc = getLoc();
     if (isToken(AsmToken::Identifier) &&
         (Op.Id = getMsgOpId(Msg.Id, getTokenStr())) >= 0) {
       lex(); // skip operation name
-    } else if (!parseExpr(Op.Id)) {
+    } else if (!parseExpr(Op.Id, "an operation name")) {
       return false;
     }
 
     if (trySkipToken(AsmToken::Comma)) {
       Stream.IsDefined = true;
       Stream.Loc = getLoc();
       if (!parseExpr(Stream.Id))
         return false;
if (trySkipId("sendmsg", AsmToken::LParen)) {
     OperandInfoTy Op(OP_NONE_);
     OperandInfoTy Stream(STREAM_ID_NONE_);
     if (parseSendMsgBody(Msg, Op, Stream) &&
         validateSendMsg(Msg, Op, Stream)) {
       ImmVal = encodeMsg(Msg.Id, Op.Id, Stream.Id);
     } else {
       return MatchOperand_ParseFail;
     }
-  } else if (parseExpr(ImmVal)) {
+  } else if (parseExpr(ImmVal, "a sendmsg macro")) {
     if (ImmVal < 0 || !isUInt<16>(ImmVal)) {
       Error(Loc, "invalid immediate: only 16-bit values are legal");
       return MatchOperand_ParseFail;
     }
   } else {
     return MatchOperand_ParseFail;
   }
 
AMDGPUAsmParser::skipToken(const AsmToken::TokenKind Kind,
   if (!trySkipToken(Kind)) {
     Error(getLoc(), ErrMsg);
     return false;
   }
   return true;
 }
 
 bool
-AMDGPUAsmParser::parseExpr(int64_t &Imm) {
-  return !getParser().parseAbsoluteExpression(Imm);
+AMDGPUAsmParser::parseExpr(int64_t &Imm, StringRef Expected) {
+  SMLoc S = getLoc();
+
+  const MCExpr *Expr;
+  if (Parser.parseExpression(Expr))
+    return false;
+
+  if (Expr->evaluateAsAbsolute(Imm))
+    return true;
+
+  if (Expected.empty()) {
+    Error(S, "expected absolute expression");
+  } else {
+    Error(S, Twine("expected ", Expected) +
+             Twine(" or an absolute expression"));
+  }
+  return false;
 }
 
 bool
 AMDGPUAsmParser::parseExpr(OperandVector &Operands) {
   SMLoc S = getLoc();
 
   const MCExpr *Expr;
   if (Parser.parseExpression(Expr))
AMDGPUAsmParser::parseSwizzleBitmaskPerm(int64_t &Imm) {
   return true;
 }
 
 bool
 AMDGPUAsmParser::parseSwizzleOffset(int64_t &Imm) {
 
   SMLoc OffsetLoc = Parser.getTok().getLoc();
 
-  if (!parseExpr(Imm)) {
+  if (!parseExpr(Imm, "a swizzle macro")) {
     return false;
   }
   if (!isUInt<16>(Imm)) {
     Error(OffsetLoc, "expected a 16-bit offset");
     return false;
   }
   return true;
 }

llvm/test/MC/AMDGPU/ds-err.s

 ds_swizzle_b32 v8, v2 offset
 
 // CHECK: error: unknown token in expression
 ds_swizzle_b32 v8, v2 offset:
 
 // CHECK: error: expected a colon
 ds_swizzle_b32 v8, v2 offset-
 
-// CHECK: error: expected absolute expression
+// CHECK: error: expected a swizzle macro or an absolute expression
 ds_swizzle_b32 v8, v2 offset:SWIZZLE(QUAD_PERM, 0, 1, 2, 3)
 
 // CHECK: error: expected a swizzle mode
 ds_swizzle_b32 v8, v2 offset:swizzle(quad_perm, 0, 1, 2, 3)
 
 // CHECK: error: expected a swizzle mode
 ds_swizzle_b32 v8, v2 offset:swizzle(XXX,1)
 

llvm/test/MC/AMDGPU/gfx10_err_pos.s

 ds_swizzle_b32 v8, v2 offset:swizzle(XXX,1)
 // CHECK: error: expected a swizzle mode
 // CHECK-NEXT:{{^}}ds_swizzle_b32 v8, v2 offset:swizzle(XXX,1)
 // CHECK-NEXT:{{^}}                                     ^
 
 //==============================================================================
 // expected absolute expression
 
-ds_swizzle_b32 v8, v2 offset:SWIZZLE(QUAD_PERM, 0, 1, 2, 3)
-// CHECK: error: expected absolute expression
-// CHECK-NEXT:{{^}}ds_swizzle_b32 v8, v2 offset:SWIZZLE(QUAD_PERM, 0, 1, 2, 3)
-// CHECK-NEXT:{{^}}                             ^
-
-s_sendmsg sendmsg(MSG_GS, GS_OP_CUTX, 0)
-// CHECK: error: expected absolute expression
-// CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GS, GS_OP_CUTX, 0)
-// CHECK-NEXT:{{^}}                          ^
-
-s_sendmsg sendmsg(MSG_GSX, GS_OP_CUT, 0)
-// CHECK: error: expected absolute expression
-// CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GSX, GS_OP_CUT, 0)
-// CHECK-NEXT:{{^}}                  ^
-
-s_setreg_b32  hwreg(HW_REG_WRONG), s2
-// CHECK: error: expected absolute expression
-// CHECK-NEXT:{{^}}s_setreg_b32  hwreg(HW_REG_WRONG), s2
-// CHECK-NEXT:{{^}}                    ^
-
 s_waitcnt vmcnt(x)
 // CHECK: error: expected absolute expression
 // CHECK-NEXT:{{^}}s_waitcnt vmcnt(x)
 // CHECK-NEXT:{{^}}                ^
 
 tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7], format:[BUF_DATA_FORMAT_32]
 // CHECK: error: expected absolute expression
 // CHECK-NEXT:{{^}}tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7], format:[BUF_DATA_FORMAT_32]
 // CHECK-NEXT:{{^}}                                                         ^
 
 tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7], s0 format: offset:52
 // CHECK: error: expected absolute expression
 // CHECK-NEXT:{{^}}tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7], s0 format: offset:52
 // CHECK-NEXT:{{^}}                                                             ^
 
 tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7], s0 format:BUF_NUM_FORMAT_UINT]
 // CHECK: error: expected absolute expression
 // CHECK-NEXT:{{^}}tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7], s0 format:BUF_NUM_FORMAT_UINT]
 // CHECK-NEXT:{{^}}                                                            ^
 
 //==============================================================================
+// expected a message name or an absolute expression
+
+s_sendmsg sendmsg(MSG_GSX, GS_OP_CUT, 0)
+// CHECK: error: expected a message name or an absolute expression
+// CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GSX, GS_OP_CUT, 0)
+// CHECK-NEXT:{{^}}                  ^
+
+//==============================================================================
+// expected a register name or an absolute expression
+
+s_setreg_b32  hwreg(HW_REG_WRONG), s2
+// CHECK: error: expected a register name or an absolute expression
+// CHECK-NEXT:{{^}}s_setreg_b32  hwreg(HW_REG_WRONG), s2
+// CHECK-NEXT:{{^}}                    ^
+
+//==============================================================================
+// expected a sendmsg macro or an absolute expression
+
+s_sendmsg undef
+// CHECK: error: expected a sendmsg macro or an absolute expression
+// CHECK-NEXT:{{^}}s_sendmsg undef
+// CHECK-NEXT:{{^}}          ^
+
+//==============================================================================
+// expected a swizzle macro or an absolute expression
+
+ds_swizzle_b32 v8, v2 offset:SWZ(QUAD_PERM, 0, 1, 2, 3)
+// CHECK: error: expected a swizzle macro or an absolute expression
+// CHECK-NEXT:{{^}}ds_swizzle_b32 v8, v2 offset:SWZ(QUAD_PERM, 0, 1, 2, 3)
+// CHECK-NEXT:{{^}}                             ^
+
+//==============================================================================
 // expected an 11-bit unsigned offset
 
 flat_atomic_cmpswap v0, v[1:2], v[3:4] offset:4095 glc
 // CHECK: error: expected an 11-bit unsigned offset
 // CHECK-NEXT:{{^}}flat_atomic_cmpswap v0, v[1:2], v[3:4] offset:4095 glc
 // CHECK-NEXT:{{^}}                                       ^
 
 //==============================================================================

 // CHECK-NEXT:{{^}}                ^
 
 v_mov_b32_sdwa v1, sext(u)
 // CHECK: error: expected an absolute expression
 // CHECK-NEXT:{{^}}v_mov_b32_sdwa v1, sext(u)
 // CHECK-NEXT:{{^}}                        ^
 
 //==============================================================================
+// expected an hwreg macro or an absolute expression
+
+s_setreg_b32 undef, s2
+// CHECK: error: expected an hwreg macro or an absolute expression
+// CHECK-NEXT:{{^}}s_setreg_b32 undef, s2
+// CHECK-NEXT:{{^}}             ^
+
+//==============================================================================
+// expected an operation name or an absolute expression
+
+s_sendmsg sendmsg(MSG_GS, GS_OP_CUTX, 0)
+// CHECK: error: expected an operation name or an absolute expression
+// CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GS, GS_OP_CUTX, 0)
+// CHECK-NEXT:{{^}}                          ^
+
+//==============================================================================
 // failed parsing operand.
 
 image_load v[0:1], v0, s[0:7] dmask:0x9 dim:1 D
 // CHECK: error: failed parsing operand.
 // CHECK-NEXT:{{^}}image_load v[0:1], v0, s[0:7] dmask:0x9 dim:1 D
 // CHECK-NEXT:{{^}}                                              ^
 
 v_ceil_f16 v0, abs(neg(1))

llvm/test/MC/AMDGPU/sopk-err.s

 // RUN: not llvm-mc -arch=amdgcn -show-encoding %s | FileCheck -check-prefixes=SICI %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tahiti -show-encoding %s | FileCheck -check-prefixes=SI,SICI %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tonga -show-encoding %s | FileCheck -check-prefixes=VI %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx900 -show-encoding %s | FileCheck -check-prefix=GFX9 %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1010 -show-encoding %s | FileCheck -check-prefix=GFX10 %s
 
 // RUN: not llvm-mc -arch=amdgcn %s 2>&1 | FileCheck -check-prefixes=GCN,SICIVI-ERR --implicit-check-not=error: %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tahiti %s 2>&1 | FileCheck -check-prefixes=GCN,SICIVI-ERR --implicit-check-not=error: %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tonga %s 2>&1 | FileCheck -check-prefixes=GCN,SICIVI-ERR --implicit-check-not=error: %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx900 %s 2>&1 | FileCheck -check-prefixes=GCN,GFX9-ERR --implicit-check-not=error: %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1010 %s 2>&1 | FileCheck -check-prefixes=GCN,GFX10-ERR --implicit-check-not=error: %s
 
 s_setreg_b32  0x1f803, s2
 // GCN: error: invalid immediate: only 16-bit values are legal
 
 s_setreg_b32  typo(0x40), s2
-// GCN: error: expected absolute expression
+// GCN: error: expected an hwreg macro or an absolute expression
 
 s_setreg_b32  hwreg(0x40), s2
 // GCN: error: invalid code of hardware register: only 6-bit values are legal
 
 s_setreg_b32  hwreg(HW_REG_WRONG), s2
-// GCN: error: expected absolute expression
+// GCN: error: expected a register name or an absolute expression
 
 s_setreg_b32  hwreg(1 2,3), s2
 // GCN: error: expected a comma or a closing parenthesis
 
 s_setreg_b32  hwreg(1,2 3), s2
 // GCN: error: expected a comma
 
 s_setreg_b32  hwreg(1,2,3, s2

llvm/test/MC/AMDGPU/sopp-err.s

 // RUN: not llvm-mc -arch=amdgcn %s 2>&1 | FileCheck --check-prefix=GCN --check-prefix=SICI --implicit-check-not=error: %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tahiti %s 2>&1 | FileCheck --check-prefix=GCN --check-prefix=SICI --implicit-check-not=error: %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=fiji %s 2>&1 | FileCheck --check-prefix=GCN --check-prefix=VI --check-prefix=SICIVI --implicit-check-not=error: %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx1010 %s 2>&1 | FileCheck --check-prefix=GCN --check-prefix=GFX10 --implicit-check-not=error: %s
 
 //===----------------------------------------------------------------------===//
 // sendmsg
 //===----------------------------------------------------------------------===//
 
 s_sendmsg sendmsg(MSG_INTERRUPTX)
-// GCN: error: expected absolute expression
+// GCN: error: expected a message name or an absolute expression
 
 s_sendmsg sendmsg(1 -)
 // GCN: error: unknown token in expression
 
 s_sendmsg sendmsg(MSG_INTERRUPT, 0)
 // GCN: error: message does not support operations
 
 s_sendmsg sendmsg(MSG_INTERRUPT, 0, 0)
 // GCN: error: message does not support operations
 
 s_sendmsg sendmsg(MSG_GS)
 // GCN: error: missing message operation
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_NOP)
 // GCN: error: invalid operation id
 
 s_sendmsg sendmsg(MSG_GS, SYSMSG_OP_ECC_ERR_INTERRUPT)
-// GCN: error: expected absolute expression
+// GCN: error: expected an operation name or an absolute expression
 
 s_sendmsg sendmsg(MSG_GS, 0)
 // GCN: error: invalid operation id
 
 s_sendmsg sendmsg(MSG_GS, -1)
 // GCN: error: invalid operation id
 
 s_sendmsg sendmsg(MSG_GS, 4)
 // GCN: error: invalid operation id
 
 s_sendmsg sendmsg(MSG_GS, 8)
 // GCN: error: invalid operation id
 
 s_sendmsg sendmsg(15, -1)
 // GCN: error: invalid operation id
 
 s_sendmsg sendmsg(15, 8)
 // GCN: error: invalid operation id
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUT, 0, 0)
 // GCN: error: expected a closing parenthesis
 
 s_sendmsg sendmsg(MSG_GSX, GS_OP_CUT, 0)
-// GCN: error: expected absolute expression
+// GCN: error: expected a message name or an absolute expression
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUTX, 0)
-// GCN: error: expected absolute expression
+// GCN: error: expected an operation name or an absolute expression
 
 s_sendmsg sendmsg(MSG_GS, 1 -)
 // GCN: error: unknown token in expression
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUT, 4)
 // GCN: error: invalid message stream id
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUT, 1 -)