Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AMDGPU] Fix CPol operands of MUBUF atomics. #73118

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

kosarev
Copy link
Collaborator

@kosarev kosarev commented Nov 22, 2023

Resolves AsmParser ambiguities, e.g., between
BUFFER_ATOMIC_XOR_X2_BOTHEN_vi and BUFFER_ATOMIC_XOR_X2_BOTHEN_RTN_vi.

Part of #69256.

Resolves AsmParser ambiguities, e.g., between
BUFFER_ATOMIC_XOR_X2_BOTHEN_vi and BUFFER_ATOMIC_XOR_X2_BOTHEN_RTN_vi.

Part of <llvm#69256>.
@llvmbot llvmbot added backend:AMDGPU mc Machine (object) code labels Nov 22, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 22, 2023

@llvm/pr-subscribers-mc

Author: Ivan Kosarev (kosarev)

Changes

Resolves AsmParser ambiguities, e.g., between
BUFFER_ATOMIC_XOR_X2_BOTHEN_vi and BUFFER_ATOMIC_XOR_X2_BOTHEN_RTN_vi.

Part of <#69256>.


Full diff: https://github.com/llvm/llvm-project/pull/73118.diff

4 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/BUFInstructions.td (+2-1)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.td (+4-3)
  • (modified) llvm/test/MC/AMDGPU/atomic-fadd-insts.s (+2-2)
  • (modified) llvm/test/MC/AMDGPU/gfx90a_asm_features.s (+3-3)
diff --git a/llvm/lib/Target/AMDGPU/BUFInstructions.td b/llvm/lib/Target/AMDGPU/BUFInstructions.td
index ea98b3a8cd0655d..44fd4ef86412703 100644
--- a/llvm/lib/Target/AMDGPU/BUFInstructions.td
+++ b/llvm/lib/Target/AMDGPU/BUFInstructions.td
@@ -637,7 +637,8 @@ class getMUBUFAtomicInsDA<RegisterClass vdataClass, bit vdata_in,
   dag VData = !if(vdata_in, (ins vdata_op:$vdata_in), (ins vdata_op:$vdata));
   dag Data = !if(!empty(vaddrList), VData, !con(VData, (ins vaddrClass:$vaddr)));
   dag MainInputs = (ins SReg_128:$srsrc, SCSrc_b32:$soffset, offset:$offset);
-  dag CPol = !if(vdata_in, (ins CPol_GLC1:$cpol), (ins CPol_0:$cpol));
+  dag CPol = !if(vdata_in, (ins CPol_GLC_WithDefault:$cpol),
+                           (ins CPol_NonGLC_WithDefault:$cpol));
 
   dag ret = !con(Data, MainInputs, CPol);
 }
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.td b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
index 02c769bf21ac3ea..1e357609078543a 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
@@ -1035,9 +1035,8 @@ class NamedBitOperand<string Id, string Name = NAME>
 
 class DefaultOperand<CustomOperand Op, int Value>
   : OperandWithDefaultOps<Op.Type, (ops (Op.Type Value))>,
