[AMDGPU][MC] Enabled constant expressions as operands of s_getreg/s_setreg

[dp]

Summary

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

This change also improves errors reporting (bug 41303).

[dp]

1. Updated to account for GFX10 hw registers.
2. Separated bits juggling from high-level code.

[artem.tamazov]

Please make this small fix and go ahead.

lib/Target/AMDGPU/SIDefines.h

338 ↗

(On Diff #200752)

This enum shall contain only "widths minus one".
If we need "exact" widths (at other levels of abstraction), then let's define dedicated enum for these:

enum Width : unsigned { // Some values from WidthMinusOne mapped into Width domain.
  WIDTH_DEFAULT_ = WIDTH_M1_DEFAULT_ + 1,
};

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

private:
   struct OperandInfoTy {
     int64_t Id;
     bool IsSymbolic = false;
 
     OperandInfoTy(int64_t Id_) : Id(Id_) {}
   };
 
   bool parseSendMsgConstruct(OperandInfoTy &Msg, OperandInfoTy &Operation, int64_t &StreamId);
-  bool parseHwregConstruct(OperandInfoTy &HwReg, int64_t &Offset, int64_t &Width);
+  bool parseHwregBody(OperandInfoTy &HwReg, int64_t &Offset, int64_t &Width);
+  void validateHwreg(const OperandInfoTy &HwReg,
+                     const int64_t Offset,
+                     const int64_t Width,
+                     const SMLoc Loc);
 
   void errorExpTgt();
   OperandMatchResultTy parseExpTgtImpl(StringRef Str, uint8_t &Val);
 
   bool validateInstruction(const MCInst &Inst, const SMLoc &IDLoc);
   bool validateSOPLiteral(const MCInst &Inst) const;
   bool validateConstantBusLimitations(const MCInst &Inst);
   bool validateEarlyClobberLimitations(const MCInst &Inst);
AMDGPUAsmParser::parseSWaitCntOps(OperandVector &Operands) {
   } else {
     parseExpr(Waitcnt);
   }
 
   Operands.push_back(AMDGPUOperand::CreateImm(this, Waitcnt, S));
   return MatchOperand_Success;
 }
 
