[llvm] 5977dfb - [AMDGPU][MC][NFC] Refactored custom operands handling

Dmitry Preobrazhensky via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 16 06:05:16 PDT 2022


Author: Dmitry Preobrazhensky
Date: 2022-03-16T16:04:55+03:00
New Revision: 5977dfba64099e224cba12f580b6867e7a3d149a

URL: https://github.com/llvm/llvm-project/commit/5977dfba64099e224cba12f580b6867e7a3d149a
DIFF: https://github.com/llvm/llvm-project/commit/5977dfba64099e224cba12f580b6867e7a3d149a.diff

LOG: [AMDGPU][MC][NFC] Refactored custom operands handling

The original design of custom operands support assumed that most GPUs
have the same or very similar operand names end encodings. This is
no longer the case. As a result the support code becomes over-complicated
and difficult to maintain.

This change implements a different design with the following benefits:

- support of aliases;
- support of operands with overlapped encodings;
- identification of defined but unsupported operands.

Differential Revision: https://reviews.llvm.org/D121696

Added: 
    

Modified: 
    llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
    llvm/lib/Target/AMDGPU/SIDefines.h
    llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.cpp
    llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.h
    llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
    llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index ae2ab2aa1f547..4e5abba7b58f6 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -1542,7 +1542,6 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
     int64_t Id;
     bool IsSymbolic = false;
     bool IsDefined = false;
-    StringRef Name;
 
     OperandInfoTy(int64_t Id_) : Id(Id_) {}
   };