-    CustomOperandProps<1, Op.ParserMatchClass.Name> {
-  let PredicateMethod = Op.ParserMatchClass.PredicateMethod;
-  let ParserMethod = Op.ParserMatchClass.ParserMethod;
+    CustomOperandProps<1> {
+  let ParserMatchClass = Op.ParserMatchClass;
   let PrintMethod = Op.PrintMethod;
 }
 
@@ -1080,6 +1079,8 @@ def CPol_0 : DefaultOperand<CPol, 0>;
 def CPol_GLC1 : DefaultOperand<CPol, 1>;
 def CPol_GLC : ValuePredicatedOperand<CPol, "Op.getImm() & CPol::GLC">;
 def CPol_NonGLC : ValuePredicatedOperand<CPol, "!(Op.getImm() & CPol::GLC)", 1>;
+def CPol_GLC_WithDefault : DefaultOperand<CPol_GLC, !shl(1, CPolBit.GLC)>;
+def CPol_NonGLC_WithDefault : DefaultOperand<CPol_NonGLC, 0>;
 
 def TFE : NamedBitOperand<"tfe">;
 def UNorm : NamedBitOperand<"unorm">;
diff --git a/llvm/test/MC/AMDGPU/atomic-fadd-insts.s b/llvm/test/MC/AMDGPU/atomic-fadd-insts.s
index e112cce30cffe1b..67391102150662d 100644
--- a/llvm/test/MC/AMDGPU/atomic-fadd-insts.s
+++ b/llvm/test/MC/AMDGPU/atomic-fadd-insts.s
@@ -41,7 +41,7 @@ buffer_atomic_add_f32 v5, off, s[8:11], s3 offset:7
 // GFX908: encoding: [0x07,0x00,0x34,0xe1,0x00,0x05,0x02,0x03]
 
 buffer_atomic_add_f32 v5, off, s[8:11], s3 offset:4095 glc
-// GFX908-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 
 buffer_atomic_add_f32 v5, off, s[8:11], s3 offset:4095 slc
 // GFX908: encoding: [0xff,0x0f,0x36,0xe1,0x00,0x05,0x02,0x03]
@@ -86,7 +86,7 @@ buffer_atomic_pk_add_f16 v5, off, s[8:11], s3 offset:7
 // GFX908: encoding: [0x07,0x00,0x38,0xe1,0x00,0x05,0x02,0x03]
 
 buffer_atomic_pk_add_f16 v5, off, s[8:11], s3 offset:4095 glc
-// GFX908-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 
 buffer_atomic_pk_add_f16 v5, off, s[8:11], s3 offset:4095 slc
 // GFX908: encoding: [0xff,0x0f,0x3a,0xe1,0x00,0x05,0x02,0x03]
diff --git a/llvm/test/MC/AMDGPU/gfx90a_asm_features.s b/llvm/test/MC/AMDGPU/gfx90a_asm_features.s
index d5f2755582d25d8..31f51eb1be2c2d9 100644
--- a/llvm/test/MC/AMDGPU/gfx90a_asm_features.s
+++ b/llvm/test/MC/AMDGPU/gfx90a_asm_features.s
@@ -955,17 +955,17 @@ v_xor_b32 v6, v29, v27 row_newbcast:15
 
 // GFX90A: buffer_atomic_add_f32 v0, v2, s[4:7], 0 idxen glc ; encoding: [0x00,0x60,0x34,0xe1,0x02,0x00,0x01,0x80]
 // GFX1010: :[[@LINE+2]]:{{[0-9]+}}: error: instruction not supported on this GPU
-// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 buffer_atomic_add_f32 v0, v2, s[4:7], 0 idxen glc
 
 // GFX90A: buffer_atomic_add_f32 v0, v2, s[4:7], 0 idxen glc ; encoding: [0x00,0x60,0x34,0xe1,0x02,0x00,0x01,0x80]
 // GFX1010: :[[@LINE+2]]:{{[0-9]+}}: error: instruction not supported on this GPU
-// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 buffer_atomic_add_f32 v0, v2, s[4:7], 0 idxen glc
 
 // GFX90A: buffer_atomic_pk_add_f16 v0, v2, s[4:7], 0 idxen glc ; encoding: [0x00,0x60,0x38,0xe1,0x02,0x00,0x01,0x80]
 // GFX1010: :[[@LINE+2]]:{{[0-9]+}}: error: instruction not supported on this GPU
-// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 buffer_atomic_pk_add_f16 v0, v2, s[4:7], 0 idxen glc
 
 // GFX90A: global_atomic_add_f32 v0, v[0:1], v2, off glc ; encoding: [0x00,0x80,0x35,0xdd,0x00,0x02,0x7f,0x00]

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 22, 2023

@llvm/pr-subscribers-backend-amdgpu

Author: Ivan Kosarev (kosarev)

Changes

Resolves AsmParser ambiguities, e.g., between
BUFFER_ATOMIC_XOR_X2_BOTHEN_vi and BUFFER_ATOMIC_XOR_X2_BOTHEN_RTN_vi.

Part of <#69256>.


Full diff: https://github.com/llvm/llvm-project/pull/73118.diff

4 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/BUFInstructions.td (+2-1)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.td (+4-3)
  • (modified) llvm/test/MC/AMDGPU/atomic-fadd-insts.s (+2-2)
  • (modified) llvm/test/MC/AMDGPU/gfx90a_asm_features.s (+3-3)
diff --git a/llvm/lib/Target/AMDGPU/BUFInstructions.td b/llvm/lib/Target/AMDGPU/BUFInstructions.td
index ea98b3a8cd0655d..44fd4ef86412703 100644
--- a/llvm/lib/Target/AMDGPU/BUFInstructions.td
+++ b/llvm/lib/Target/AMDGPU/BUFInstructions.td
@@ -637,7 +637,8 @@ class getMUBUFAtomicInsDA<RegisterClass vdataClass, bit vdata_in,
   dag VData = !if(vdata_in, (ins vdata_op:$vdata_in), (ins vdata_op:$vdata));
   dag Data = !if(!empty(vaddrList), VData, !con(VData, (ins vaddrClass:$vaddr)));
   dag MainInputs = (ins SReg_128:$srsrc, SCSrc_b32:$soffset, offset:$offset);
-  dag CPol = !if(vdata_in, (ins CPol_GLC1:$cpol), (ins CPol_0:$cpol));
+  dag CPol = !if(vdata_in, (ins CPol_GLC_WithDefault:$cpol),
+                           (ins CPol_NonGLC_WithDefault:$cpol));
 
   dag ret = !con(Data, MainInputs, CPol);
 }
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.td b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
index 02c769bf21ac3ea..1e357609078543a 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.td
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.td
@@ -1035,9 +1035,8 @@ class NamedBitOperand<string Id, string Name = NAME>
 
 class DefaultOperand<CustomOperand Op, int Value>
   : OperandWithDefaultOps<Op.Type, (ops (Op.Type Value))>,