-bool AMDGPUAsmParser::parseHwregConstruct(OperandInfoTy &HwReg, int64_t &Offset,
-                                          int64_t &Width) {
+bool
+AMDGPUOperand::isSWaitCnt() const {
+  return isImm();
+}
+
+//===----------------------------------------------------------------------===//
+// hwreg
+//===----------------------------------------------------------------------===//
+
+bool
+AMDGPUAsmParser::parseHwregBody(OperandInfoTy &HwReg,
+                                int64_t &Offset,
   using namespace llvm::AMDGPU::Hwreg;
 
-  if (Parser.getTok().getString() != "hwreg")
-    return true;
-  Parser.Lex();
-
-  if (getLexer().isNot(AsmToken::LParen))
-    return true;
-  Parser.Lex();
+  HwReg.IsSymbolic = false;
 
-  if (getLexer().is(AsmToken::Identifier)) {
+  // Check if this is a HW register name
+  if (isToken(AsmToken::Identifier)) {
+    const StringRef Tok = getTokenStr();
+    for (int i = ID_SYMBOLIC_FIRST_; i < ID_SYMBOLIC_LAST_; ++i) {
+      if (Tok == IdSymbolic[i]) {
         HwReg.IsSymbolic = true;
-    HwReg.Id = ID_UNKNOWN_;
-    const StringRef tok = Parser.getTok().getString();
-    int Last = ID_SYMBOLIC_LAST_;
-    if (isSI() || isCI() || isVI())
-      Last = ID_SYMBOLIC_FIRST_GFX9_;
-    for (int i = ID_SYMBOLIC_FIRST_; i < Last; ++i) {
-      if (tok == IdSymbolic[i]) {
         HwReg.Id = i;
+        lex();
         break;
       }
     }
-    Parser.Lex();
-  } else {
-    HwReg.IsSymbolic = false;
-    if (getLexer().isNot(AsmToken::Integer))
-      return true;
-    if (getParser().parseAbsoluteExpression(HwReg.Id))
-      return true;
   }
 
-  if (getLexer().is(AsmToken::RParen)) {
-    Parser.Lex();
+  // The register may be specified by a numeric code
+  if (!HwReg.IsSymbolic && !parseExpr(HwReg.Id))
     return false;
-  }
-
-  // optional params
-  if (getLexer().isNot(AsmToken::Comma))
-    return true;
-  Parser.Lex();
 
-  if (getLexer().isNot(AsmToken::Integer))
-    return true;
-  if (getParser().parseAbsoluteExpression(Offset))
+  if (trySkipToken(AsmToken::RParen))
     return true;
 
-  if (getLexer().isNot(AsmToken::Comma))
-    return true;
-  Parser.Lex();
+  // parse optional params
+  return
+    skipToken(AsmToken::Comma, "expected a comma or a closing parenthesis") &&
+    parseExpr(Offset) &&
+    skipToken(AsmToken::Comma, "expected a comma") &&
+    parseExpr(Width) &&
+    skipToken(AsmToken::RParen, "expected a closing parenthesis");
+}
 
-  if (getLexer().isNot(AsmToken::Integer))
-    return true;
-  if (getParser().parseAbsoluteExpression(Width))
-    return true;
+void
+AMDGPUAsmParser::validateHwreg(const OperandInfoTy &HwReg,
+                               const int64_t Offset,
+                               const int64_t Width,
+                               const SMLoc Loc) {
 
-  if (getLexer().isNot(AsmToken::RParen))
-    return true;
-  Parser.Lex();
+  using namespace llvm::AMDGPU::Hwreg;
 
-  return false;
+  if (HwReg.IsSymbolic) {
+    if ((isSI() || isCI() || isVI()) && HwReg.Id >= ID_SYMBOLIC_FIRST_GFX9_) {
+      Error(Loc, "specified hardware register is not supported on this GPU");
+    }
+  } else if (HwReg.Id < 0 || !isUInt<ID_WIDTH_>(HwReg.Id)) {
+    Error(Loc, "invalid code of hardware register: only 6-bit values are legal");
+  } else if (Offset < 0 || !isUInt<OFFSET_WIDTH_>(Offset)) {
+    Error(Loc, "invalid bit offset: only 5-bit values are legal");
+  } else if ((Width - 1) < 0 || !isUInt<WIDTH_M1_WIDTH_>(Width - 1)) {
+    Error(Loc, "invalid bitfield width: only values from 1 to 32 are legal");
+  }
 }
 
-OperandMatchResultTy AMDGPUAsmParser::parseHwreg(OperandVector &Operands) {
+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_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;
+  int64_t ImmVal = 0;
+  SMLoc Loc = getLoc();
 
-    case AsmToken::Identifier: {
-        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 || !isUInt<OFFSET_WIDTH_>(Offset))
-          Error(S, "invalid bit offset: only 5-bit values are legal");
-        if ((Width-1) < 0 || !isUInt<WIDTH_M1_WIDTH_>(Width-1))
-          Error(S, "invalid bitfield width: only values from 1 to 32 are legal");
-        Imm16Val = (HwReg.Id << ID_SHIFT_) | (Offset << OFFSET_SHIFT_) | ((Width-1) << WIDTH_M1_SHIFT_);
-      }
-      break;
-  }
-  Operands.push_back(AMDGPUOperand::CreateImm(this, Imm16Val, S, AMDGPUOperand::ImmTyHwreg));
-  return MatchOperand_Success;
+  // If parse failed, do not return error code
+  // to avoid excessive error messages.
+  if (trySkipId("hwreg", AsmToken::LParen)) {
+    if (parseHwregBody(HwReg, Offset, Width)) {
+      validateHwreg(HwReg, Offset, Width, Loc);
+      ImmVal = (HwReg.Id << ID_SHIFT_) |
+               (Offset << OFFSET_SHIFT_) |
+               ((Width - 1) << WIDTH_M1_SHIFT_);
+    }
+  } else if (parseExpr(ImmVal)) {
+    if (ImmVal < 0 || !isUInt<16>(ImmVal))
+      Error(Loc, "invalid immediate: only 16-bit values are legal");
   }
 
-bool AMDGPUOperand::isSWaitCnt() const {
-  return isImm();
+  Operands.push_back(AMDGPUOperand::CreateImm(this, ImmVal, Loc, AMDGPUOperand::ImmTyHwreg));
+  return MatchOperand_Success;
 }
 
 bool AMDGPUOperand::isHwreg() const {
   return isImmTy(ImmTyHwreg);
 }
 
+//===----------------------------------------------------------------------===//
+// sendmsg
+//===----------------------------------------------------------------------===//
+
 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))

test/MC/AMDGPU/sopk-err.s

 // RUN: not llvm-mc -arch=amdgcn %s 2>&1 | FileCheck -check-prefix=GCN %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tahiti %s 2>&1 | FileCheck -check-prefix=GCN -check-prefix=SI-ERR %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=tonga %s 2>&1 | FileCheck -check-prefix=GCN -check-prefix=VI-ERR %s
 // RUN: not llvm-mc -arch=amdgcn -mcpu=gfx900 -show-encoding %s 2>&1 | FileCheck -check-prefix=GFX9 %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
+
 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: invalid symbolic name of hardware register
+// GCN: error: expected 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
+// GCN: error: expected a closing parenthesis
 
 s_setreg_b32  hwreg(3,32,32), s2
 // GCN: error: invalid bit offset: only 5-bit values are legal
 
 s_setreg_b32  hwreg(3,0,33), s2
 // GCN: error: invalid bitfield width: only values from 1 to 32 are legal
 
 s_setreg_imm32_b32  0x1f803, 0xff
 // GCN: error: invalid immediate: only 16-bit values are legal
 
 s_setreg_imm32_b32  hwreg(3,0,33), 0xff
 // GCN: error: invalid bitfield width: only values from 1 to 32 are legal
 
 s_getreg_b32  s2, hwreg(3,32,32)
 // GCN: error: invalid bit offset: only 5-bit values are legal
 
 s_getreg_b32 s2, hwreg(HW_REG_SH_MEM_BASES)
-// SI-ERR: error: invalid symbolic name of hardware register
-// VI-ERR: error: invalid symbolic name of hardware register
+// SI-ERR: error: specified hardware register is not supported on this GPU
+// VI-ERR: error: specified hardware register is not supported on this GPU
 // GFX9: s_getreg_b32 s2, hwreg(HW_REG_SH_MEM_BASES) ; encoding: [0x0f,0xf8,0x82,0xb8]
 
 s_cmpk_le_u32 s2, -1
 // GCN: error: invalid operand for instruction
 
 s_cmpk_le_u32 s2, 0x1ffff
 // GCN: error: invalid operand for instruction
 
 s_cmpk_le_u32 s2, 0x10000
 // GCN: error: invalid operand for instruction
 
 s_mulk_i32 s2, 0xFFFFFFFFFFFF0000
 // GCN: error: invalid operand for instruction
 
 s_mulk_i32 s2, 0x10000
 // GCN: error: invalid operand for instruction

test/MC/AMDGPU/sopk.s

 s_mulk_i32 s2, 0xFFFF
 // SICI: s_mulk_i32 s2, 0xffff ; encoding: [0xff,0xff,0x02,0xb8]
 // VI9:  s_mulk_i32 s2, 0xffff ; encoding: [0xff,0xff,0x82,0xb7]
 
 s_cbranch_i_fork s[2:3], 0x6
 // SICI: s_cbranch_i_fork s[2:3], 0x6 ; encoding: [0x06,0x00,0x82,0xb8]
 // VI9:  s_cbranch_i_fork s[2:3], 0x6 ; encoding: [0x06,0x00,0x02,0xb8]
 
+//===----------------------------------------------------------------------===//
+// getreg/setreg and hwreg macro
+//===----------------------------------------------------------------------===//
+
 // raw number mapped to known HW register
 s_getreg_b32 s2, 0x6
 // SICI: s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x02,0xb9]
 // VI9:  s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x82,0xb8]
 
 // HW register identifier, non-default offset/width
 s_getreg_b32 s2, hwreg(HW_REG_GPR_ALLOC, 1, 31)
 // SICI: s_getreg_b32 s2, hwreg(HW_REG_GPR_ALLOC, 1, 31) ; encoding: [0x45,0xf0,0x02,0xb9]

 // SICI: s_setreg_imm32_b32 hwreg(HW_REG_LDS_ALLOC, 0, 1), 0xff ; encoding: [0x06,0x00,0x80,0xba,0xff,0x00,0x00,0x00]
 // VI9:  s_setreg_imm32_b32 hwreg(HW_REG_LDS_ALLOC, 0, 1), 0xff ; encoding: [0x06,0x00,0x00,0xba,0xff,0x00,0x00,0x00]
 
 // HW register identifier, non-default offset/width
 s_setreg_imm32_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), 0xff
 // SICI: s_setreg_imm32_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), 0xff ; encoding: [0x45,0xf0,0x80,0xba,0xff,0x00,0x00,0x00]
 // VI9:  s_setreg_imm32_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), 0xff ; encoding: [0x45,0xf0,0x00,0xba,0xff,0x00,0x00,0x00]
 
