[llvm] [AMDGPU] Add parseStringOrIntWithPrefix helper in asm parser (PR #102213)

Stanislav Mekhanoshin via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 6 13:19:55 PDT 2024


https://github.com/rampitec created https://github.com/llvm/llvm-project/pull/102213

When we have a modifier with a value (like dst_sel:DWORD for example) we only accept symbolic values. SP3 allows to use numberic constants as well. Adding a helper function to allow both.

Besides the compatibility it is easier to use.

>From f14337e0d5dc3d0d642bf334f928c89afcc6a78d Mon Sep 17 00:00:00 2001
From: Stanislav Mekhanoshin <Stanislav.Mekhanoshin at amd.com>
Date: Tue, 6 Aug 2024 13:15:35 -0700
Subject: [PATCH] [AMDGPU] Add parseStringOrIntWithPrefix helper in asm parser

When we have a modifier with a value (like dst_sel:DWORD for
example) we only accept symbolic values. SP3 allows to use
numberic constants as well. Adding a helper function to allow
both.

Besides the compatibility it is easier to use.
---
 .../AMDGPU/AsmParser/AMDGPUAsmParser.cpp      | 127 +++++++++---------
 llvm/test/MC/AMDGPU/gfx10_asm_vop1.s          |   9 ++
 llvm/test/MC/AMDGPU/gfx10_err_pos.s           |  32 ++++-
 llvm/test/MC/AMDGPU/gfx12_asm_smem.s          |  12 ++
 4 files changed, 108 insertions(+), 72 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 26a839a95df96b..c8b594ffbc6452 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -1618,6 +1618,14 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
   ParseStatus parseTH(OperandVector &Operands, int64_t &TH);
   ParseStatus parseStringWithPrefix(StringRef Prefix, StringRef &Value,
                                     SMLoc &StringLoc);
+  ParseStatus parseStringOrIntWithPrefix(OperandVector &Operands,
+                                         StringRef Name,
+                                         ArrayRef<const char *> Ids,
+                                         int64_t &IntVal);
+  ParseStatus parseStringOrIntWithPrefix(OperandVector &Operands,
+                                         StringRef Name,
+                                         ArrayRef<const char *> Ids,
+                                         AMDGPUOperand::ImmTy Type);
 
   bool isModifier();
   bool isOperandModifier(const AsmToken &Token, const AsmToken &NextToken) const;
@@ -6633,27 +6641,17 @@ ParseStatus AMDGPUAsmParser::parseCPol(OperandVector &Operands) {
 
 ParseStatus AMDGPUAsmParser::parseScope(OperandVector &Operands,
                                         int64_t &Scope) {
-  Scope = AMDGPU::CPol::SCOPE_CU; // default;
+  static const unsigned Scopes[] = {CPol::SCOPE_CU, CPol::SCOPE_SE,
+                                    CPol::SCOPE_DEV, CPol::SCOPE_SYS};
 
-  StringRef Value;
-  SMLoc StringLoc;
-  ParseStatus Res;
-
-  Res = parseStringWithPrefix("scope", Value, StringLoc);
-  if (!Res.isSuccess())
-    return Res;
-
-  Scope = StringSwitch<int64_t>(Value)
-              .Case("SCOPE_CU", AMDGPU::CPol::SCOPE_CU)
-              .Case("SCOPE_SE", AMDGPU::CPol::SCOPE_SE)
-              .Case("SCOPE_DEV", AMDGPU::CPol::SCOPE_DEV)
-              .Case("SCOPE_SYS", AMDGPU::CPol::SCOPE_SYS)
-              .Default(0xffffffff);
+  ParseStatus Res = parseStringOrIntWithPrefix(
+      Operands, "scope", {"SCOPE_CU", "SCOPE_SE", "SCOPE_DEV", "SCOPE_SYS"},
+      Scope);
 
-  if (Scope == 0xffffffff)
-    return Error(StringLoc, "invalid scope value");
+  if (Res.isSuccess())
+    Scope = Scopes[Scope];
 
-  return ParseStatus::Success;
+  return Res;
 }
 
 ParseStatus AMDGPUAsmParser::parseTH(OperandVector &Operands, int64_t &TH) {
@@ -6742,6 +6740,44 @@ ParseStatus AMDGPUAsmParser::parseStringWithPrefix(StringRef Prefix,
                                                   : ParseStatus::Failure;
 }
 
+ParseStatus AMDGPUAsmParser::parseStringOrIntWithPrefix(
+    OperandVector &Operands, StringRef Name, ArrayRef<const char *> Ids,
+    int64_t &IntVal) {
+  if (!trySkipId(Name, AsmToken::Colon))
+    return ParseStatus::NoMatch;
+
+  SMLoc StringLoc = getLoc();
+
+  StringRef Value;
+  if (isToken(AsmToken::Identifier)) {
+    Value = getTokenStr();
+    lex();
+
+    for (IntVal = 0; IntVal < (int64_t)Ids.size(); ++IntVal)
+      if (Value == Ids[IntVal])
+        break;
+  } else if (!parseExpr(IntVal))
+    return ParseStatus::Failure;
+
+  if (IntVal < 0 || IntVal >= (int64_t)Ids.size())
+    return Error(StringLoc, "invalid " + Twine(Name) + " value");
+
+  return ParseStatus::Success;
+}
+
+ParseStatus AMDGPUAsmParser::parseStringOrIntWithPrefix(
+    OperandVector &Operands, StringRef Name, ArrayRef<const char *> Ids,
+    AMDGPUOperand::ImmTy Type) {
+  SMLoc S = getLoc();
+  int64_t IntVal;
+
+  ParseStatus Res = parseStringOrIntWithPrefix(Operands, Name, Ids, IntVal);
+  if (Res.isSuccess())
+    Operands.push_back(AMDGPUOperand::CreateImm(this, IntVal, S, Type));
+
+  return Res;
+}
+
 //===----------------------------------------------------------------------===//
 // MTBUF format
 //===----------------------------------------------------------------------===//
@@ -9396,57 +9432,16 @@ void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands, bool I
 ParseStatus AMDGPUAsmParser::parseSDWASel(OperandVector &Operands,
                                           StringRef Prefix,
                                           AMDGPUOperand::ImmTy Type) {
-  using namespace llvm::AMDGPU::SDWA;
-
-  SMLoc S = getLoc();
-  StringRef Value;
-
-  SMLoc StringLoc;
-  ParseStatus Res = parseStringWithPrefix(Prefix, Value, StringLoc);
-  if (!Res.isSuccess())
-    return Res;
-
-  int64_t Int;
-  Int = StringSwitch<int64_t>(Value)
-        .Case("BYTE_0", SdwaSel::BYTE_0)
-        .Case("BYTE_1", SdwaSel::BYTE_1)
-        .Case("BYTE_2", SdwaSel::BYTE_2)
-        .Case("BYTE_3", SdwaSel::BYTE_3)
-        .Case("WORD_0", SdwaSel::WORD_0)
-        .Case("WORD_1", SdwaSel::WORD_1)
-        .Case("DWORD", SdwaSel::DWORD)
-        .Default(0xffffffff);
-
-  if (Int == 0xffffffff)
-    return Error(StringLoc, "invalid " + Twine(Prefix) + " value");
-
-  Operands.push_back(AMDGPUOperand::CreateImm(this, Int, S, Type));
-  return ParseStatus::Success;
+  return parseStringOrIntWithPrefix(
+      Operands, Prefix,
+      {"BYTE_0", "BYTE_1", "BYTE_2", "BYTE_3", "WORD_0", "WORD_1", "DWORD"},
+      Type);
 }
 
 ParseStatus AMDGPUAsmParser::parseSDWADstUnused(OperandVector &Operands) {
-  using namespace llvm::AMDGPU::SDWA;
-
-  SMLoc S = getLoc();
-  StringRef Value;
-
-  SMLoc StringLoc;
-  ParseStatus Res = parseStringWithPrefix("dst_unused", Value, StringLoc);
-  if (!Res.isSuccess())
-    return Res;
-
-  int64_t Int;
-  Int = StringSwitch<int64_t>(Value)
-        .Case("UNUSED_PAD", DstUnused::UNUSED_PAD)
-        .Case("UNUSED_SEXT", DstUnused::UNUSED_SEXT)
-        .Case("UNUSED_PRESERVE", DstUnused::UNUSED_PRESERVE)
-        .Default(0xffffffff);
-
-  if (Int == 0xffffffff)
-    return Error(StringLoc, "invalid dst_unused value");
-
-  Operands.push_back(AMDGPUOperand::CreateImm(this, Int, S, AMDGPUOperand::ImmTySDWADstUnused));
-  return ParseStatus::Success;
+  return parseStringOrIntWithPrefix(
+      Operands, "dst_unused", {"UNUSED_PAD", "UNUSED_SEXT", "UNUSED_PRESERVE"},
+      AMDGPUOperand::ImmTySDWADstUnused);
 }
 
 void AMDGPUAsmParser::cvtSdwaVOP1(MCInst &Inst, const OperandVector &Operands) {
diff --git a/llvm/test/MC/AMDGPU/gfx10_asm_vop1.s b/llvm/test/MC/AMDGPU/gfx10_asm_vop1.s
index 3cc25501ff7c03..80ec5934ad9937 100644
--- a/llvm/test/MC/AMDGPU/gfx10_asm_vop1.s
+++ b/llvm/test/MC/AMDGPU/gfx10_asm_vop1.s
@@ -163,6 +163,15 @@ v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:UNUSED_SEXT src0_sel:DWORD
 v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:UNUSED_PRESERVE src0_sel:DWORD
 // GFX10: encoding: [0xf9,0x02,0x0a,0x7e,0x01,0x16,0x06,0x00]
 
+v_mov_b32_sdwa v5, v1 dst_sel:WORD_1 dst_unused:0 src0_sel:06
+// GFX10: encoding: [0xf9,0x02,0x0a,0x7e,0x01,0x05,0x06,0x00]
+
+v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:1 src0_sel:0x6
+// GFX10: encoding: [0xf9,0x02,0x0a,0x7e,0x01,0x0e,0x06,0x00]
+
+v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:2 src0_sel:6
+// GFX10: encoding: [0xf9,0x02,0x0a,0x7e,0x01,0x16,0x06,0x00]
+
 v_mov_b32_sdwa v5, v1 dst_sel:DWORD src0_sel:DWORD
 // GFX10: encoding: [0xf9,0x02,0x0a,0x7e,0x01,0x16,0x06,0x00]
 
diff --git a/llvm/test/MC/AMDGPU/gfx10_err_pos.s b/llvm/test/MC/AMDGPU/gfx10_err_pos.s
index 4db454f0ced5aa..d99da6e0cb8695 100644
--- a/llvm/test/MC/AMDGPU/gfx10_err_pos.s
+++ b/llvm/test/MC/AMDGPU/gfx10_err_pos.s
@@ -484,23 +484,43 @@ v_mov_b32_sdwa v1, sext(u)
 // CHECK-NEXT:{{^}}                        ^
 
 //==============================================================================
-// expected an identifier
+// expected a valid identifier or number in a valid range
 
 v_mov_b32_sdwa v5, v1 dst_sel:
-// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: expected an identifier
+// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: unknown token in expression
 // CHECK-NEXT:{{^}}v_mov_b32_sdwa v5, v1 dst_sel:
 // CHECK-NEXT:{{^}}                              ^
 
-v_mov_b32_sdwa v5, v1 dst_sel:0
-// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: expected an identifier
-// CHECK-NEXT:{{^}}v_mov_b32_sdwa v5, v1 dst_sel:0
+v_mov_b32_sdwa v5, v1 dst_sel:0a
+// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: invalid operand for instruction
+// CHECK-NEXT:{{^}}v_mov_b32_sdwa v5, v1 dst_sel:0a
+// CHECK-NEXT:{{^}}                               ^
+
+v_mov_b32_sdwa v5, v1 dst_sel:BYTE_1x
+// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: invalid dst_sel value
+// CHECK-NEXT:{{^}}v_mov_b32_sdwa v5, v1 dst_sel:BYTE_1
 // CHECK-NEXT:{{^}}                              ^
 
 v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:[UNUSED_PAD]
-// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: expected an identifier
+// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: expected absolute expression
 // CHECK-NEXT:{{^}}v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:[UNUSED_PAD]
 // CHECK-NEXT:{{^}}                                               ^
 
+v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:XXX
+// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: invalid dst_unused value
+// CHECK-NEXT:{{^}}v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:XXX
+// CHECK-NEXT:{{^}}                                               ^
+
+v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:3
+// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: invalid dst_unused value
+// CHECK-NEXT:{{^}}v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:3
+// CHECK-NEXT:{{^}}                                               ^
+
+v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:-1
+// CHECK: :[[@LINE-1]]:{{[0-9]+}}: error: invalid dst_unused value
+// CHECK-NEXT:{{^}}v_mov_b32_sdwa v5, v1 dst_sel:DWORD dst_unused:-1
+// CHECK-NEXT:{{^}}                                               ^
+
 //==============================================================================
 // expected an opening square bracket
 
diff --git a/llvm/test/MC/AMDGPU/gfx12_asm_smem.s b/llvm/test/MC/AMDGPU/gfx12_asm_smem.s
index a64f35337d0e5a..668f767661f682 100644
--- a/llvm/test/MC/AMDGPU/gfx12_asm_smem.s
+++ b/llvm/test/MC/AMDGPU/gfx12_asm_smem.s
@@ -772,6 +772,18 @@ s_load_b32 s5, s[4:5], s0 offset:0x0 scope:SCOPE_DEV
 s_load_b32 s5, s[4:5], s0 offset:0x0 scope:SCOPE_SYS
 // GFX12: s_load_b32 s5, s[4:5], s0 offset:0x0 scope:SCOPE_SYS ; encoding: [0x42,0x01,0x60,0xf4,0x00,0x00,0x00,0x00]
 
+s_load_b32 s5, s[4:5], s0 offset:0x0 scope:0
+// GFX12: s_load_b32 s5, s[4:5], s0 offset:0x0    ; encoding: [0x42,0x01,0x00,0xf4,0x00,0x00,0x00,0x00]
+
+s_load_b32 s5, s[4:5], s0 offset:0x0 scope:1
+// GFX12: s_load_b32 s5, s[4:5], s0 offset:0x0 scope:SCOPE_SE ; encoding: [0x42,0x01,0x20,0xf4,0x00,0x00,0x00,0x00]
+
+s_load_b32 s5, s[4:5], s0 offset:0x0 scope:0x2
+// GFX12: s_load_b32 s5, s[4:5], s0 offset:0x0 scope:SCOPE_DEV ; encoding: [0x42,0x01,0x40,0xf4,0x00,0x00,0x00,0x00]
+
+s_load_b32 s5, s[4:5], s0 offset:0x0 scope:03
+// GFX12: s_load_b32 s5, s[4:5], s0 offset:0x0 scope:SCOPE_SYS ; encoding: [0x42,0x01,0x60,0xf4,0x00,0x00,0x00,0x00]
+
 s_load_b32 s5, s[4:5], s0 offset:0x0 th:TH_LOAD_HT scope:SCOPE_SE
 // GFX12: s_load_b32 s5, s[4:5], s0 offset:0x0 th:TH_LOAD_HT scope:SCOPE_SE ; encoding: [0x42,0x01,0x20,0xf5,0x00,0x00,0x00,0x00]
 



More information about the llvm-commits mailing list