@@ -6258,9 +6257,8 @@ AMDGPUAsmParser::parseHwregBody(OperandInfoTy &HwReg,
   // The register may be specified by name or using a numeric code
   HwReg.Loc = getLoc();
   if (isToken(AsmToken::Identifier) &&
-      (HwReg.Id = getHwregId(getTokenStr(), getSTI())) >= 0) {
+      (HwReg.Id = getHwregId(getTokenStr(), getSTI())) != OPR_ID_UNKNOWN) {
     HwReg.IsSymbolic = true;
-    HwReg.Name = getTokenStr();
     lex(); // skip register name
   } else if (!parseExpr(HwReg.Id, "a register name")) {
     return false;
@@ -6292,16 +6290,18 @@ AMDGPUAsmParser::validateHwreg(const OperandInfoTy &HwReg,
 
   using namespace llvm::AMDGPU::Hwreg;
 
-  if (HwReg.IsSymbolic &&
-      !isValidHwreg(HwReg.Id, getSTI(), HwReg.Name)) {
-    Error(HwReg.Loc,
-          "specified hardware register is not supported on this GPU");
-    return false;
-  }
-  if (!isValidHwreg(HwReg.Id)) {
-    Error(HwReg.Loc,
-          "invalid code of hardware register: only 6-bit values are legal");
-    return false;
+  if (HwReg.IsSymbolic) {
+    if (HwReg.Id == OPR_ID_UNSUPPORTED) {
+      Error(HwReg.Loc,
+            "specified hardware register is not supported on this GPU");
+      return false;
+    }
+  } else {
+    if (!isValidHwreg(HwReg.Id)) {
+      Error(HwReg.Loc,
+            "invalid code of hardware register: only 6-bit values are legal");
+      return false;
+    }
   }
   if (!isValidHwregOffset(Offset.Id)) {
     Error(Offset.Loc, "invalid bit offset: only 5-bit values are legal");
@@ -6323,7 +6323,7 @@ AMDGPUAsmParser::parseHwreg(OperandVector &Operands) {
   SMLoc Loc = getLoc();
 
   if (trySkipId("hwreg", AsmToken::LParen)) {
-    OperandInfoTy HwReg(ID_UNKNOWN_);
+    OperandInfoTy HwReg(OPR_ID_UNKNOWN);
     OperandInfoTy Offset(OFFSET_DEFAULT_);
     OperandInfoTy Width(WIDTH_DEFAULT_);
     if (parseHwregBody(HwReg, Offset, Width) &&

diff  --git a/llvm/lib/Target/AMDGPU/SIDefines.h b/llvm/lib/Target/AMDGPU/SIDefines.h
index 56371514e5e55..61814daa2a9b0 100644
--- a/llvm/lib/Target/AMDGPU/SIDefines.h
+++ b/llvm/lib/Target/AMDGPU/SIDefines.h
@@ -363,8 +363,6 @@ enum StreamId : unsigned { // Stream ID, (2) [9:8].
 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_MODE = 1,
   ID_STATUS = 2,
   ID_TRAPSTS = 3,
@@ -373,28 +371,23 @@ enum Id { // HwRegCode, (6) [5:0]
   ID_LDS_ALLOC = 6,
   ID_IB_STS = 7,
   ID_MEM_BASES = 15,
-  ID_SYMBOLIC_FIRST_GFX9_ = ID_MEM_BASES,
   ID_TBA_LO = 16,
   ID_TBA_HI = 17,
   ID_TMA_LO = 18,
   ID_TMA_HI = 19,
   ID_XCC_ID = 20,
-  ID_SYMBOLIC_FIRST_GFX940_ = ID_XCC_ID,
   ID_SQ_PERF_SNAPSHOT_DATA = 21,
   ID_SQ_PERF_SNAPSHOT_DATA1 = 22,
   ID_SQ_PERF_SNAPSHOT_PC_LO = 23,
   ID_SQ_PERF_SNAPSHOT_PC_HI = 24,
-  ID_SYMBOLIC_LAST_GFX940_ = ID_SQ_PERF_SNAPSHOT_PC_HI + 1,
   ID_FLAT_SCR_LO = 20,
-  ID_SYMBOLIC_FIRST_GFX10_ = ID_FLAT_SCR_LO,
   ID_FLAT_SCR_HI = 21,
   ID_XNACK_MASK = 22,
   ID_HW_ID1 = 23,
   ID_HW_ID2 = 24,
   ID_POPS_PACKER = 25,
   ID_SHADER_CYCLES = 29,
-  ID_SYMBOLIC_FIRST_GFX1030_ = ID_SHADER_CYCLES,
-  ID_SYMBOLIC_LAST_ = 30,
+
   ID_SHIFT_ = 0,
   ID_WIDTH_ = 6,
   ID_MASK_ = (((1 << ID_WIDTH_) - 1) << ID_SHIFT_)

diff  --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.cpp
index 44f25d82da1e4..228023ef8d500 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.cpp
@@ -6,10 +6,9 @@
 //
 //===----------------------------------------------------------------------===//
 #include "AMDGPUAsmUtils.h"
+#include "AMDGPUBaseInfo.h"
 #include "SIDefines.h"
 
-#include "llvm/ADT/StringRef.h"
-
 namespace llvm {
 namespace AMDGPU {
 namespace SendMsg {
@@ -54,49 +53,62 @@ const char *const OpGsSymbolic[OP_GS_LAST_] = {
 
 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",
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr,
-  "HW_REG_SH_MEM_BASES",
-  "HW_REG_TBA_LO",
-  "HW_REG_TBA_HI",
-  "HW_REG_TMA_LO",
-  "HW_REG_TMA_HI",
-  "HW_REG_FLAT_SCR_LO",
-  "HW_REG_FLAT_SCR_HI",
-  "HW_REG_XNACK_MASK",
-  "HW_REG_HW_ID1",
-  "HW_REG_HW_ID2",
-  "HW_REG_POPS_PACKER",
-  nullptr,
-  nullptr,
-  nullptr,
-  "HW_REG_SHADER_CYCLES"
+// Disable lint checking for this block since it makes the table unreadable.
+// NOLINTBEGIN
+const CustomOperand<const MCSubtargetInfo &> Opr[] = {
+  {},
+  {{"HW_REG_MODE"},          ID_MODE},
+  {{"HW_REG_STATUS"},        ID_STATUS},
+  {{"HW_REG_TRAPSTS"},       ID_TRAPSTS},
+  {{"HW_REG_HW_ID"},         ID_HW_ID,
+                                     [](const MCSubtargetInfo &STI) {
+                                       return isSI(STI) || isCI(STI) ||
+                                              isVI(STI) || isGFX9(STI);
+                                     }},
+  {{"HW_REG_GPR_ALLOC"},     ID_GPR_ALLOC},
+  {{"HW_REG_LDS_ALLOC"},     ID_LDS_ALLOC},
+  {{"HW_REG_IB_STS"},        ID_IB_STS},
+  {},
+  {},
+  {},
+  {},
+  {},
+  {},
+  {},
+  {{"HW_REG_SH_MEM_BASES"},  ID_MEM_BASES,   isGFX9Plus},
+  {{"HW_REG_TBA_LO"},        ID_TBA_LO,      isGFX9_GFX10},
+  {{"HW_REG_TBA_HI"},        ID_TBA_HI,      isGFX9_GFX10},
+  {{"HW_REG_TMA_LO"},        ID_TMA_LO,      isGFX9_GFX10},
+  {{"HW_REG_TMA_HI"},        ID_TMA_HI,      isGFX9_GFX10},
+  {{"HW_REG_FLAT_SCR_LO"},   ID_FLAT_SCR_LO, isGFX10Plus},
+  {{"HW_REG_FLAT_SCR_HI"},   ID_FLAT_SCR_HI, isGFX10Plus},
+  {{"HW_REG_XNACK_MASK"},    ID_XNACK_MASK,
+                                     [](const MCSubtargetInfo &STI) {
+                                       return isGFX10(STI) &&
+                                              !AMDGPU::isGFX10_BEncoding(STI);
+                                     }},
+  {{"HW_REG_HW_ID1"},        ID_HW_ID1,      isGFX10Plus},
+  {{"HW_REG_HW_ID2"},        ID_HW_ID2,      isGFX10Plus},
+  {{"HW_REG_POPS_PACKER"},   ID_POPS_PACKER, isGFX10},
+  {},
+  {},
+  {},
+  {{"HW_REG_SHADER_CYCLES"}, ID_SHADER_CYCLES, isGFX10_BEncoding},
+
+  // GFX940 specific registers
+  {{"HW_REG_XCC_ID"},                 ID_XCC_ID,                 isGFX940},
+  {{"HW_REG_SQ_PERF_SNAPSHOT_DATA"},  ID_SQ_PERF_SNAPSHOT_DATA,  isGFX940},
+  {{"HW_REG_SQ_PERF_SNAPSHOT_DATA1"}, ID_SQ_PERF_SNAPSHOT_DATA1, isGFX940},
+  {{"HW_REG_SQ_PERF_SNAPSHOT_PC_LO"}, ID_SQ_PERF_SNAPSHOT_PC_LO, isGFX940},
+  {{"HW_REG_SQ_PERF_SNAPSHOT_PC_HI"}, ID_SQ_PERF_SNAPSHOT_PC_HI, isGFX940},
+
+  // Aliases
+  {{"HW_REG_HW_ID"},                  ID_HW_ID1,                 isGFX10},
 };
+// NOLINTEND
 
-// This is gfx940 specific portion from ID_SYMBOLIC_FIRST_GFX940_ to
-// ID_SYMBOLIC_LAST_GFX940_
-const char* const IdSymbolicGFX940Specific[] = {
-  "HW_REG_XCC_ID",
-  "HW_REG_SQ_PERF_SNAPSHOT_DATA",
-  "HW_REG_SQ_PERF_SNAPSHOT_DATA1",
-  "HW_REG_SQ_PERF_SNAPSHOT_PC_LO",
-  "HW_REG_SQ_PERF_SNAPSHOT_PC_HI"
-};
+const int OPR_SIZE = static_cast<int>(
+    sizeof(Opr) / sizeof(CustomOperand<const MCSubtargetInfo &>));
 
 } // namespace Hwreg
 

diff  --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.h b/llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.h
index 2b46bb0782b65..d08035b77ff47 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.h
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUAsmUtils.h
@@ -11,12 +11,24 @@
 
 #include "SIDefines.h"
 
+#include "llvm/ADT/StringRef.h"
+
 namespace llvm {
 
 class StringLiteral;
+class MCSubtargetInfo;
 
 namespace AMDGPU {
 
+const int OPR_ID_UNKNOWN = -1;
+const int OPR_ID_UNSUPPORTED = -2;
+
+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_];
@@ -27,8 +39,8 @@ extern const char *const OpGsSymbolic[OP_GS_LAST_];
 
 namespace Hwreg { // Symbolic names for the hwreg(...) syntax.
 
-extern const char* const IdSymbolic[];
-extern const char* const IdSymbolicGFX940Specific[];
+extern const CustomOperand<const MCSubtargetInfo &> Opr[];
+extern const int OPR_SIZE;
 
 } // namespace Hwreg
 

diff  --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
index 29eba61cafa0e..14d8b1db48090 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -1039,74 +1039,70 @@ unsigned encodeWaitcnt(const IsaVersion &Version, const Waitcnt &Decoded) {
 }
 
 //===----------------------------------------------------------------------===//
-// hwreg
+// Custom Operands.
+//
+// A table of custom operands shall describe "primary" operand names
+// first followed by aliases if any. It is not required but recommended
+// to arrange operands so that operand encoding match operand position
+// in the table. This will make disassembly a bit more efficient.
+// Unused slots in the table shall have an empty name.
+//
 //===----------------------------------------------------------------------===//
 
-namespace Hwreg {
-
-static const char* getHwregName(int64_t Id, const MCSubtargetInfo &STI) {
-  if (isGFX940(STI) && Id >= ID_SYMBOLIC_FIRST_GFX940_ &&
-      Id < ID_SYMBOLIC_LAST_GFX940_)
-    return IdSymbolicGFX940Specific[Id - ID_SYMBOLIC_FIRST_GFX940_];
-  return (Id < ID_SYMBOLIC_LAST_) ? IdSymbolic[Id] : nullptr;
-}
-
-int64_t getHwregId(const StringRef Name, const MCSubtargetInfo &STI) {
-  if (isGFX10(STI) && Name == "HW_REG_HW_ID") // An alias
-    return ID_HW_ID1;
-  for (int Id = ID_SYMBOLIC_FIRST_; Id < ID_SYMBOLIC_LAST_; ++Id) {
-    if (IdSymbolic[Id] && Name == IdSymbolic[Id])
-      return Id;
-    // These are all defined, however may be invalid for subtarget and need
-    // further validation.
-    if (Id >= ID_SYMBOLIC_FIRST_GFX940_ && Id < ID_SYMBOLIC_LAST_GFX940_ &&
-        Name == IdSymbolicGFX940Specific[Id - ID_SYMBOLIC_FIRST_GFX940_])
-      return Id;
-  }
-  return ID_UNKNOWN_;
-}
-
-static unsigned getLastSymbolicHwreg(const MCSubtargetInfo &STI) {
-  if (isSI(STI) || isCI(STI) || isVI(STI))
-    return ID_SYMBOLIC_FIRST_GFX9_;
-  else if (isGFX940(STI))
-    return ID_SYMBOLIC_LAST_GFX940_;
-  else if (isGFX9(STI))
-    return ID_SYMBOLIC_FIRST_GFX10_;
-  else if (isGFX10(STI) && !isGFX10_BEncoding(STI))
-    return ID_SYMBOLIC_FIRST_GFX1030_;
-  else
-    return ID_SYMBOLIC_LAST_;
-}
-
-bool isValidHwreg(int64_t Id, const MCSubtargetInfo &STI, StringRef Name) {
-  if (isGFX10(STI) && Name == "HW_REG_HW_ID") // An alias
-    return true;
-
-  const char *HWRegName = getHwregName(Id, STI);
-  if (!HWRegName || !Name.startswith(HWRegName))
-    return false;
-
-  if (isGFX10Plus(STI)) {
-    switch (Id) {
-    case ID_HW_ID1:
-    case ID_HW_ID2:
-      return true;
-    case ID_XNACK_MASK:
-      return !AMDGPU::isGFX10_BEncoding(STI);
-    default:
-      break;
+template <class T>
+static bool isValidOpr(int Idx, const CustomOperand<T> OpInfo[], int OpInfoSize,
+                       T Context) {
+  return 0 <= Idx && Idx < OpInfoSize && !OpInfo[Idx].Name.empty() &&
+         (!OpInfo[Idx].Cond || OpInfo[Idx].Cond(Context));
+}
+
+template <class T>
+static int getOprIdx(std::function<bool(const CustomOperand<T> &)> Test,
+                     const CustomOperand<T> OpInfo[], int OpInfoSize,
+                     T Context) {
+  int InvalidIdx = OPR_ID_UNKNOWN;
+  for (int Idx = 0; Idx < OpInfoSize; ++Idx) {
+    if (Test(OpInfo[Idx])) {
+      if (!OpInfo[Idx].Cond || OpInfo[Idx].Cond(Context))
+        return Idx;
+      InvalidIdx = OPR_ID_UNSUPPORTED;
     }
   }
+  return InvalidIdx;
+}
+
+template <class T>
+static int getOprIdx(const StringRef Name, const CustomOperand<T> OpInfo[],
+                     int OpInfoSize, T Context) {
+  auto Test = [=](const CustomOperand<T> &Op) { return Op.Name == Name; };
+  return getOprIdx<T>(Test, OpInfo, OpInfoSize, Context);
+}
+
+template <class T>
+static int getOprIdx(int Id, const CustomOperand<T> OpInfo[], int OpInfoSize,
+                     T Context, bool QuickCheck = true) {
+  auto Test = [=](const CustomOperand<T> &Op) {
+    return Op.Encoding == Id && !Op.Name.empty();
+  };
+  // This is an optimization that should work in most cases.
+  // As a side effect, it may cause selection of an alias
+  // instead of a primary operand name in case of sparse tables.
+  if (QuickCheck && isValidOpr<T>(Id, OpInfo, OpInfoSize, Context) &&
+      OpInfo[Id].Encoding == Id) {
+    return Id;
+  }
+  return getOprIdx<T>(Test, OpInfo, OpInfoSize, Context);
+}
 
-  if (ID_SYMBOLIC_FIRST_ > Id || Id >= getLastSymbolicHwreg(STI))
-    return false;
+//===----------------------------------------------------------------------===//
+// hwreg
+//===----------------------------------------------------------------------===//
 
-  if (isGFX940(STI) && Id >= ID_SYMBOLIC_FIRST_GFX10_ &&
-      Id < ID_SYMBOLIC_FIRST_GFX940_)
-    return false;
+namespace Hwreg {
 
-  return true;
+int64_t getHwregId(const StringRef Name, const MCSubtargetInfo &STI) {
+  int Idx = getOprIdx<const MCSubtargetInfo &>(Name, Opr, OPR_SIZE, STI);
+  return (Idx < 0) ? Idx : Opr[Idx].Encoding;
 }
 
 bool isValidHwreg(int64_t Id) {
@@ -1128,8 +1124,8 @@ uint64_t encodeHwreg(uint64_t Id, uint64_t Offset, uint64_t Width) {
 }
 
 StringRef getHwreg(unsigned Id, const MCSubtargetInfo &STI) {
-  const char *HWRegName = getHwregName(Id, STI);
-  return (HWRegName && isValidHwreg(Id, STI, HWRegName)) ? HWRegName : "";
+  int Idx = getOprIdx<const MCSubtargetInfo &>(Id, Opr, OPR_SIZE, STI);
+  return (Idx < 0) ? "" : Opr[Idx].Name;
 }
 
 void decodeHwreg(unsigned Val, unsigned &Id, unsigned &Offset, unsigned &Width) {
@@ -1537,6 +1533,10 @@ 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);
 }

diff  --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
index 0677c23d9b82d..c925f003c9672 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -612,9 +612,6 @@ namespace Hwreg {
 LLVM_READONLY
 int64_t getHwregId(const StringRef Name, const MCSubtargetInfo &STI);
 
-LLVM_READONLY
-bool isValidHwreg(int64_t Id, const MCSubtargetInfo &STI, StringRef Name);
-
 LLVM_READNONE
 bool isValidHwreg(int64_t Id);
 
@@ -775,6 +772,7 @@ bool isSI(const MCSubtargetInfo &STI);
 bool isCI(const MCSubtargetInfo &STI);
 bool isVI(const MCSubtargetInfo &STI);
 bool isGFX9(const MCSubtargetInfo &STI);
+bool isGFX9_GFX10(const MCSubtargetInfo &STI);
 bool isGFX9Plus(const MCSubtargetInfo &STI);
 bool isGFX10(const MCSubtargetInfo &STI);
 bool isGFX10Plus(const MCSubtargetInfo &STI);


        


More information about the llvm-commits mailing list