[AMDGPU][llvm-mc] s_getreg/setreg* - Support symbolic names of hardware registers.

[artem.tamazov]

Summary

Possibility to specify code of hardware register kept.
Disassemble to symbolic name, if name is known.
Tests updated/added.

[artem.tamazov]

Pong...

[arsenm]

If the raw numbers are supposed to work still, those tests should be kept

[artem.tamazov]

In D19335#409765, @arsenm wrote:

If the raw numbers are supposed to work still, those tests should be kept

Assembly tests with raw numbers are still there, like "s_setreg_imm32_b32 0x6, 0xff" in sopk.s. However, disassembly is always to "hwreg(...)" format, that is why raw numbers were replaced in Disassembler and CodeGen tests.

[nhaustov]

Ok.

[artem.tamazov]

Added some test cases with raw HW register codes

[artem.tamazov]

Added more raw number/register code tests

[artem.tamazov]

I am going to commit this after 24hrs, if no objections/comments arrive.

lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

 
   bool parseCnt(int64_t &IntVal);
   OperandMatchResultTy parseSWaitCntOps(OperandVector &Operands);
-  bool parseHwreg(int64_t &HwRegCode, int64_t &Offset, int64_t &Width);
+  bool parseHwreg(int64_t &HwRegCode, int64_t &Offset, int64_t &Width, bool &IsIdentifier);
   OperandMatchResultTy parseHwregOp(OperandVector &Operands);
   OperandMatchResultTy parseSOppBrTarget(OperandVector &Operands);
 

   return MatchOperand_Success;
 }
 
