[AMDGPU][MC][NFC] Refactored sendmsg(...) handling

[dp]

Summary

Switched to using new custom operands design.
See https://reviews.llvm.org/D121696.

[foad]

llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.cpp

24

I think it would be neater to define and use a new isGFX8GFX9 instead of having a lambda inside this table. (And the same for the hwreg table that you pushed already.)

[dp]

Corrected as suggested by Jay.

[rampitec]

LGTM

[foad]

LGTM (with or without addressing my nitpicks about naming).

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

1514

Maybe it would be simpler to implement this as "isNotGFX10Plus" instead?

1528

In AMDGPU.td there is a predicate called "isGFX10Before1030". Maybe we should use the same name in both places? I don't have a preference for which name is better.

[dp]

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

1514

Thanks, corrected.

1528

I'd have preferred isGFX10RDNA1 but I do not want to correct code that is not related to this change. So I'll use isGFX10Before1030 as you suggested.

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

 
 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) {
+  if (isToken(AsmToken::Identifier) &&
+      (Msg.Id = getMsgId(getTokenStr(), getSTI())) != OPR_ID_UNKNOWN) {
     Msg.IsSymbolic = true;
     lex(); // skip message name
   } else if (!parseExpr(Msg.Id, "a message name")) {
     return false;
   }
 
   if (trySkipToken(AsmToken::Comma)) {
     Op.IsDefined = true;
AMDGPUAsmParser::validateSendMsg(const OperandInfoTy &Msg,
                                  const OperandInfoTy &Stream) {
   using namespace llvm::AMDGPU::SendMsg;
 
   // Validation strictness depends on whether message is specified
   // in a symbolic or in a numeric form. In the latter case
   // only encoding possibility is checked.
   bool Strict = Msg.IsSymbolic;
 
-  if (!isValidMsgId(Msg.Id, getSTI(), Strict)) {
+  if (Strict) {
+    if (Msg.Id == OPR_ID_UNSUPPORTED) {
+      Error(Msg.Loc, "specified message id is not supported on this GPU");
+      return false;
+    }
+  } else {
+    if (!isValidMsgId(Msg.Id)) {
       Error(Msg.Loc, "invalid message id");
       return false;
     }
+  }
   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;
   }

 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 Msg(OPR_ID_UNKNOWN);
     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;
     }

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp

void AMDGPUInstPrinter::printSendMsg(const MCInst *MI, unsigned OpNo,
 
   const unsigned Imm16 = MI->getOperand(OpNo).getImm();
 
   uint16_t MsgId;
   uint16_t OpId;
   uint16_t StreamId;
   decodeMsg(Imm16, MsgId, OpId, StreamId);
 
-  if (isValidMsgId(MsgId, STI) &&
-      isValidMsgOp(MsgId, OpId, STI) &&
+  StringRef MsgName = getMsgName(MsgId, STI);
+
+  if (!MsgName.empty() && isValidMsgOp(MsgId, OpId, STI) &&
       isValidMsgStream(MsgId, OpId, StreamId, STI)) {
-    O << "sendmsg(" << getMsgName(MsgId);
+    O << "sendmsg(" << MsgName;
     if (msgRequiresOp(MsgId)) {
       O << ", " << getMsgOpName(MsgId, OpId);
       if (msgSupportsStream(MsgId, OpId)) {
         O << ", " << StreamId;
       }
     }
     O << ')';
   } else if (encodeMsg(MsgId, OpId, StreamId) == Imm16) {

llvm/lib/Target/AMDGPU/SIDefines.h

enum CPol {
   ALL = GLC | SLC | DLC | SCC
 };
 
 } // namespace CPol
 
 namespace SendMsg { // Encoding of SIMM16 used in s_sendmsg* insns.
 
 enum Id { // Message ID, width(4) [3:0].
-  ID_UNKNOWN_ = -1,
   ID_INTERRUPT = 1,
   ID_GS = 2,
   ID_GS_DONE = 3,
   ID_SAVEWAVE = 4,           // added in GFX8
   ID_STALL_WAVE_GEN = 5,     // added in GFX9
   ID_HALT_WAVES = 6,         // added in GFX9
   ID_ORDERED_PS_DONE = 7,    // added in GFX9
   ID_EARLY_PRIM_DEALLOC = 8, // added in GFX9, removed in GFX10
   ID_GS_ALLOC_REQ = 9,       // added in GFX9
   ID_GET_DOORBELL = 10,      // added in GFX9
   ID_GET_DDID = 11,          // added in GFX10
   ID_SYSMSG = 15,
-  ID_GAPS_LAST_, // Indicate that sequence has gaps.
-  ID_GAPS_FIRST_ = ID_INTERRUPT,
+
   ID_SHIFT_ = 0,
   ID_WIDTH_ = 4,
   ID_MASK_ = (((1 << ID_WIDTH_) - 1) << ID_SHIFT_)
 };
 
 enum Op { // Both GS and SYS operation IDs.
   OP_UNKNOWN_ = -1,
   OP_SHIFT_ = 4,

llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.h

 template <class T> struct CustomOperand {
   StringLiteral Name;
   int Encoding = 0;
   bool (*Cond)(T Context) = nullptr;
 };
 
 namespace SendMsg { // Symbolic names for the sendmsg(...) syntax.
 
-extern const char *const IdSymbolic[ID_GAPS_LAST_];
+extern const CustomOperand<const MCSubtargetInfo &> Msg[];
+extern const int MSG_SIZE;
+
 extern const char *const OpSysSymbolic[OP_SYS_LAST_];
 extern const char *const OpGsSymbolic[OP_GS_LAST_];
 
 } // namespace SendMsg
 
 namespace Hwreg { // Symbolic names for the hwreg(...) syntax.
 
 extern const CustomOperand<const MCSubtargetInfo &> Opr[];

llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.cpp

 //===-- AMDGPUAsmUtils.cpp - AsmParser/InstPrinter common -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "AMDGPUAsmUtils.h"
 #include "AMDGPUBaseInfo.h"
 #include "SIDefines.h"
 
 namespace llvm {
 namespace AMDGPU {
 namespace SendMsg {
 
-// This must be in sync with llvm::AMDGPU::SendMsg::Id enum members, see SIDefines.h.
-const char *const IdSymbolic[ID_GAPS_LAST_] = {
-  nullptr,
-  "MSG_INTERRUPT",
-  "MSG_GS",
-  "MSG_GS_DONE",
-  "MSG_SAVEWAVE",
-  "MSG_STALL_WAVE_GEN",
-  "MSG_HALT_WAVES",
-  "MSG_ORDERED_PS_DONE",
-  "MSG_EARLY_PRIM_DEALLOC",
-  "MSG_GS_ALLOC_REQ",
-  "MSG_GET_DOORBELL",
-  "MSG_GET_DDID",
-  nullptr,
-  nullptr,
-  nullptr,
-  "MSG_SYSMSG"
+// Disable lint checking for this block since it makes the table unreadable.
+// NOLINTBEGIN
+const CustomOperand<const MCSubtargetInfo &> Msg[] = {
+  {{""}},
+  {{"MSG_INTERRUPT"},           ID_INTERRUPT},
+  {{"MSG_GS"},                  ID_GS},
+  {{"MSG_GS_DONE"},             ID_GS_DONE},
+  {{"MSG_SAVEWAVE"},            ID_SAVEWAVE,
+                                     [](const MCSubtargetInfo &STI) {
+                                       return isVI(STI) || isGFX9Plus(STI);
+                                     }},
+  {{"MSG_STALL_WAVE_GEN"},      ID_STALL_WAVE_GEN,      isGFX9Plus},
+  {{"MSG_HALT_WAVES"},          ID_HALT_WAVES,          isGFX9Plus},
+  {{"MSG_ORDERED_PS_DONE"},     ID_ORDERED_PS_DONE,     isGFX9Plus},
+  {{"MSG_EARLY_PRIM_DEALLOC"},  ID_EARLY_PRIM_DEALLOC,  isGFX9},
+  {{"MSG_GS_ALLOC_REQ"},        ID_GS_ALLOC_REQ,        isGFX9Plus},
+  {{"MSG_GET_DOORBELL"},        ID_GET_DOORBELL,        isGFX9Plus},
+  {{"MSG_GET_DDID"},            ID_GET_DDID,            isGFX10Plus},
+  {{""}},
+  {{""}},
+  {{""}},
+  {{"MSG_SYSMSG"},              ID_SYSMSG},
 };
+// NOLINTEND
+
+const int MSG_SIZE = static_cast<int>(
+    sizeof(Msg) / sizeof(CustomOperand<const MCSubtargetInfo &>));
 
 // These two must be in sync with llvm::AMDGPU::SendMsg::Op enum members, see SIDefines.h.
 const char *const OpSysSymbolic[OP_SYS_LAST_] = {
   nullptr,
   "SYSMSG_OP_ECC_ERR_INTERRUPT",
   "SYSMSG_OP_REG_RD",
   "SYSMSG_OP_HOST_TRAP_ACK",
   "SYSMSG_OP_TTRACE_PC"

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h

 
 unsigned getDefaultFormatEncoding(const MCSubtargetInfo &STI);
 
 } // namespace MTBUFFormat
 
 namespace SendMsg {
 
 LLVM_READONLY
-int64_t getMsgId(const StringRef Name);
+int64_t getMsgId(const StringRef Name, const MCSubtargetInfo &STI);
 
 LLVM_READONLY
 int64_t getMsgOpId(int64_t MsgId, const StringRef Name);
 
 LLVM_READNONE
-StringRef getMsgName(int64_t MsgId);
+StringRef getMsgName(int64_t MsgId, const MCSubtargetInfo &STI);
 
 LLVM_READNONE
 StringRef getMsgOpName(int64_t MsgId, int64_t OpId);
 
 LLVM_READNONE
-bool isValidMsgId(int64_t MsgId, const MCSubtargetInfo &STI, bool Strict = true);
+bool isValidMsgId(int64_t MsgId);
 
 LLVM_READNONE
 bool isValidMsgOp(int64_t MsgId, int64_t OpId, const MCSubtargetInfo &STI,
                   bool Strict = true);
 
 LLVM_READNONE
 bool isValidMsgStream(int64_t MsgId, int64_t OpId, int64_t StreamId,
                       const MCSubtargetInfo &STI, bool Strict = true);

llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp

 } // namespace MTBUFFormat
 
 //===----------------------------------------------------------------------===//
 // SendMsg
 //===----------------------------------------------------------------------===//
 
 namespace SendMsg {
 
-int64_t getMsgId(const StringRef Name) {
-  for (int i = ID_GAPS_FIRST_; i < ID_GAPS_LAST_; ++i) {
-    if (IdSymbolic[i] && Name == IdSymbolic[i])
-      return i;
-  }
-  return ID_UNKNOWN_;
+int64_t getMsgId(const StringRef Name, const MCSubtargetInfo &STI) {
+  int Idx = getOprIdx<const MCSubtargetInfo &>(Name, Msg, MSG_SIZE, STI);
+  return (Idx < 0) ? Idx : Msg[Idx].Encoding;
 }
 
-bool isValidMsgId(int64_t MsgId, const MCSubtargetInfo &STI, bool Strict) {
-  if (Strict) {
-    switch (MsgId) {
-    case ID_SAVEWAVE:
-      return isVI(STI) || isGFX9Plus(STI);
-    case ID_STALL_WAVE_GEN:
-    case ID_HALT_WAVES:
-    case ID_ORDERED_PS_DONE:
-    case ID_GS_ALLOC_REQ:
-    case ID_GET_DOORBELL:
-      return isGFX9Plus(STI);
-    case ID_EARLY_PRIM_DEALLOC:
-      return isGFX9(STI);
-    case ID_GET_DDID:
-      return isGFX10Plus(STI);
-    default:
-      return 0 <= MsgId && MsgId < ID_GAPS_LAST_ && IdSymbolic[MsgId];
-    }
-  } else {
-    return 0 <= MsgId && isUInt<ID_WIDTH_>(MsgId);
-  }
-}
+bool isValidMsgId(int64_t MsgId) {
 
-StringRef getMsgName(int64_t MsgId) {
-  assert(0 <= MsgId && MsgId < ID_GAPS_LAST_);
-  return IdSymbolic[MsgId];
+StringRef getMsgName(int64_t MsgId, const MCSubtargetInfo &STI) {
+  int Idx = getOprIdx<const MCSubtargetInfo &>(MsgId, Msg, MSG_SIZE, STI);
+  return (Idx < 0) ? "" : Msg[Idx].Name;
 }
 
 int64_t getMsgOpId(int64_t MsgId, const StringRef Name) {
   const char* const *S = (MsgId == ID_SYSMSG) ? OpSysSymbolic : OpGsSymbolic;
   const int F = (MsgId == ID_SYSMSG) ? OP_SYS_FIRST_ : OP_GS_FIRST_;
   const int L = (MsgId == ID_SYSMSG) ? OP_SYS_LAST_ : OP_GS_LAST_;
   for (int i = F; i < L; ++i) {
     if (Name == S[i]) {
       return i;
     }
   }
   return OP_UNKNOWN_;
 }
 
 bool isValidMsgOp(int64_t MsgId, int64_t OpId, const MCSubtargetInfo &STI,
                   bool Strict) {
-  assert(isValidMsgId(MsgId, STI, Strict));
+  assert(isValidMsgId(MsgId));
 
   if (!Strict)
     return 0 <= OpId && isUInt<OP_WIDTH_>(OpId);
 
   switch(MsgId)
   {
   case ID_GS:
     return (OP_GS_FIRST_ <= OpId && OpId < OP_GS_LAST_) && OpId != OP_GS_NOP;

 bool isVI(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureVolcanicIslands];
 }
 
 bool isGFX9(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureGFX9];
 }
 
 bool isGFX9_GFX10(const MCSubtargetInfo &STI) {
   return isGFX9(STI) || isGFX10(STI);
 }
 
 bool isGFX9Plus(const MCSubtargetInfo &STI) {
   return isGFX9(STI) || isGFX10Plus(STI);
 }
 
 bool isGFX10(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureGFX10];
 }
 
 bool isGFX10Plus(const MCSubtargetInfo &STI) { return isGFX10(STI); }
 
 bool isGCN3Encoding(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureGCN3Encoding];
 }
 
 bool isGFX10_AEncoding(const MCSubtargetInfo &STI) {
   return STI.getFeatureBits()[AMDGPU::FeatureGFX10_AEncoding];
 }
 
 bool isGFX10_BEncoding(const MCSubtargetInfo &STI) {

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

 
 s_sendmsg sendmsg(MSG_GS_DONE, GS_OP_NOP, 0)
 // GCN: error: message operation does not support streams
 
 s_sendmsg sendmsg(MSG_GS_DONE, 0, 0)
 // GCN: error: message operation does not support streams
 
 s_sendmsg sendmsg(MSG_SAVEWAVE)
-// SICI: error: invalid message id
+// SICI: error: specified message id is not supported on this GPU
 
 s_sendmsg sendmsg(MSG_STALL_WAVE_GEN)
-// SICI: error: invalid message id
-// VI: error: invalid message id
+// SICI: error: specified message id is not supported on this GPU
+// VI: error: specified message id is not supported on this GPU
 
 s_sendmsg sendmsg(MSG_HALT_WAVES)
-// SICI: error: invalid message id
-// VI: error: invalid message id
+// SICI: error: specified message id is not supported on this GPU
+// VI: error: specified message id is not supported on this GPU
 
 s_sendmsg sendmsg(MSG_ORDERED_PS_DONE)
-// SICI: error: invalid message id
-// VI: error: invalid message id
+// SICI: error: specified message id is not supported on this GPU
+// VI: error: specified message id is not supported on this GPU
 
 s_sendmsg sendmsg(MSG_EARLY_PRIM_DEALLOC)
-// SICI: error: invalid message id
-// VI: error: invalid message id
-// GFX10: error: invalid message id
+// SICI: error: specified message id is not supported on this GPU
+// VI: error: specified message id is not supported on this GPU
+// GFX10: error: specified message id is not supported on this GPU
 
 s_sendmsg sendmsg(MSG_GS_ALLOC_REQ)
-// VI: error: invalid message id
-// SICI: error: invalid message id
+// VI: error: specified message id is not supported on this GPU
+// SICI: error: specified message id is not supported on this GPU
 
 s_sendmsg sendmsg(MSG_GS_ALLOC_REQ, 0)
-// VI: error: invalid message id
-// SICI: error: invalid message id
+// VI: error: specified message id is not supported on this GPU
+// SICI: error: specified message id is not supported on this GPU
 // GFX10: error: message does not support operations
 
 s_sendmsg sendmsg(MSG_GET_DOORBELL)
-// SICI: error: invalid message id
-// VI: error: invalid message id
+// SICI: error: specified message id is not supported on this GPU
+// VI: error: specified message id is not supported on this GPU
 
 s_sendmsg sendmsg(MSG_GET_DDID)
-// SICI: error: invalid message id
-// VI: error: invalid message id
+// SICI: error: specified message id is not supported on this GPU
+// VI: error: specified message id is not supported on this GPU
 
 s_sendmsg sendmsg(-1)
 // VI: error: invalid message id
 // SICI: error: invalid message id
 // GFX10: error: invalid message id
 
 s_sendmsg sendmsg(16)
 // VI: error: invalid message id

llvm/test/MC/AMDGPU/sopp.s

 // SI: s_sendmsg sendmsg(4, 0, 0) ; encoding: [0x04,0x00,0x90,0xbf]
 // VI: s_sendmsg sendmsg(MSG_SAVEWAVE) ; encoding: [0x04,0x00,0x90,0xbf]
 
 s_sendmsg sendmsg(4)
 // SI: s_sendmsg sendmsg(4, 0, 0) ; encoding: [0x04,0x00,0x90,0xbf]
 // VI: s_sendmsg sendmsg(MSG_SAVEWAVE) ; encoding: [0x04,0x00,0x90,0xbf]
 
 s_sendmsg sendmsg(MSG_SAVEWAVE)
-// NOSICI: error: invalid message id
+// NOSICI: error: specified message id is not supported on this GPU
 // VI: s_sendmsg sendmsg(MSG_SAVEWAVE) ; encoding: [0x04,0x00,0x90,0xbf]
 
 s_sendmsg 0x1f
 // GCN: s_sendmsg sendmsg(MSG_SYSMSG, SYSMSG_OP_ECC_ERR_INTERRUPT) ; encoding: [0x1f,0x00,0x90,0xbf]
 
 s_sendmsg sendmsg(15, 1)
 // GCN: s_sendmsg sendmsg(MSG_SYSMSG, SYSMSG_OP_ECC_ERR_INTERRUPT) ; encoding: [0x1f,0x00,0x90,0xbf]