+//===----------------------------------------------------------------------===//
+// expressions and hwreg macro
+//===----------------------------------------------------------------------===//
+
+hwreg=6
+s_getreg_b32 s2, hwreg
+// SICI: s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x02,0xb9]
+// VI9:  s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x82,0xb8]
+
+x=5
+s_getreg_b32 s2, x+1
+// SICI: s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x02,0xb9]
+// VI9:  s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x82,0xb8]
+
+x=5
+s_getreg_b32 s2, 1+x
+// SICI: s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x02,0xb9]
+// VI9:  s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x82,0xb8]
+
+reg=50
+offset=2
+width=30
+s_getreg_b32 s2, hwreg(reg + 1, offset - 1, width + 1)
+// SICI: s_getreg_b32 s2, hwreg(51, 1, 31) ; encoding: [0x73,0xf0,0x02,0xb9]
+// VI9:  s_getreg_b32 s2, hwreg(51, 1, 31) ; encoding: [0x73,0xf0,0x82,0xb8]
+
+s_getreg_b32 s2, hwreg(1 + reg, -1 + offset, 1 + width)
+// SICI: s_getreg_b32 s2, hwreg(51, 1, 31) ; encoding: [0x73,0xf0,0x02,0xb9]
+// VI9:  s_getreg_b32 s2, hwreg(51, 1, 31) ; encoding: [0x73,0xf0,0x82,0xb8]
+
+//===----------------------------------------------------------------------===//
+// Instructions
+//===----------------------------------------------------------------------===//
+
 s_endpgm_ordered_ps_done
 // GFX9:     s_endpgm_ordered_ps_done ; encoding: [0x00,0x00,0x9e,0xbf]
 // NOSICIVI: error: instruction not supported on this GPU
 
 s_call_b64 s[12:13], 12609
 // GFX9:     s_call_b64 s[12:13], 0x3141 ; encoding: [0x41,0x31,0x8c,0xba]
 // NOSICIVI: error: instruction not supported on this GPU
 
 s_call_b64 s[100:101], 12609
 // GFX9:     s_call_b64 s[100:101], 0x3141 ; encoding: [0x41,0x31,0xe4,0xba]
 // NOSICIVI: error: instruction not supported on this GPU
 
 s_call_b64 s[10:11], 49617
 // GFX9:     s_call_b64 s[10:11], 0xc1d1 ; encoding: [0xd1,0xc1,0x8a,0xba]
 // NOSICIVI: error: instruction not supported on this GPU