-bool AMDGPUAsmParser::parseHwreg(int64_t &HwRegCode, int64_t &Offset, int64_t &Width) {
+bool AMDGPUAsmParser::parseHwreg(int64_t &HwRegCode, int64_t &Offset, int64_t &Width, bool &IsIdentifier) {
   if (Parser.getTok().getString() != "hwreg")
     return true;
   Parser.Lex();

     return true;
   Parser.Lex();
 
-  if (getLexer().isNot(AsmToken::Integer))
-    return true;
-  if (getParser().parseAbsoluteExpression(HwRegCode))
-    return true;
+  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);
+    Parser.Lex();
+  } else {
+    IsIdentifier = false;
+    if (getLexer().isNot(AsmToken::Integer))
+      return true;
+    if (getParser().parseAbsoluteExpression(HwRegCode))
+      return true;
+  }
 
   if (getLexer().is(AsmToken::RParen)) {
     Parser.Lex();

       break;
 
     case AsmToken::Identifier: {
-        int64_t HwRegCode = 0;
+        bool IsIdentifier = false;
+        int64_t HwRegCode = -1;
         int64_t Offset = 0; // default
         int64_t Width = 32; // default
-        if (parseHwreg(HwRegCode, Offset, Width))
+        if (parseHwreg(HwRegCode, Offset, Width, IsIdentifier))
           return MatchOperand_ParseFail;
         // HwRegCode (6) [5:0]
         // Offset (5) [10:6]
         // WidthMinusOne (5) [15:11]
         if (HwRegCode < 0 || HwRegCode > 63)
-          Error(S, "invalid code of hardware register: only 6-bit values are legal");
+          if (IsIdentifier)
+            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)
           Error(S, "invalid bit offset: only 5-bit values are legal");
         if (Width < 1 || Width > 32)

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp

   const unsigned Offset = (SImm16 >> 6) & 0x1f;
   const unsigned Width = ((SImm16 >> 11) & 0x1F) + 1;
 
-  if (Width == 32 && Offset == 0) {
-    O << "hwreg(" << HwRegCode << ')';
-  } else {
-    O << "hwreg(" << HwRegCode << ", " << Offset << ", " << Width << ')';
+  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 (! (Width == 32 && Offset == 0)) {
+    O << ", " << Offset << ", " << Width;
   }
+  O << ')';
 }
 
 #include "AMDGPUGenAsmWriter.inc"

test/CodeGen/AMDGPU/llvm.amdgcn.s.getreg.ll

 ; RUN: llc -mtriple=amdgcn--amdhsa -mcpu=fiji -verify-machineinstrs < %s | FileCheck %s
 
 ; FUNC-LABEL: {{^}}s_getreg_test:
-; CHECK: s_getreg_b32 s{{[0-9]+}}, hwreg(6, 8, 23)
+; CHECK: s_getreg_b32 s{{[0-9]+}}, hwreg(HW_REG_LDS_ALLOC, 8, 23)
 define void @s_getreg_test(i32 addrspace(1)* %out) { ; simm16=45574 for lds size.
   %lds_size_64dwords = call i32 @llvm.amdgcn.s.getreg(i32 45574) #0
   %lds_size_bytes = shl i32 %lds_size_64dwords, 8

test/MC/AMDGPU/sopk-err.s

 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
+
 s_setreg_b32  hwreg(3,32,32), s2
 // GCN: error: invalid bit offset: only 5-bit values are legal
 

test/MC/AMDGPU/sopk.s

 // SICI: s_cbranch_i_fork s[2:3], 0x6 ; encoding: [0x06,0x00,0x82,0xb8]
 // VI:   s_cbranch_i_fork s[2:3], 0x6 ; encoding: [0x06,0x00,0x02,0xb8]
 
+// raw number mapped to known HW register
 s_getreg_b32 s2, 0x6
-// SICI: s_getreg_b32 s2, hwreg(6, 0, 1) ; encoding: [0x06,0x00,0x02,0xb9]
-// VI:   s_getreg_b32 s2, hwreg(6, 0, 1) ; encoding: [0x06,0x00,0x82,0xb8]
+// SICI: s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x02,0xb9]
+// VI:   s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC, 0, 1) ; encoding: [0x06,0x00,0x82,0xb8]
 
-s_getreg_b32 s2, hwreg(5, 1, 31)
-// SICI: s_getreg_b32 s2, hwreg(5, 1, 31) ; encoding: [0x45,0xf0,0x02,0xb9]
-// VI:   s_getreg_b32 s2, hwreg(5, 1, 31) ; encoding: [0x45,0xf0,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]
+// VI:   s_getreg_b32 s2, hwreg(HW_REG_GPR_ALLOC, 1, 31) ; encoding: [0x45,0xf0,0x82,0xb8]
 
+// raw number mapped to unknown HW register
+s_getreg_b32 s2, hwreg(51, 1, 31)
+// SICI: s_getreg_b32 s2, hwreg(51, 1, 31) ; encoding: [0x73,0xf0,0x02,0xb9]
+// VI:   s_getreg_b32 s2, hwreg(51, 1, 31) ; encoding: [0x73,0xf0,0x82,0xb8]
+
+// raw number mapped to known HW register
 s_setreg_b32 0x6, s2
-// SICI: s_setreg_b32 hwreg(6, 0, 1), s2 ; encoding: [0x06,0x00,0x82,0xb9]
-// VI:   s_setreg_b32 hwreg(6, 0, 1), s2 ; encoding: [0x06,0x00,0x02,0xb9]
+// SICI: s_setreg_b32 hwreg(HW_REG_LDS_ALLOC, 0, 1), s2 ; encoding: [0x06,0x00,0x82,0xb9]
+// VI:   s_setreg_b32 hwreg(HW_REG_LDS_ALLOC, 0, 1), s2 ; encoding: [0x06,0x00,0x02,0xb9]
 
+// raw number mapped to known HW register, default offset/width
 s_setreg_b32 0xf803, s2
-// SICI: s_setreg_b32 hwreg(3), s2       ; encoding: [0x03,0xf8,0x82,0xb9]
-// VI:   s_setreg_b32 hwreg(3), s2       ; encoding: [0x03,0xf8,0x02,0xb9]
+// SICI: s_setreg_b32 hwreg(HW_REG_TRAPSTS), s2       ; encoding: [0x03,0xf8,0x82,0xb9]
+// VI:   s_setreg_b32 hwreg(HW_REG_TRAPSTS), s2       ; encoding: [0x03,0xf8,0x02,0xb9]
+
+// HW register identifier, default offset/width implied
+s_setreg_b32 hwreg(HW_REG_HW_ID), s2
+// SICI: s_setreg_b32 hwreg(HW_REG_HW_ID), s2       ; encoding: [0x04,0xf8,0x82,0xb9]
+// VI:   s_setreg_b32 hwreg(HW_REG_HW_ID), s2       ; encoding: [0x04,0xf8,0x02,0xb9]
 
-s_setreg_b32 hwreg(4), s2
-// SICI: s_setreg_b32 hwreg(4), s2       ; encoding: [0x04,0xf8,0x82,0xb9]
-// VI:   s_setreg_b32 hwreg(4), s2       ; encoding: [0x04,0xf8,0x02,0xb9]
+// HW register identifier, non-default offset/width
+s_setreg_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), s2
+// SICI: s_setreg_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), s2       ; encoding: [0x45,0xf0,0x82,0xb9]
+// VI:   s_setreg_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), s2       ; encoding: [0x45,0xf0,0x02,0xb9]
 
+// HW register code, non-default offset/width
 s_setreg_b32 hwreg(5, 1, 31), s2
-// SICI: s_setreg_b32 hwreg(5, 1, 31), s2       ; encoding: [0x45,0xf0,0x82,0xb9]
-// VI:   s_setreg_b32 hwreg(5, 1, 31), s2       ; encoding: [0x45,0xf0,0x02,0xb9]
+// SICI: s_setreg_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), s2       ; encoding: [0x45,0xf0,0x82,0xb9]
+// VI:   s_setreg_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), s2       ; encoding: [0x45,0xf0,0x02,0xb9]
 
+// raw number mapped to known HW register
 s_setreg_imm32_b32 0x6, 0xff
-// SICI: s_setreg_imm32_b32 hwreg(6, 0, 1), 0xff ; encoding: [0x06,0x00,0x80,0xba,0xff,0x00,0x00,0x00]
-// VI:   s_setreg_imm32_b32 hwreg(6, 0, 1), 0xff ; encoding: [0x06,0x00,0x00,0xba,0xff,0x00,0x00,0x00]
+// SICI: s_setreg_imm32_b32 hwreg(HW_REG_LDS_ALLOC, 0, 1), 0xff ; encoding: [0x06,0x00,0x80,0xba,0xff,0x00,0x00,0x00]
+// VI:   s_setreg_imm32_b32 hwreg(HW_REG_LDS_ALLOC, 0, 1), 0xff ; encoding: [0x06,0x00,0x00,0xba,0xff,0x00,0x00,0x00]
 
-s_setreg_imm32_b32 hwreg(5, 1, 31), 0xff
-// SICI: s_setreg_imm32_b32 hwreg(5, 1, 31), 0xff ; encoding: [0x45,0xf0,0x80,0xba,0xff,0x00,0x00,0x00]
-// VI:   s_setreg_imm32_b32 hwreg(5, 1, 31), 0xff ; encoding: [0x45,0xf0,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]
+// VI:   s_setreg_imm32_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), 0xff ; encoding: [0x45,0xf0,0x00,0xba,0xff,0x00,0x00,0x00]

test/MC/Disassembler/AMDGPU/sopk_vi.txt

 # VI:   s_cbranch_i_fork s[2:3], 0x6 ; encoding: [0x06,0x00,0x02,0xb8]
 0x06 0x00 0x02 0xb8
 
-# VI:   s_getreg_b32 s2, hwreg(6) ; encoding: [0x06,0xf8,0x82,0xb8]
+# VI:   s_getreg_b32 s2, hwreg(HW_REG_LDS_ALLOC) ; encoding: [0x06,0xf8,0x82,0xb8]
 0x06 0xf8 0x82 0xb8
 
-# VI:   s_setreg_b32 hwreg(6, 0, 1), s2 ; encoding: [0x06,0x00,0x02,0xb9]
+# VI:   s_getreg_b32 s2, hwreg(51) ; encoding: [0x33,0xf8,0x82,0xb8]
+0x33,0xf8,0x82,0xb8
+
+# VI:   s_getreg_b32 s2, hwreg(51, 1, 31) ; encoding: [0x73,0xf0,0x82,0xb8]
+0x73,0xf0,0x82,0xb8
+
+# VI:   s_setreg_b32 hwreg(HW_REG_LDS_ALLOC, 0, 1), s2 ; encoding: [0x06,0x00,0x02,0xb9]
 0x06 0x00 0x02 0xb9
 
-# VI:   s_setreg_imm32_b32 hwreg(5, 1, 31), 0xff ; encoding: [0x45,0xf0,0x00,0xba,0xff,0x00,0x00,0x00]
+# VI:   s_setreg_imm32_b32 hwreg(HW_REG_GPR_ALLOC, 1, 31), 0xff ; encoding: [0x45,0xf0,0x00,0xba,0xff,0x00,0x00,0x00]
 0x45 0xf0 0x00 0xba 0xff 0x00 0x00 0x00