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] Add missing GFX10 buffer format d16 hi instructions #84809

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Mar 11, 2024

No description provided.

@llvmbot llvmbot added backend:AMDGPU mc Machine (object) code labels Mar 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 11, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-amdgpu

Author: Jay Foad (jayfoad)

Changes

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

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/BUFInstructions.td (+2-3)
  • (modified) llvm/test/MC/AMDGPU/gfx10_asm_mubuf.s (+6)
  • (modified) llvm/test/MC/Disassembler/AMDGPU/gfx10_mubuf.txt (+6)
diff --git a/llvm/lib/Target/AMDGPU/BUFInstructions.td b/llvm/lib/Target/AMDGPU/BUFInstructions.td
index a1bbe170ee29a6..c7091028b3b5e5 100644
--- a/llvm/lib/Target/AMDGPU/BUFInstructions.td
+++ b/llvm/lib/Target/AMDGPU/BUFInstructions.td
@@ -2691,9 +2691,8 @@ defm BUFFER_LOAD_SBYTE_D16        : MUBUF_Real_AllAddr_gfx10<0x022>;
 defm BUFFER_LOAD_SBYTE_D16_HI     : MUBUF_Real_AllAddr_gfx10<0x023>;
 defm BUFFER_LOAD_SHORT_D16        : MUBUF_Real_AllAddr_gfx10<0x024>;
 defm BUFFER_LOAD_SHORT_D16_HI     : MUBUF_Real_AllAddr_gfx10<0x025>;
-// FIXME-GFX10: Add following instructions:
-//defm BUFFER_LOAD_FORMAT_D16_HI_X  : MUBUF_Real_AllAddr_gfx10<0x026>;
-//defm BUFFER_STORE_FORMAT_D16_HI_X : MUBUF_Real_AllAddr_gfx10<0x027>;
+defm BUFFER_LOAD_FORMAT_D16_HI_X  : MUBUF_Real_AllAddr_gfx10<0x026>;
+defm BUFFER_STORE_FORMAT_D16_HI_X : MUBUF_Real_AllAddr_gfx10<0x027>;
 defm BUFFER_LOAD_FORMAT_D16_X     : MUBUF_Real_AllAddr_gfx10<0x080>;
 defm BUFFER_LOAD_FORMAT_D16_XY    : MUBUF_Real_AllAddr_gfx10<0x081>;
 defm BUFFER_LOAD_FORMAT_D16_XYZ   : MUBUF_Real_AllAddr_gfx10<0x082>;
diff --git a/llvm/test/MC/AMDGPU/gfx10_asm_mubuf.s b/llvm/test/MC/AMDGPU/gfx10_asm_mubuf.s
index aacdfcb4e871ed..b77f8e0a319270 100644
--- a/llvm/test/MC/AMDGPU/gfx10_asm_mubuf.s
+++ b/llvm/test/MC/AMDGPU/gfx10_asm_mubuf.s
@@ -17,6 +17,9 @@ buffer_load_format_d16_xyz v[1:2], off, s[4:7], s1
 buffer_load_format_d16_xyzw v[1:2], off, s[4:7], s1
 // GFX10: encoding: [0x00,0x00,0x0c,0xe2,0x00,0x01,0x01,0x01]
 
+buffer_load_format_d16_hi_x v1, off, s[4:7], s1
+// GFX10: encoding: [0x00,0x00,0x98,0xe0,0x00,0x01,0x01,0x01]
+
 buffer_load_format_x v5, off, s[8:11], s3 offset:4095
 // GFX10: encoding: [0xff,0x0f,0x00,0xe0,0x00,0x05,0x02,0x03]
 
@@ -245,6 +248,9 @@ buffer_store_format_d16_xyz v[1:2], off, s[4:7], s1
 buffer_store_format_d16_xyzw v[1:2], off, s[4:7], s1
 // GFX10: encoding: [0x00,0x00,0x1c,0xe2,0x00,0x01,0x01,0x01]
 
