[llvm] [AMDGPU] Add parseStringOrIntWithPrefix helper in asm parser (PR #102213)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 6 13:20:29 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mc
@llvm/pr-subscribers-backend-amdgpu
Author: Stanislav Mekhanoshin (rampitec)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/102213.diff
4 Files Affected:
- (modified) llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (+61-66)
- (modified) llvm/test/MC/AMDGPU/gfx10_asm_vop1.s (+9)
- (modified) llvm/test/MC/AMDGPU/gfx10_err_pos.s (+26-6)
- (modified) llvm/test/MC/AMDGPU/gfx12_asm_smem.s (+12)
``````````diff
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 26a839a95df96..c8b594ffbc645 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 3cc25501ff7c0..80ec5934ad993 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 4db454f0ced5a..d99da6e0cb869 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 a64f35337d0e5..668f767661f68 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]
``````````
</details>
https://github.com/llvm/llvm-project/pull/102213
More information about the llvm-commits
mailing list