-    CustomOperandProps<1, Op.ParserMatchClass.Name> {
-  let PredicateMethod = Op.ParserMatchClass.PredicateMethod;
-  let ParserMethod = Op.ParserMatchClass.ParserMethod;
+    CustomOperandProps<1> {
+  let ParserMatchClass = Op.ParserMatchClass;
   let PrintMethod = Op.PrintMethod;
 }
 
@@ -1080,6 +1079,8 @@ def CPol_0 : DefaultOperand<CPol, 0>;
 def CPol_GLC1 : DefaultOperand<CPol, 1>;
 def CPol_GLC : ValuePredicatedOperand<CPol, "Op.getImm() & CPol::GLC">;
 def CPol_NonGLC : ValuePredicatedOperand<CPol, "!(Op.getImm() & CPol::GLC)", 1>;
+def CPol_GLC_WithDefault : DefaultOperand<CPol_GLC, !shl(1, CPolBit.GLC)>;
+def CPol_NonGLC_WithDefault : DefaultOperand<CPol_NonGLC, 0>;
 
 def TFE : NamedBitOperand<"tfe">;
 def UNorm : NamedBitOperand<"unorm">;
diff --git a/llvm/test/MC/AMDGPU/atomic-fadd-insts.s b/llvm/test/MC/AMDGPU/atomic-fadd-insts.s
index e112cce30cffe1b..67391102150662d 100644
--- a/llvm/test/MC/AMDGPU/atomic-fadd-insts.s
+++ b/llvm/test/MC/AMDGPU/atomic-fadd-insts.s
@@ -41,7 +41,7 @@ buffer_atomic_add_f32 v5, off, s[8:11], s3 offset:7
 // GFX908: encoding: [0x07,0x00,0x34,0xe1,0x00,0x05,0x02,0x03]
 
 buffer_atomic_add_f32 v5, off, s[8:11], s3 offset:4095 glc
-// GFX908-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 
 buffer_atomic_add_f32 v5, off, s[8:11], s3 offset:4095 slc
 // GFX908: encoding: [0xff,0x0f,0x36,0xe1,0x00,0x05,0x02,0x03]
@@ -86,7 +86,7 @@ buffer_atomic_pk_add_f16 v5, off, s[8:11], s3 offset:7
 // GFX908: encoding: [0x07,0x00,0x38,0xe1,0x00,0x05,0x02,0x03]
 
 buffer_atomic_pk_add_f16 v5, off, s[8:11], s3 offset:4095 glc
-// GFX908-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 
 buffer_atomic_pk_add_f16 v5, off, s[8:11], s3 offset:4095 slc
 // GFX908: encoding: [0xff,0x0f,0x3a,0xe1,0x00,0x05,0x02,0x03]
diff --git a/llvm/test/MC/AMDGPU/gfx90a_asm_features.s b/llvm/test/MC/AMDGPU/gfx90a_asm_features.s
index d5f2755582d25d8..31f51eb1be2c2d9 100644
--- a/llvm/test/MC/AMDGPU/gfx90a_asm_features.s
+++ b/llvm/test/MC/AMDGPU/gfx90a_asm_features.s
@@ -955,17 +955,17 @@ v_xor_b32 v6, v29, v27 row_newbcast:15
 
 // GFX90A: buffer_atomic_add_f32 v0, v2, s[4:7], 0 idxen glc ; encoding: [0x00,0x60,0x34,0xe1,0x02,0x00,0x01,0x80]
 // GFX1010: :[[@LINE+2]]:{{[0-9]+}}: error: instruction not supported on this GPU
-// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 buffer_atomic_add_f32 v0, v2, s[4:7], 0 idxen glc
 
 // GFX90A: buffer_atomic_add_f32 v0, v2, s[4:7], 0 idxen glc ; encoding: [0x00,0x60,0x34,0xe1,0x02,0x00,0x01,0x80]
 // GFX1010: :[[@LINE+2]]:{{[0-9]+}}: error: instruction not supported on this GPU
-// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 buffer_atomic_add_f32 v0, v2, s[4:7], 0 idxen glc
 
 // GFX90A: buffer_atomic_pk_add_f16 v0, v2, s[4:7], 0 idxen glc ; encoding: [0x00,0x60,0x38,0xe1,0x02,0x00,0x01,0x80]
 // GFX1010: :[[@LINE+2]]:{{[0-9]+}}: error: instruction not supported on this GPU
-// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: instruction must not use glc
+// GFX908: :[[@LINE+1]]:{{[0-9]+}}: error: operands are not valid for this GPU or mode
 buffer_atomic_pk_add_f16 v0, v2, s[4:7], 0 idxen glc
 
 // GFX90A: global_atomic_add_f32 v0, v[0:1], v2, off glc ; encoding: [0x00,0x80,0x35,0xdd,0x00,0x02,0x7f,0x00]

Comment on lines +1082 to +1083
def CPol_GLC_WithDefault : DefaultOperand<CPol_GLC, !shl(1, CPolBit.GLC)>;
def CPol_NonGLC_WithDefault : DefaultOperand<CPol_NonGLC, 0>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the behaviour of CPol_(Non)GLC_WithDefault different from CPol_(Non)GLC?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The *_WithDefault operands are OperandWithDefaultOps, which they need to be because the patterns specified for the atomic instructions provide no value for the CPol operands.

@kosarev kosarev merged commit 7814806 into llvm:main Nov 23, 2023
5 checks passed
@kosarev kosarev deleted the asmparser_fix_ambiguities branch November 23, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants