[AMDGPU][MC] Corrected error position for hwreg() and sendmsg()

[dp]

Summary

Partially fixes bug 47518

[dp]

Corrected indentation

[rampitec]

LGTM

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

public:
   void cvtExp(MCInst &Inst, const OperandVector &Operands);
 
   bool parseCnt(int64_t &IntVal);
   OperandMatchResultTy parseSWaitCntOps(OperandVector &Operands);
   OperandMatchResultTy parseHwreg(OperandVector &Operands);
 
 private:
   struct OperandInfoTy {
+    SMLoc Loc;
     int64_t Id;
     bool IsSymbolic = false;
     bool IsDefined = false;
 
     OperandInfoTy(int64_t Id_) : Id(Id_) {}
   };
 
   bool parseSendMsgBody(OperandInfoTy &Msg, OperandInfoTy &Op, OperandInfoTy &Stream);
   bool validateSendMsg(const OperandInfoTy &Msg,
                        const OperandInfoTy &Op,
-                       const OperandInfoTy &Stream,
-                       const SMLoc Loc);
+                       const OperandInfoTy &Stream);
 
-  bool parseHwregBody(OperandInfoTy &HwReg, int64_t &Offset, int64_t &Width);
+  bool parseHwregBody(OperandInfoTy &HwReg,
+                      OperandInfoTy &Offset,
+                      OperandInfoTy &Width);
   bool validateHwreg(const OperandInfoTy &HwReg,
-                     const int64_t Offset,
-                     const int64_t Width,
-                     const SMLoc Loc);
+                     const OperandInfoTy &Offset,
+                     const OperandInfoTy &Width);
 
   OperandMatchResultTy parseExpTgtImpl(StringRef Str, uint8_t &Val);
   SMLoc getFlatOffsetLoc(const OperandVector &Operands) const;
   SMLoc getSMEMOffsetLoc(const OperandVector &Operands) const;
 
   bool validateInstruction(const MCInst &Inst, const SMLoc &IDLoc, const OperandVector &Operands);
   bool validateFlatOffset(const MCInst &Inst, const OperandVector &Operands);
   bool validateSMEMOffset(const MCInst &Inst, const OperandVector &Operands);

 }
 
 //===----------------------------------------------------------------------===//
 // hwreg
 //===----------------------------------------------------------------------===//
 
 bool
 AMDGPUAsmParser::parseHwregBody(OperandInfoTy &HwReg,
-                                int64_t &Offset,
-                                int64_t &Width) {
+                                OperandInfoTy &Offset,
+                                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)) {
     return false;
   }
 
   if (trySkipToken(AsmToken::RParen))
     return true;
 
   // parse optional params