+buffer_store_format_d16_hi_x v1, off, s[4:7], s1
+// GFX10: encoding: [0x00,0x00,0x9c,0xe0,0x00,0x01,0x01,0x01]
+
 buffer_store_format_x v1, off, s[12:15], s4 offset:4095
 // GFX10: encoding: [0xff,0x0f,0x10,0xe0,0x00,0x01,0x03,0x04]
 
diff --git a/llvm/test/MC/Disassembler/AMDGPU/gfx10_mubuf.txt b/llvm/test/MC/Disassembler/AMDGPU/gfx10_mubuf.txt
index b0731be4484c7a..849c89e37011f4 100644
--- a/llvm/test/MC/Disassembler/AMDGPU/gfx10_mubuf.txt
+++ b/llvm/test/MC/Disassembler/AMDGPU/gfx10_mubuf.txt
@@ -1328,6 +1328,9 @@
 # GFX10: buffer_load_format_d16_xyzw v[1:2], off, s[4:7], s1 ; encoding: [0x00,0x00,0x0c,0xe2,0x00,0x01,0x01,0x01]
 0x00,0x00,0x0c,0xe2,0x00,0x01,0x01,0x01
 
+# GFX10: buffer_load_format_d16_hi_x v1, off, s[4:7], s1 ; encoding: [0x00,0x00,0x98,0xe0,0x00,0x01,0x01,0x01]
+0x00,0x00,0x98,0xe0,0x00,0x01,0x01,0x01
+
 # GFX10: buffer_load_format_x v255, off, s[8:11], s3 offset:4095 ; encoding: [0xff,0x0f,0x00,0xe0,0x00,0xff,0x02,0x03]
 0xff,0x0f,0x00,0xe0,0x00,0xff,0x02,0x03
 
@@ -2039,6 +2042,9 @@
 # GFX10: buffer_store_format_d16_xyzw v[1:2], off, s[4:7], s1 ; encoding: [0x00,0x00,0x1c,0xe2,0x00,0x01,0x01,0x01]
 0x00,0x00,0x1c,0xe2,0x00,0x01,0x01,0x01
 
+# GFX10: buffer_store_format_d16_hi_x v1, off, s[4:7], s1 ; encoding: [0x00,0x00,0x9c,0xe0,0x00,0x01,0x01,0x01]
+0x00,0x00,0x9c,0xe0,0x00,0x01,0x01,0x01
+
 # GFX10: buffer_store_format_x v1, off, s[12:15], -1 offset:4095 ; encoding: [0xff,0x0f,0x10,0xe0,0x00,0x01,0x03,0xc1]
 0xff,0x0f,0x10,0xe0,0x00,0x01,0x03,0xc1
 

Copy link
Contributor

@Sisyph Sisyph left a comment

Choose a reason for hiding this comment

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

Why were these initially disabled? The origin commit review is not informative. Can you please add a codegen test to make sure they work?

@jayfoad
Copy link
Contributor Author

jayfoad commented Mar 11, 2024

Why were these initially disabled?

I don't know. I wasn't involved. @rampitec do you remember?

Can you please add a codegen test to make sure they work?

I am not expecting codegen to use these instructions (without some extra work) but I can check.

@jayfoad
Copy link
Contributor Author

jayfoad commented Mar 11, 2024

Can you please add a codegen test to make sure they work?

I am not expecting codegen to use these instructions (without some extra work) but I can check.

These instructions have existed since GFX9. As far as I can tell we have never implemented codegen for them.

@rampitec
Copy link
Collaborator

Why were these initially disabled?

I don't know. I wasn't involved. @rampitec do you remember?

I do not remember now either.

Copy link
Contributor

@Sisyph Sisyph left a comment

Choose a reason for hiding this comment

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

This is fine to fix the Asm/Disasm then.

@jayfoad jayfoad merged commit 36dece0 into llvm:main Mar 12, 2024
6 of 7 checks passed
@jayfoad jayfoad deleted the gfx10-buf branch March 12, 2024 08:20
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

4 participants