[AMDGPU][llvm-mc] s_getreg/setreg* - hwreg - factor out strings/literals etc.

[artem.tamazov]

Summary

Hwreg(...) syntax implementation unified with sendmsg(...).
Common strings moved to Utils
MathExtras.h functionality utilized.
Added missing build dependency in Disassembler.

[artem.tamazov]

Going to commit in 24hrs if none comments arrive.

[arsenm]

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

1729 ↗

(On Diff #57665)

Is this really a StringRef?

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

919 ↗

(On Diff #57665)

Demorgan's law this

[SamWot]

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

1727–1735 ↗

(On Diff #57665)

Why do you need to change this? Previous variant looks much more readable for me?

1801 ↗

(On Diff #57665)

You should return MatchOperand_ParseFail here.
parseHwregOperand could already eat some tokens, so this will damage later parsers.
I use this rule to decide where you should return NoMatch or ParseFail:

• NoMatch - if this parser didn't yet eat any tokens and it is possible that current string can be parsed by any other parser.
• ParseFail - we ate some tokens or current string should not be parsed by any other parser.

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

906–920 ↗

(On Diff #57665)

Why do you need to change this? Previous variant looks much more readable for me?

[artem.tamazov]

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

1727–1735 ↗

(On Diff #57665)

We want to define strings used in asmparser and instprinter in one place. Can't use stringswitch with externally defined symbols.

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

906–920 ↗

(On Diff #57665)

Same reason.

[artem.tamazov]

Review fixes done.

[arsenm]

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

72–76 ↗

(On Diff #58114)

Why not put this table in Utils and avoid repeating it?

test/MC/AMDGPU/sopp-err.s

39 ↗

(On Diff #58114)

Should the message say which operand?

[artem.tamazov]

BTW thanks for reviewing))

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

72–76 ↗

(On Diff #58114)

That is planned to be done in separate change. For now, there is
// FIXME ODR: Move this to some common place for AsmParser and InstPrinter

test/MC/AMDGPU/sopp-err.s

39 ↗

(On Diff #58114)

Actually, there is a caret symbol which points to the operand:

C:\work-1\asm-git-1\playground\debugging.s:1:28: error: not a valid operand.
s_sendmsg sendmsg(2, 3, 0, 0)
                           ^

But normally we do not check everything. Shall we?

[artem.tamazov]

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

72–76 ↗

(On Diff #58114)

Pls let me know if you think it is better to do that just in this patch.

[arsenm]

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

72–76 ↗

(On Diff #58114)

Probably best to just do it now

[artem.tamazov]

Review fixes.

[artem.tamazov]

Ready for the next review round.

[arsenm]

LGTM

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

1719 ↗

(On Diff #58419)

Can you use isMask_32 for this?

lib/Target/AMDGPU/Utils/AMDGPUAsmParserPrinter.h

1 ↗

(On Diff #58419)

Missing C++ mode comment

[artem.tamazov]

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

1719 ↗

(On Diff #58419)

Thanks for the tip. It seems that isShiftedUInt() can be also used in some places... Will try to use as much as possible stuff from MathExtras.h and update the review.

Note that current design (factored out enums and strings) does not look readable and easy to use. Perhaps it is worth to implement and use deeper abstractions, perhaps classes which clearly encapsulate sendmsg/hwreg functionality required for the parser/printer. For example, Hwreg::getIdSymbolic(enum Id id) or HwReg::getIdNumeric(StringRef& identifier) etc. Let's do that in the future, when time permits.

[artem.tamazov]

Review fixed done. Pre-submit testing started.

[artem.tamazov]

whitespace fixes

[SamWot]

LGTM

lib/Target/AMDGPU/Utils/AMDGPUAsmParserPrinter.h

1 ↗

(On Diff #58601)

Can you rename this file to something like AMDGPUAsmUtils? This name confuses me alot: there are AsmParser, AsmPrinter and InstPrinter.

[artem.tamazov]

New files in Utils renamed.

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

 //===-- AMDGPUAsmParser.cpp - Parse SI asm to MCInst instructions ---------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AMDKernelCodeT.h"
 #include "MCTargetDesc/AMDGPUMCTargetDesc.h"
 #include "MCTargetDesc/AMDGPUTargetStreamer.h"
 #include "SIDefines.h"
 #include "Utils/AMDGPUBaseInfo.h"
 #include "Utils/AMDKernelCodeTUtils.h"
+#include "Utils/AMDGPUAsmUtils.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCParser/MCAsmLexer.h"
 #include "llvm/MC/MCParser/MCAsmParser.h"
 #include "llvm/MC/MCParser/MCParsedAsmOperand.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCSymbolELF.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ELF.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
-
-// FIXME ODR: Move this to some common place for AsmParser and InstPrinter
-namespace llvm {
-namespace AMDGPU {
-namespace SendMsg {
-
-// This must be in sync with llvm::AMDGPU::SendMsg::Id enum members.
-static
-const char* const IdSymbolic[] = {
-  nullptr,
-  "MSG_INTERRUPT",
-  "MSG_GS",
-  "MSG_GS_DONE",
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  "MSG_SYSMSG"
-};
-
-// These two must be in sync with llvm::AMDGPU::SendMsg::Op enum members.
-static
-const char* const OpSysSymbolic[] = {
-  nullptr,
-  "SYSMSG_OP_ECC_ERR_INTERRUPT",
-  "SYSMSG_OP_REG_RD",
-  "SYSMSG_OP_HOST_TRAP_ACK",
-  "SYSMSG_OP_TTRACE_PC"
-};
-
-static
-const char* const OpGsSymbolic[] = {
-  "GS_OP_NOP",
-  "GS_OP_CUT",
-  "GS_OP_EMIT",
-  "GS_OP_EMIT_CUT"
-};
-
-} // namespace SendMsg
-} // namespace AMDGPU
-} // namespace llvm
+#include "llvm/Support/MathExtras.h"
 
 using namespace llvm;
 
 namespace {
 
 struct OptionalOperand;
 
 enum RegisterKind { IS_UNKNOWN, IS_VGPR, IS_SGPR, IS_TTMP, IS_SPECIAL };
public:
   OperandMatchResultTy parseRegOrImm(OperandVector &Operands);
   OperandMatchResultTy parseRegOrImmWithInputMods(OperandVector &Operands);
 
   void cvtDSOffset01(MCInst &Inst, const OperandVector &Operands);
   void cvtDS(MCInst &Inst, const OperandVector &Operands);
 
   bool parseCnt(int64_t &IntVal);
   OperandMatchResultTy parseSWaitCntOps(OperandVector &Operands);
-  bool parseHwregOperand(int64_t &HwRegCode, int64_t &Offset, int64_t &Width, bool &IsIdentifier);
   OperandMatchResultTy parseHwreg(OperandVector &Operands);
 
 private:
   struct OperandInfoTy {
     int64_t Id;
     bool IsSymbolic;
     OperandInfoTy(int64_t Id_) : Id(Id_), IsSymbolic(false) { }
   };
 
-  bool parseSendMsg(OperandInfoTy &Msg, OperandInfoTy &Operation, int64_t &StreamId);
-
+  bool parseSendMsgConstruct(OperandInfoTy &Msg, OperandInfoTy &Operation, int64_t &StreamId);
+  bool parseHwregConstruct(OperandInfoTy &HwReg, int64_t &Offset, int64_t &Width);
 public:
   OperandMatchResultTy parseOptionalOperand(OperandVector &Operands);
 
   OperandMatchResultTy parseSendMsgOp(OperandVector &Operands);
   OperandMatchResultTy parseSOppBrTarget(OperandVector &Operands);
   AMDGPUOperand::Ptr defaultHwreg() const;
 
   void cvtMubuf(MCInst &Inst, const OperandVector &Operands) { cvtMubufImpl(Inst, Operands, false, false); }
case AsmToken::Identifier:
           return MatchOperand_ParseFail;
       } while(getLexer().isNot(AsmToken::EndOfStatement));
       break;
   }
   Operands.push_back(AMDGPUOperand::CreateImm(CntVal, S));
   return MatchOperand_Success;
 }
 
-bool AMDGPUAsmParser::parseHwregOperand(int64_t &HwRegCode, int64_t &Offset, int64_t &Width, bool &IsIdentifier) {
+bool AMDGPUAsmParser::parseHwregConstruct(OperandInfoTy &HwReg, int64_t &Offset, int64_t &Width) {
+  using namespace llvm::AMDGPU::Hwreg;
+
   if (Parser.getTok().getString() != "hwreg")
     return true;
   Parser.Lex();
 
   if (getLexer().isNot(AsmToken::LParen))
     return true;
   Parser.Lex();
 
   if (getLexer().is(AsmToken::Identifier)) {
-    IsIdentifier = true;
-    HwRegCode = StringSwitch<unsigned>(Parser.getTok().getString())
-      .Case("HW_REG_MODE"     , 1)
-      .Case("HW_REG_STATUS"   , 2)
-      .Case("HW_REG_TRAPSTS"  , 3)
-      .Case("HW_REG_HW_ID"    , 4)
-      .Case("HW_REG_GPR_ALLOC", 5)
-      .Case("HW_REG_LDS_ALLOC", 6)
-      .Case("HW_REG_IB_STS"   , 7)
-      .Default(-1);
+    HwReg.IsSymbolic = true;
+    HwReg.Id = ID_UNKNOWN_;
+    const StringRef tok = Parser.getTok().getString();
+    for (int i = ID_SYMBOLIC_FIRST_; i < ID_SYMBOLIC_LAST_; ++i) {
+      if (tok == IdSymbolic[i]) {
+        HwReg.Id = i;
+        break;
+      }
+    }
     Parser.Lex();
   } else {
-    IsIdentifier = false;
+    HwReg.IsSymbolic = false;
     if (getLexer().isNot(AsmToken::Integer))
       return true;
-    if (getParser().parseAbsoluteExpression(HwRegCode))
+    if (getParser().parseAbsoluteExpression(HwReg.Id))
       return true;
   }
 
   if (getLexer().is(AsmToken::RParen)) {
     Parser.Lex();
     return false;
   }
 
if (getLexer().isNot(AsmToken::RParen))
     return true;
   Parser.Lex();
 
   return false;
 }
 
 AMDGPUAsmParser::OperandMatchResultTy
 AMDGPUAsmParser::parseHwreg(OperandVector &Operands) {
+  using namespace llvm::AMDGPU::Hwreg;
+
   int64_t Imm16Val = 0;
   SMLoc S = Parser.getTok().getLoc();
 
   switch(getLexer().getKind()) {
     default: return MatchOperand_NoMatch;
     case AsmToken::Integer:
       // The operand can be an integer value.
       if (getParser().parseAbsoluteExpression(Imm16Val))
-        return MatchOperand_ParseFail;
-      if (!isInt<16>(Imm16Val) && !isUInt<16>(Imm16Val)) {
+        return MatchOperand_NoMatch;
+      if (Imm16Val < 0 || !isUInt<16>(Imm16Val)) {
         Error(S, "invalid immediate: only 16-bit values are legal");
         // Do not return error code, but create an imm operand anyway and proceed
         // to the next operand, if any. That avoids unneccessary error messages.
       }
       break;
 
     case AsmToken::Identifier: {
-        bool IsIdentifier = false;
-        int64_t HwRegCode = -1;
-        int64_t Offset = 0; // default
-        int64_t Width = 32; // default
-        if (parseHwregOperand(HwRegCode, Offset, Width, IsIdentifier))
-          return MatchOperand_ParseFail;
-        // HwRegCode (6) [5:0]
-        // Offset (5) [10:6]
-        // WidthMinusOne (5) [15:11]
-        if (HwRegCode < 0 || HwRegCode > 63) {
-          if (IsIdentifier)
+        OperandInfoTy HwReg(ID_UNKNOWN_);
+        int64_t Offset = OFFSET_DEFAULT_;
+        int64_t Width = WIDTH_M1_DEFAULT_ + 1;
+        if (parseHwregConstruct(HwReg, Offset, Width))
+          return MatchOperand_ParseFail;
+        if (HwReg.Id < 0 || !isUInt<ID_WIDTH_>(HwReg.Id)) {
+          if (HwReg.IsSymbolic)
             Error(S, "invalid symbolic name of hardware register");
           else
             Error(S, "invalid code of hardware register: only 6-bit values are legal");
         }
-        if (Offset < 0 || Offset > 31)
+        if (Offset < 0 || !isUInt<OFFSET_WIDTH_>(Offset))
           Error(S, "invalid bit offset: only 5-bit values are legal");
-        if (Width < 1 || Width > 32)
+        if ((Width-1) < 0 || !isUInt<WIDTH_M1_WIDTH_>(Width-1))
           Error(S, "invalid bitfield width: only values from 1 to 32 are legal");
-        Imm16Val = HwRegCode | (Offset << 6) | ((Width-1) << 11);
+        Imm16Val = (HwReg.Id << ID_SHIFT_) | (Offset << OFFSET_SHIFT_) | ((Width-1) << WIDTH_M1_SHIFT_);
       }
       break;
   }
   Operands.push_back(AMDGPUOperand::CreateImm(Imm16Val, S, AMDGPUOperand::ImmTyHwreg));
   return MatchOperand_Success;
 }
 
 bool AMDGPUOperand::isSWaitCnt() const {
   return isImm();
 }
 
 bool AMDGPUOperand::isHwreg() const {
   return isImmTy(ImmTyHwreg);
 }
 
 AMDGPUOperand::Ptr AMDGPUAsmParser::defaultHwreg() const {
   return AMDGPUOperand::CreateImm(0, SMLoc(), AMDGPUOperand::ImmTyHwreg);
 }
 
-bool AMDGPUAsmParser::parseSendMsg(OperandInfoTy &Msg, OperandInfoTy &Operation, int64_t &StreamId) {
+bool AMDGPUAsmParser::parseSendMsgConstruct(OperandInfoTy &Msg, OperandInfoTy &Operation, int64_t &StreamId) {
   using namespace llvm::AMDGPU::SendMsg;
 
   if (Parser.getTok().getString() != "sendmsg")
     return true;
   Parser.Lex();
 
   if (getLexer().isNot(AsmToken::LParen))
     return true;
bool AMDGPUAsmParser::parseSendMsgConstruct(OperandInfoTy &Msg, OperandInfoTy &Operation, int64_t &StreamId) {
 
   assert(Msg.Id == ID_GS || Msg.Id == ID_GS_DONE || Msg.Id == ID_SYSMSG);
   Operation.Id = ID_UNKNOWN_;
   if (getLexer().is(AsmToken::Identifier)) {
     Operation.IsSymbolic = true;
     const char* const *S = (Msg.Id == ID_SYSMSG) ? OpSysSymbolic : OpGsSymbolic;
     const int F = (Msg.Id == ID_SYSMSG) ? OP_SYS_FIRST_ : OP_GS_FIRST_;
     const int L = (Msg.Id == ID_SYSMSG) ? OP_SYS_LAST_ : OP_GS_LAST_;
-    const std::string Tok = Parser.getTok().getString();
+    const StringRef Tok = Parser.getTok().getString();
     for (int i = F; i < L; ++i) {
       if (Tok == S[i]) {
         Operation.Id = i;
         break;
       }
     }
     Parser.Lex();
   } else {
AMDGPUAsmParser::parseSendMsgOp(OperandVector &Operands) {
 
   switch(getLexer().getKind()) {
   default:
     return MatchOperand_NoMatch;
   case AsmToken::Integer:
     // The operand can be an integer value.
     if (getParser().parseAbsoluteExpression(Imm16Val))
       return MatchOperand_NoMatch;
-    if (!isInt<16>(Imm16Val) && !isUInt<16>(Imm16Val)) {
+    if (Imm16Val < 0 || !isUInt<16>(Imm16Val)) {
       Error(S, "invalid immediate: only 16-bit values are legal");
       // Do not return error code, but create an imm operand anyway and proceed
       // to the next operand, if any. That avoids unneccessary error messages.
     }
     break;
   case AsmToken::Identifier: {
       OperandInfoTy Msg(ID_UNKNOWN_);
       OperandInfoTy Operation(OP_UNKNOWN_);
-      int64_t StreamId = STREAM_ID_DEFAULT;
-      if (parseSendMsg(Msg, Operation, StreamId))
-        return MatchOperand_NoMatch;
+      int64_t StreamId = STREAM_ID_DEFAULT_;
+      if (parseSendMsgConstruct(Msg, Operation, StreamId))
+        return MatchOperand_ParseFail;
       do {
         // Validate and encode message ID.
         if (! ((ID_INTERRUPT <= Msg.Id && Msg.Id <= ID_GS_DONE)
                 || Msg.Id == ID_SYSMSG)) {
           if (Msg.IsSymbolic)
             Error(S, "invalid/unsupported symbolic name of message");
           else
             Error(S, "invalid/unsupported code of message");
           break;
         }
-        Imm16Val = Msg.Id;
+        Imm16Val = (Msg.Id << ID_SHIFT_);
         // Validate and encode operation ID.
         if (Msg.Id == ID_GS || Msg.Id == ID_GS_DONE) {
           if (! (OP_GS_FIRST_ <= Operation.Id && Operation.Id < OP_GS_LAST_)) {
             if (Operation.IsSymbolic)
               Error(S, "invalid symbolic name of GS_OP");
             else
               Error(S, "invalid code of GS_OP: only 2-bit values are legal");
             break;

llvm/trunk/lib/Target/AMDGPU/Disassembler/CMakeLists.txt

 include_directories( ${CMAKE_CURRENT_BINARY_DIR}/.. ${CMAKE_CURRENT_SOURCE_DIR}/.. )
 
 add_llvm_library(LLVMAMDGPUDisassembler
   AMDGPUDisassembler.cpp
   )
 
-add_dependencies(LLVMAMDGPUDisassembler AMDGPUCommonTableGen)
+add_dependencies(LLVMAMDGPUDisassembler AMDGPUCommonTableGen LLVMAMDGPUUtils)

llvm/trunk/lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

 //===-- AMDGPUInstPrinter.cpp - AMDGPU MC Inst -> ASM ---------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 // \file
 //===----------------------------------------------------------------------===//
 
 #include "AMDGPUInstPrinter.h"
 #include "MCTargetDesc/AMDGPUMCTargetDesc.h"
 #include "SIDefines.h"
+#include "Utils/AMDGPUAsmUtils.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <string>
 
-// FIXME ODR: Move this to some common place for AsmParser and InstPrinter
-namespace llvm {
-namespace AMDGPU {
-namespace SendMsg {
-
-// This must be in sync with llvm::AMDGPU::SendMsg::Id enum members.
-static
-const char* const IdSymbolic[] = {
-  nullptr,
-  "MSG_INTERRUPT",
-  "MSG_GS",
-  "MSG_GS_DONE",
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  "MSG_SYSMSG"
-};
-
-// These two must be in sync with llvm::AMDGPU::SendMsg::Op enum members.
-static
-const char* const OpSysSymbolic[] = {
-  nullptr,
-  "SYSMSG_OP_ECC_ERR_INTERRUPT",
-  "SYSMSG_OP_REG_RD",
-  "SYSMSG_OP_HOST_TRAP_ACK",
-  "SYSMSG_OP_TTRACE_PC"
-};
-
-static
-const char* const OpGsSymbolic[] = {
-  "GS_OP_NOP",
-  "GS_OP_CUT",
-  "GS_OP_EMIT",
-  "GS_OP_EMIT_CUT"
-};
-
-} // namespace SendMsg
-} // namespace AMDGPU
-} // namespace llvm
-
 using namespace llvm;
 
 void AMDGPUInstPrinter::printInst(const MCInst *MI, raw_ostream &OS,
                                   StringRef Annot, const MCSubtargetInfo &STI) {
   OS.flush();
   printInstruction(MI, OS);
 
   printAnnotation(OS, Annot);
if (Lgkmcnt != 0xF) {
     if (NeedSpace)
       O << ' ';
     O << "lgkmcnt(" << Lgkmcnt << ')';
   }
 }
 
 void AMDGPUInstPrinter::printHwreg(const MCInst *MI, unsigned OpNo,
                                    raw_ostream &O) {
+  using namespace llvm::AMDGPU::Hwreg;
+
   unsigned SImm16 = MI->getOperand(OpNo).getImm();
-  const unsigned HwRegCode = SImm16 & 0x3F;
-  const unsigned Offset = (SImm16 >> 6) & 0x1f;
-  const unsigned Width = ((SImm16 >> 11) & 0x1F) + 1;
+  const unsigned Id = (SImm16 & ID_MASK_) >> ID_SHIFT_;
+  const unsigned Offset = (SImm16 & OFFSET_MASK_) >> OFFSET_SHIFT_;
+  const unsigned Width = ((SImm16 & WIDTH_M1_MASK_) >> WIDTH_M1_SHIFT_) + 1;
 
   O << "hwreg(";
-  switch(HwRegCode) {
-    case 1: O << "HW_REG_MODE"      ; break;
-    case 2: O << "HW_REG_STATUS"    ; break;
-    case 3: O << "HW_REG_TRAPSTS"   ; break;
-    case 4: O << "HW_REG_HW_ID"     ; break;
-    case 5: O << "HW_REG_GPR_ALLOC" ; break;
-    case 6: O << "HW_REG_LDS_ALLOC" ; break;
-    case 7: O << "HW_REG_IB_STS"    ; break;
-    default: O << HwRegCode; break;
+  if (ID_SYMBOLIC_FIRST_ <= Id && Id < ID_SYMBOLIC_LAST_) {
+    O << IdSymbolic[Id];
+  } else {
+    O << Id;
   }
-  if (! (Width == 32 && Offset == 0)) {
+  if (Width != WIDTH_M1_DEFAULT_ + 1 || Offset != OFFSET_DEFAULT_) {
     O << ", " << Offset << ", " << Width;
   }
   O << ')';
 }
 
 #include "AMDGPUGenAsmWriter.inc"

llvm/trunk/lib/Target/AMDGPU/InstPrinter/CMakeLists.txt

 add_llvm_library(LLVMAMDGPUAsmPrinter
   AMDGPUInstPrinter.cpp
   )
+
+add_dependencies(LLVMAMDGPUAsmPrinter LLVMAMDGPUUtils)

llvm/trunk/lib/Target/AMDGPU/InstPrinter/LLVMBuild.txt

 ;   http://llvm.org/docs/LLVMBuild.html
 ;
 ;===------------------------------------------------------------------------===;
 
 [component_0]
 type = Library
 name = AMDGPUAsmPrinter
 parent = AMDGPU
-required_libraries = MC Support
+required_libraries = MC Support AMDGPUUtils
 add_to_library_groups = AMDGPU
 

llvm/trunk/lib/Target/AMDGPU/SIDefines.h

 } // namespace EncValues
 } // namespace AMDGPU
 } // namespace llvm
 
 namespace llvm {
 namespace AMDGPU {
 namespace SendMsg { // Encoding of SIMM16 used in s_sendmsg* insns.
 
-enum Id { // Message ID, width(3) [3:0].
+enum Id { // Message ID, width(4) [3:0].
   ID_UNKNOWN_ = -1,
   ID_INTERRUPT = 1,
   ID_GS,
   ID_GS_DONE,
   ID_SYSMSG = 15,
   ID_GAPS_LAST_, // Indicate that sequence has gaps.
   ID_GAPS_FIRST_ = ID_INTERRUPT,
-  ID_MASK_ = 0xf
+  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,
   // width(2) [5:4]
   OP_GS_NOP = 0,
   OP_GS_CUT,
   OP_GS_EMIT,
   OP_GS_EMIT_CUT,
   OP_GS_LAST_,
   OP_GS_FIRST_ = OP_GS_NOP,
-  OP_GS_MASK_ = (0x3 << 4),
+  OP_GS_WIDTH_ = 2,
+  OP_GS_MASK_ = (((1 << OP_GS_WIDTH_) - 1) << OP_SHIFT_),
   // width(3) [6:4]
   OP_SYS_ECC_ERR_INTERRUPT = 1,
   OP_SYS_REG_RD,
   OP_SYS_HOST_TRAP_ACK,
   OP_SYS_TTRACE_PC,
   OP_SYS_LAST_,
   OP_SYS_FIRST_ = OP_SYS_ECC_ERR_INTERRUPT,
-  OP_SYS_MASK_ = (0x7 << 4),
-  OP_SHIFT_ = 4
+  OP_SYS_WIDTH_ = 3,
+  OP_SYS_MASK_ = (((1 << OP_SYS_WIDTH_) - 1) << OP_SHIFT_)
 };
 
 enum StreamId { // Stream ID, (2) [9:8].
-  STREAM_ID_DEFAULT = 0,
+  STREAM_ID_DEFAULT_ = 0,
   STREAM_ID_LAST_ = 4,
-  STREAM_ID_FIRST_ = STREAM_ID_DEFAULT,
-  STREAM_ID_MASK_ = (0x3 << 8),
-  STREAM_ID_SHIFT_ = 8
+  STREAM_ID_FIRST_ = STREAM_ID_DEFAULT_,
+  STREAM_ID_SHIFT_ = 8,
+  STREAM_ID_WIDTH_=  2,
+  STREAM_ID_MASK_ = (((1 << STREAM_ID_WIDTH_) - 1) << STREAM_ID_SHIFT_)
 };
 
 } // namespace SendMsg
+
+namespace Hwreg { // Encoding of SIMM16 used in s_setreg/getreg* insns.
+
+enum Id { // HwRegCode, (6) [5:0]
+  ID_UNKNOWN_ = -1,
+  ID_SYMBOLIC_FIRST_ = 1, // There are corresponding symbolic names defined.
+  ID_SYMBOLIC_LAST_ = 8,
+  ID_SHIFT_ = 0,
+  ID_WIDTH_ = 6,
+  ID_MASK_ = (((1 << ID_WIDTH_) - 1) << ID_SHIFT_)
+};
+
+enum Offset { // Offset, (5) [10:6]
+  OFFSET_DEFAULT_ = 0,
+  OFFSET_SHIFT_ = 6,
+  OFFSET_WIDTH_ = 5,
+  OFFSET_MASK_ = (((1 << OFFSET_WIDTH_) - 1) << OFFSET_SHIFT_)
+};
+
+enum WidthMinusOne { // WidthMinusOne, (5) [15:11]
+  WIDTH_M1_DEFAULT_ = 31,
+  WIDTH_M1_SHIFT_ = 11,
+  WIDTH_M1_WIDTH_ = 5,
+  WIDTH_M1_MASK_ = (((1 << WIDTH_M1_WIDTH_) - 1) << WIDTH_M1_SHIFT_)
+};
+
+} // namespace Hwreg
 } // namespace AMDGPU
 } // namespace llvm
 
 #define R_00B028_SPI_SHADER_PGM_RSRC1_PS                                0x00B028
 #define R_00B02C_SPI_SHADER_PGM_RSRC2_PS                                0x00B02C
 #define   S_00B02C_EXTRA_LDS_SIZE(x)                                  (((x) & 0xFF) << 8)
 #define R_00B128_SPI_SHADER_PGM_RSRC1_VS                                0x00B128
 #define R_00B228_SPI_SHADER_PGM_RSRC1_GS                                0x00B228

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

+//===-- AMDGPUAsmUtils.h - AsmParser/InstPrinter common ---------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TARGET_AMDGPU_UTILS_AMDGPUASMUTILS_H
+#define LLVM_LIB_TARGET_AMDGPU_UTILS_AMDGPUASMUTILS_H
+
+namespace llvm {
+namespace AMDGPU {
+namespace SendMsg { // Symbolic names for the sendmsg(...) syntax.
+
+extern const char* const IdSymbolic[];
+extern const char* const OpSysSymbolic[];
+extern const char* const OpGsSymbolic[];
+
+} // namespace SendMsg
+
+namespace Hwreg { // Symbolic names for the hwreg(...) syntax.
+
+extern const char* const IdSymbolic[];
+
+} // namespace Hwreg
+} // namespace AMDGPU
+} // namespace llvm
+
+#endif

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

+//===-- AMDGPUAsmUtils.cpp - AsmParser/InstPrinter common -----------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include "AMDGPUAsmUtils.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[] = {
+  nullptr,
+  "MSG_INTERRUPT",
+  "MSG_GS",
+  "MSG_GS_DONE",
+  nullptr,
+  nullptr,
+  nullptr,
+  nullptr,
+  nullptr,
+  nullptr,
+  nullptr,
+  nullptr,
+  nullptr,
+  nullptr,
+  nullptr,
+  "MSG_SYSMSG"
+};
+
+// These two must be in sync with llvm::AMDGPU::SendMsg::Op enum members, see SIDefines.h.
+const char* const OpSysSymbolic[] = {
+  nullptr,
+  "SYSMSG_OP_ECC_ERR_INTERRUPT",
+  "SYSMSG_OP_REG_RD",
+  "SYSMSG_OP_HOST_TRAP_ACK",
+  "SYSMSG_OP_TTRACE_PC"
+};
+
+const char* const OpGsSymbolic[] = {
+  "GS_OP_NOP",
+  "GS_OP_CUT",
+  "GS_OP_EMIT",
+  "GS_OP_EMIT_CUT"
+};
+
+} // namespace SendMsg
+
+namespace Hwreg {
+
+// This must be in sync with llvm::AMDGPU::Hwreg::ID_SYMBOLIC_FIRST_/LAST_, see SIDefines.h.
+const char* const IdSymbolic[] = {
+  nullptr,
+  "HW_REG_MODE",
+  "HW_REG_STATUS",
+  "HW_REG_TRAPSTS",
+  "HW_REG_HW_ID",
+  "HW_REG_GPR_ALLOC",
+  "HW_REG_LDS_ALLOC",
+  "HW_REG_IB_STS"
+};
+
+} // namespace Hwreg
+} // namespace AMDGPU
+} // namespace llvm

llvm/trunk/lib/Target/AMDGPU/Utils/CMakeLists.txt

 add_llvm_library(LLVMAMDGPUUtils
   AMDGPUBaseInfo.cpp
   AMDKernelCodeTUtils.cpp
+  AMDGPUAsmUtils.cpp
   )

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

 // RUN: not llvm-mc -arch=amdgcn -show-encoding %s 2>&1 | FileCheck --check-prefix=GCN --check-prefix=SICI %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=SI -show-encoding %s 2>&1 | FileCheck --check-prefix=GCN --check-prefix=SICI %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=fiji -show-encoding %s 2>&1 | FileCheck --check-prefix=GCN --check-prefix=VI %s
 
 s_sendmsg sendmsg(11)
 // GCN: error: invalid/unsupported code of message
 
 s_sendmsg sendmsg(MSG_INTERRUPTX)
 // GCN: error: invalid/unsupported symbolic name of message
 
 s_sendmsg sendmsg(MSG_INTERRUPT, 0)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(MSG_GS)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_NOP)
 // GCN: error: invalid GS_OP: NOP is for GS_DONE only
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUT, 0, 0)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(MSG_GSX, GS_OP_CUT, 0)
 // GCN: error: invalid/unsupported symbolic name of message
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUTX, 0)
 // GCN: error: invalid symbolic name of GS_OP
 
 s_sendmsg sendmsg(MSG_GS, GS_OP_CUT, 4)
 // GCN: error: invalid stream id: only 2-bit values are legal
 
 s_sendmsg sendmsg(2)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(2, 0)
 // GCN: error: invalid GS_OP: NOP is for GS_DONE only
 
 s_sendmsg sendmsg(2, 3, 0, 0)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(2, 4, 1)
 // GCN: error: invalid code of GS_OP: only 2-bit values are legal
 
 s_sendmsg sendmsg(2, 2, 4)
 // GCN: error: invalid stream id: only 2-bit values are legal
 
 s_sendmsg sendmsg(2, 2, 0, 0)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(MSG_GS_DONE, GS_OP_NOP, 0)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(15)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(15, 1, 0)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(15, 0)
 // GCN: error: invalid/unsupported code of SYSMSG_OP
 
 s_sendmsg sendmsg(15, 5)
 // GCN: error: invalid/unsupported code of SYSMSG_OP
 
 s_sendmsg sendmsg(MSG_SYSMSG)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(MSG_SYSMSG, SYSMSG_OP_ECC_ERR_INTERRUPT, 0)
-// GCN: error: not a valid operand
+// GCN: error: failed parsing operand
 
 s_sendmsg sendmsg(MSG_SYSMSG, 0)
 // GCN: error: invalid/unsupported code of SYSMSG_OP
 
 s_sendmsg sendmsg(MSG_SYSMSG, 5)
 // GCN: error: invalid/unsupported code of SYSMSG_OP