-  return
-    skipToken(AsmToken::Comma, "expected a comma or a closing parenthesis") &&
-    parseExpr(Offset) &&
-    skipToken(AsmToken::Comma, "expected a comma") &&
-    parseExpr(Width) &&
+  if (!skipToken(AsmToken::Comma, "expected a comma or a closing parenthesis"))
+    return false;
+
+  Offset.Loc = getLoc();
+  if (!parseExpr(Offset.Id))
+    return false;
+
+  if (!skipToken(AsmToken::Comma, "expected a comma"))
+    return false;
+
+  Width.Loc = getLoc();
+  return parseExpr(Width.Id) &&
          skipToken(AsmToken::RParen, "expected a closing parenthesis");
 }
 
 bool
 AMDGPUAsmParser::validateHwreg(const OperandInfoTy &HwReg,
-                               const int64_t Offset,
-                               const int64_t Width,
-                               const SMLoc Loc) {
+                               const OperandInfoTy &Offset,
+                               const OperandInfoTy &Width) {
 
   using namespace llvm::AMDGPU::Hwreg;
 
   if (HwReg.IsSymbolic && !isValidHwreg(HwReg.Id, getSTI())) {
-    Error(Loc, "specified hardware register is not supported on this GPU");
+    Error(HwReg.Loc,
+          "specified hardware register is not supported on this GPU");
     return false;
-  } else if (!isValidHwreg(HwReg.Id)) {
-    Error(Loc, "invalid code of hardware register: only 6-bit values are legal");
+  }
+  if (!isValidHwreg(HwReg.Id)) {
+    Error(HwReg.Loc,
+          "invalid code of hardware register: only 6-bit values are legal");
     return false;
-  } else if (!isValidHwregOffset(Offset)) {
-    Error(Loc, "invalid bit offset: only 5-bit values are legal");
+  }
+  if (!isValidHwregOffset(Offset.Id)) {
+    Error(Offset.Loc, "invalid bit offset: only 5-bit values are legal");
     return false;
-  } else if (!isValidHwregWidth(Width)) {
-    Error(Loc, "invalid bitfield width: only values from 1 to 32 are legal");
+  }
+  if (!isValidHwregWidth(Width.Id)) {
+    Error(Width.Loc,
+          "invalid bitfield width: only values from 1 to 32 are legal");
     return false;
   }
   return true;
 }
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseHwreg(OperandVector &Operands) {
   using namespace llvm::AMDGPU::Hwreg;
 
   int64_t ImmVal = 0;
   SMLoc Loc = getLoc();
 
   if (trySkipId("hwreg", AsmToken::LParen)) {
     OperandInfoTy HwReg(ID_UNKNOWN_);
-    int64_t Offset = OFFSET_DEFAULT_;
-    int64_t Width = WIDTH_DEFAULT_;
+    OperandInfoTy Offset(OFFSET_DEFAULT_);
+    OperandInfoTy Width(WIDTH_DEFAULT_);
     if (parseHwregBody(HwReg, Offset, Width) &&
-        validateHwreg(HwReg, Offset, Width, Loc)) {
-      ImmVal = encodeHwreg(HwReg.Id, Offset, Width);
+        validateHwreg(HwReg, Offset, Width)) {
+      ImmVal = encodeHwreg(HwReg.Id, Offset.Id, Width.Id);
     } else {
       return MatchOperand_ParseFail;
     }
   } else if (parseExpr(ImmVal)) {
     if (ImmVal < 0 || !isUInt<16>(ImmVal)) {
       Error(Loc, "invalid immediate: only 16-bit values are legal");
       return MatchOperand_ParseFail;
     }

 //===----------------------------------------------------------------------===//
 
 bool
 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)) {
     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)) {
       return false;
     }
 
     if (trySkipToken(AsmToken::Comma)) {
       Stream.IsDefined = true;
+      Stream.Loc = getLoc();
       if (!parseExpr(Stream.Id))
         return false;
     }
   }
 
   return skipToken(AsmToken::RParen, "expected a closing parenthesis");
 }
 
 bool
 AMDGPUAsmParser::validateSendMsg(const OperandInfoTy &Msg,
                                  const OperandInfoTy &Op,
-                                 const OperandInfoTy &Stream,
-                                 const SMLoc S) {
+                                 const OperandInfoTy &Stream) {
   using namespace llvm::AMDGPU::SendMsg;
 
   // Validation strictness depends on whether message is specified
   // in a symbolc or in a numeric form. In the latter case
   // only encoding possibility is checked.
   bool Strict = Msg.IsSymbolic;
 
   if (!isValidMsgId(Msg.Id, getSTI(), Strict)) {
-    Error(S, "invalid message id");
+    Error(Msg.Loc, "invalid message id");
     return false;
-  } else if (Strict && (msgRequiresOp(Msg.Id) != Op.IsDefined)) {
-    Error(S, Op.IsDefined ?
-             "message does not support operations" :
-             "missing message operation");
+  }
+  if (Strict && (msgRequiresOp(Msg.Id) != Op.IsDefined)) {
+    if (Op.IsDefined) {
+      Error(Op.Loc, "message does not support operations");
+    } else {
+      Error(Msg.Loc, "missing message operation");
+    }
     return false;
-  } else if (!isValidMsgOp(Msg.Id, Op.Id, Strict)) {
-    Error(S, "invalid operation id");
+  }
+  if (!isValidMsgOp(Msg.Id, Op.Id, Strict)) {
+    Error(Op.Loc, "invalid operation id");
     return false;
-  } else if (Strict && !msgSupportsStream(Msg.Id, Op.Id) && Stream.IsDefined) {
-    Error(S, "message operation does not support streams");
+  }
+  if (Strict && !msgSupportsStream(Msg.Id, Op.Id) && Stream.IsDefined) {
+    Error(Stream.Loc, "message operation does not support streams");
     return false;
-  } else if (!isValidMsgStream(Msg.Id, Op.Id, Stream.Id, Strict)) {
-    Error(S, "invalid message stream id");
+  }
+  if (!isValidMsgStream(Msg.Id, Op.Id, Stream.Id, Strict)) {
+    Error(Stream.Loc, "invalid message stream id");
     return false;
   }
   return true;
 }
 
 OperandMatchResultTy
 AMDGPUAsmParser::parseSendMsgOp(OperandVector &Operands) {
   using namespace llvm::AMDGPU::SendMsg;
 
   int64_t ImmVal = 0;
   SMLoc Loc = getLoc();
 
   if (trySkipId("sendmsg", AsmToken::LParen)) {
     OperandInfoTy Msg(ID_UNKNOWN_);
     OperandInfoTy Op(OP_NONE_);
     OperandInfoTy Stream(STREAM_ID_NONE_);
     if (parseSendMsgBody(Msg, Op, Stream) &&
-        validateSendMsg(Msg, Op, Stream, Loc)) {
+        validateSendMsg(Msg, Op, Stream)) {
       ImmVal = encodeMsg(Msg.Id, Op.Id, Stream.Id);
     } else {
       return MatchOperand_ParseFail;
     }
   } else if (parseExpr(ImmVal)) {
     if (ImmVal < 0 || !isUInt<16>(ImmVal)) {
       Error(Loc, "invalid immediate: only 16-bit values are legal");
       return MatchOperand_ParseFail;

llvm/test/MC/AMDGPU/gfx10_err_pos.s

 // CHECK-NEXT:{{^}}^
 
 //==============================================================================
 // invalid bit offset: only 5-bit values are legal
 
 s_getreg_b32  s2, hwreg(3,32,32)
 // CHECK: error: invalid bit offset: only 5-bit values are legal
 // CHECK-NEXT:{{^}}s_getreg_b32  s2, hwreg(3,32,32)
-// CHECK-NEXT:{{^}}                  ^
+// CHECK-NEXT:{{^}}                          ^
 
 //==============================================================================
 // invalid bitfield width: only values from 1 to 32 are legal
 
 s_setreg_b32  hwreg(3,0,33), s2
 // CHECK: error: invalid bitfield width: only values from 1 to 32 are legal
 // CHECK-NEXT:{{^}}s_setreg_b32  hwreg(3,0,33), s2
-// CHECK-NEXT:{{^}}              ^
+// CHECK-NEXT:{{^}}                        ^
 
 //==============================================================================
 // invalid code of hardware register: only 6-bit values are legal
 
 s_setreg_b32  hwreg(0x40), s2
 // CHECK: error: invalid code of hardware register: only 6-bit values are legal
 // CHECK-NEXT:{{^}}s_setreg_b32  hwreg(0x40), s2
-// CHECK-NEXT:{{^}}              ^
+// CHECK-NEXT:{{^}}                    ^
 
 //==============================================================================
 // invalid counter name x
 
 s_waitcnt vmcnt(0) & expcnt(0) x(0)
 // CHECK: error: invalid counter name x
 // CHECK-NEXT:{{^}}s_waitcnt vmcnt(0) & expcnt(0) x(0)
 // CHECK-NEXT:{{^}}                               ^

 // CHECK-NEXT:{{^}}                                                   ^
 
 //==============================================================================
 // invalid message id
 
 s_sendmsg sendmsg(-1)
 // CHECK: error: invalid message id
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(-1)
-// CHECK-NEXT:{{^}}          ^
+// CHECK-NEXT:{{^}}                  ^
 
 //==============================================================================
 // invalid message stream id
 
 s_sendmsg sendmsg(2, 2, 4)
 // CHECK: error: invalid message stream id
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(2, 2, 4)
-// CHECK-NEXT:{{^}}          ^
+// CHECK-NEXT:{{^}}                        ^
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUT, 4)
 // CHECK: error: invalid message stream id
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GS, GS_OP_CUT, 4)
-// CHECK-NEXT:{{^}}          ^
+// CHECK-NEXT:{{^}}                                     ^
 
 //==============================================================================
 // invalid mul value.
 
 v_cvt_f64_i32 v[5:6], s1 mul:3
 // CHECK: error: invalid mul value.
 // CHECK-NEXT:{{^}}v_cvt_f64_i32 v[5:6], s1 mul:3
 // CHECK-NEXT:{{^}}                         ^

 // CHECK-NEXT:{{^}}             ^
 
 //==============================================================================
 // invalid operation id
 
 s_sendmsg sendmsg(15, -1)
 // CHECK: error: invalid operation id
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(15, -1)
-// CHECK-NEXT:{{^}}          ^
+// CHECK-NEXT:{{^}}                      ^
 
 //==============================================================================
 // invalid or unsupported register size
 
 s_mov_b64 s[0:17], -1
 // CHECK: error: invalid or unsupported register size
 // CHECK-NEXT:{{^}}s_mov_b64 s[0:17], -1
 // CHECK-NEXT:{{^}}          ^

 // CHECK-NEXT:{{^}}                                                 ^
 
 //==============================================================================
 // message does not support operations
 
 s_sendmsg sendmsg(MSG_GS_ALLOC_REQ, 0)
 // CHECK: error: message does not support operations
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GS_ALLOC_REQ, 0)
-// CHECK-NEXT:{{^}}          ^
+// CHECK-NEXT:{{^}}                                    ^
 
 //==============================================================================
 // message operation does not support streams
 
 s_sendmsg sendmsg(MSG_GS_DONE, GS_OP_NOP, 0)
 // CHECK: error: message operation does not support streams
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_GS_DONE, GS_OP_NOP, 0)
-// CHECK-NEXT:{{^}}          ^
+// CHECK-NEXT:{{^}}                                          ^
 
 //==============================================================================
 // missing message operation
 
 s_sendmsg sendmsg(MSG_SYSMSG)
 // CHECK: error: missing message operation
 // CHECK-NEXT:{{^}}s_sendmsg sendmsg(MSG_SYSMSG)
-// CHECK-NEXT:{{^}}          ^
+// CHECK-NEXT:{{^}}                  ^
 
 //==============================================================================
 // missing register index
 
 s_mov_b64 s[10:11], [s
 // CHECK: error: missing register index
 // CHECK-NEXT:{{^}}s_mov_b64 s[10:11], [s
 // CHECK-NEXT:{{^}}                      ^

 // CHECK-NEXT:{{^}}^
 
 //==============================================================================
 // specified hardware register is not supported on this GPU
 
 s_getreg_b32 s2, hwreg(HW_REG_SHADER_CYCLES)
 // CHECK: error: specified hardware register is not supported on this GPU
 // CHECK-NEXT:{{^}}s_getreg_b32 s2, hwreg(HW_REG_SHADER_CYCLES)
-// CHECK-NEXT:{{^}}                 ^
+// CHECK-NEXT:{{^}}                       ^
 
 //==============================================================================
 // too few operands for instruction
 
 tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7]
 // CHECK: error: too few operands for instruction
 // CHECK-NEXT:{{^}}tbuffer_store_format_xyzw v[1:4], off, ttmp[4:7]
 // CHECK-NEXT:{{^}}^