Skip to content

[AMDGPU] New GFX11+ aliases v_dot4_i32_i8 and v_dot8_i32_i4 #118997

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

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Dec 6, 2024

Previously we decided not to support these aliases because in other
generations they are real instructions with different behavior. Now I am
inclined to support them anyway for compatibility with SP3.

Previously we decided not to support these aliases because in other
generations they are real instructions with different behavior. Now I am
inclined to support them anyway for compatibility with SP3.
@llvmbot llvmbot added backend:AMDGPU mc Machine (object) code labels Dec 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: Jay Foad (jayfoad)

Changes

Previously we decided not to support these aliases because in other
generations they are real instructions with different behavior. Now I am
inclined to support them anyway for compatibility with SP3.


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

5 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/VOP3PInstructions.td (+5)
  • (modified) llvm/test/MC/AMDGPU/gfx11_asm_err.s (-14)
  • (added) llvm/test/MC/AMDGPU/gfx11_asm_vop3p_alias.s (+7)
  • (modified) llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s (+6)
  • (modified) llvm/test/MC/AMDGPU/gfx12_err.s (-14)
diff --git a/llvm/lib/Target/AMDGPU/VOP3PInstructions.td b/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
index bae37358ffe0c7..7638879afcb56c 100644
--- a/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
@@ -1845,6 +1845,11 @@ defm V_DOT4_I32_IU8  : VOP3P_Real_gfx11_gfx12<0x16>;
 defm V_DOT8_I32_IU4  : VOP3P_Real_gfx11_gfx12<0x18>;
 defm V_DOT2_F32_BF16 : VOP3P_Real_gfx11_gfx12<0x1a>;
 
+let AssemblerPredicate = isGFX11Plus in {
+  def : AMDGPUMnemonicAlias<"v_dot4_i32_i8", "v_dot4_i32_iu8">;
+  def : AMDGPUMnemonicAlias<"v_dot8_i32_i4", "v_dot8_i32_iu4">;
+}
+
 multiclass VOP3P_Real_WMMA <bits<7> op> {
   let WaveSizePredicate = isWave32, DecoderNamespace = "GFX11" in {
     defm _twoaddr_w32 : VOP3P_Real_Base <GFX11Gen, op>;
diff --git a/llvm/test/MC/AMDGPU/gfx11_asm_err.s b/llvm/test/MC/AMDGPU/gfx11_asm_err.s
index 68442b01bf7d90..1b1d1c04359bdf 100644
--- a/llvm/test/MC/AMDGPU/gfx11_asm_err.s
+++ b/llvm/test/MC/AMDGPU/gfx11_asm_err.s
@@ -75,20 +75,6 @@ v_cvt_f16_u16_e64_dpp v5, s1 row_shl:1 row_mask:0xf bank_mask:0xf
 v_dual_mul_f32 v0, v0, v2 : : v_dual_mul_f32 v1, v1, v3
 // GFX11: :[[@LINE-1]]:{{[0-9]+}}: error: unknown token in expression
 
-// On GFX11, v_dot8_i32_i4 is a valid SP3 alias for v_dot8_i32_iu4.
-// However, we intentionally leave it unimplemented because on other
-// processors v_dot8_i32_i4 denotes an instruction of a different
-// behaviour, which is considered potentially dangerous.
-v_dot8_i32_i4 v0, v1, v2, v3
-// GFX11: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
-// On GFX11, v_dot4_i32_i8 is a valid SP3 alias for v_dot4_i32_iu8.
-// However, we intentionally leave it unimplemented because on other
-// processors v_dot4_i32_i8 denotes an instruction of a different
-// behaviour, which is considered potentially dangerous.
-v_dot4_i32_i8 v0, v1, v2, v3
-// GFX11: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
 v_dot4c_i32_i8 v0, v1, v2
 // GFX11: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
 
diff --git a/llvm/test/MC/AMDGPU/gfx11_asm_vop3p_alias.s b/llvm/test/MC/AMDGPU/gfx11_asm_vop3p_alias.s
new file mode 100644
index 00000000000000..d2fdf013939fb5
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/gfx11_asm_vop3p_alias.s
@@ -0,0 +1,7 @@
+// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1100 -show-encoding %s | FileCheck -check-prefix=GFX11 %s
+
+v_dot4_i32_i8 v5, v1, v2, s3
+// GFX11: v_dot4_i32_iu8 v5, v1, v2, s3           ; encoding: [0x05,0x40,0x16,0xcc,0x01,0x05,0x0e,0x18]
+
+v_dot8_i32_i4 v5, v1, v2, s3
+// GFX11: v_dot8_i32_iu4 v5, v1, v2, s3           ; encoding: [0x05,0x40,0x18,0xcc,0x01,0x05,0x0e,0x18]
diff --git a/llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s b/llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s
index 5915cbc011863a..fadd24283d211f 100644
--- a/llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s
+++ b/llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s
@@ -5,3 +5,9 @@ v_pk_min_f16 v0, v1, v2
 
 v_pk_max_f16 v0, v1, v2
 // GFX12: v_pk_max_num_f16 v0, v1, v2             ; encoding: [0x00,0x40,0x1c,0xcc,0x01,0x05,0x02,0x18]
+
+v_dot4_i32_i8 v5, v1, v2, s3
+// GFX12: v_dot4_i32_iu8 v5, v1, v2, s3           ; encoding: [0x05,0x40,0x16,0xcc,0x01,0x05,0x0e,0x18]
+
+v_dot8_i32_i4 v5, v1, v2, s3
+// GFX12: v_dot8_i32_iu4 v5, v1, v2, s3           ; encoding: [0x05,0x40,0x18,0xcc,0x01,0x05,0x0e,0x18]
diff --git a/llvm/test/MC/AMDGPU/gfx12_err.s b/llvm/test/MC/AMDGPU/gfx12_err.s
index d8578d87279d1a..d55b86b54ec7d8 100644
--- a/llvm/test/MC/AMDGPU/gfx12_err.s
+++ b/llvm/test/MC/AMDGPU/gfx12_err.s
@@ -22,20 +22,6 @@ v_cvt_f16_u16_e64_dpp v5, s1 row_shl:1 row_mask:0xf bank_mask:0xf
 v_dual_mul_f32 v0, v0, v2 : : v_dual_mul_f32 v1, v1, v3
 // GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: unknown token in expression
 
-// On GFX12, v_dot8_i32_i4 is a valid SP3 alias for v_dot8_i32_iu4.
-// However, we intentionally leave it unimplemented because on other
-// processors v_dot8_i32_i4 denotes an instruction of a different
-// behaviour, which is considered potentially dangerous.
-v_dot8_i32_i4 v0, v1, v2, v3
-// GFX12-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
-// On GFX12, v_dot4_i32_i8 is a valid SP3 alias for v_dot4_i32_iu8.
-// However, we intentionally leave it unimplemented because on other
-// processors v_dot4_i32_i8 denotes an instruction of a different
-// behaviour, which is considered potentially dangerous.
-v_dot4_i32_i8 v0, v1, v2, v3
-// GFX12-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
 v_dot4c_i32_i8 v0, v1, v2
 // GFX12-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
 

@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2024

@llvm/pr-subscribers-mc

Author: Jay Foad (jayfoad)

Changes

Previously we decided not to support these aliases because in other
generations they are real instructions with different behavior. Now I am
inclined to support them anyway for compatibility with SP3.


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

5 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/VOP3PInstructions.td (+5)
  • (modified) llvm/test/MC/AMDGPU/gfx11_asm_err.s (-14)
  • (added) llvm/test/MC/AMDGPU/gfx11_asm_vop3p_alias.s (+7)
  • (modified) llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s (+6)
  • (modified) llvm/test/MC/AMDGPU/gfx12_err.s (-14)
diff --git a/llvm/lib/Target/AMDGPU/VOP3PInstructions.td b/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
index bae37358ffe0c7..7638879afcb56c 100644
--- a/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP3PInstructions.td
@@ -1845,6 +1845,11 @@ defm V_DOT4_I32_IU8  : VOP3P_Real_gfx11_gfx12<0x16>;
 defm V_DOT8_I32_IU4  : VOP3P_Real_gfx11_gfx12<0x18>;
 defm V_DOT2_F32_BF16 : VOP3P_Real_gfx11_gfx12<0x1a>;
 
+let AssemblerPredicate = isGFX11Plus in {
+  def : AMDGPUMnemonicAlias<"v_dot4_i32_i8", "v_dot4_i32_iu8">;
+  def : AMDGPUMnemonicAlias<"v_dot8_i32_i4", "v_dot8_i32_iu4">;
+}
+
 multiclass VOP3P_Real_WMMA <bits<7> op> {
   let WaveSizePredicate = isWave32, DecoderNamespace = "GFX11" in {
     defm _twoaddr_w32 : VOP3P_Real_Base <GFX11Gen, op>;
diff --git a/llvm/test/MC/AMDGPU/gfx11_asm_err.s b/llvm/test/MC/AMDGPU/gfx11_asm_err.s
index 68442b01bf7d90..1b1d1c04359bdf 100644
--- a/llvm/test/MC/AMDGPU/gfx11_asm_err.s
+++ b/llvm/test/MC/AMDGPU/gfx11_asm_err.s
@@ -75,20 +75,6 @@ v_cvt_f16_u16_e64_dpp v5, s1 row_shl:1 row_mask:0xf bank_mask:0xf
 v_dual_mul_f32 v0, v0, v2 : : v_dual_mul_f32 v1, v1, v3
 // GFX11: :[[@LINE-1]]:{{[0-9]+}}: error: unknown token in expression
 
-// On GFX11, v_dot8_i32_i4 is a valid SP3 alias for v_dot8_i32_iu4.
-// However, we intentionally leave it unimplemented because on other
-// processors v_dot8_i32_i4 denotes an instruction of a different
-// behaviour, which is considered potentially dangerous.
-v_dot8_i32_i4 v0, v1, v2, v3
-// GFX11: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
-// On GFX11, v_dot4_i32_i8 is a valid SP3 alias for v_dot4_i32_iu8.
-// However, we intentionally leave it unimplemented because on other
-// processors v_dot4_i32_i8 denotes an instruction of a different
-// behaviour, which is considered potentially dangerous.
-v_dot4_i32_i8 v0, v1, v2, v3
-// GFX11: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
 v_dot4c_i32_i8 v0, v1, v2
 // GFX11: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
 
diff --git a/llvm/test/MC/AMDGPU/gfx11_asm_vop3p_alias.s b/llvm/test/MC/AMDGPU/gfx11_asm_vop3p_alias.s
new file mode 100644
index 00000000000000..d2fdf013939fb5
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/gfx11_asm_vop3p_alias.s
@@ -0,0 +1,7 @@
+// RUN: llvm-mc -triple=amdgcn -mcpu=gfx1100 -show-encoding %s | FileCheck -check-prefix=GFX11 %s
+
+v_dot4_i32_i8 v5, v1, v2, s3
+// GFX11: v_dot4_i32_iu8 v5, v1, v2, s3           ; encoding: [0x05,0x40,0x16,0xcc,0x01,0x05,0x0e,0x18]
+
+v_dot8_i32_i4 v5, v1, v2, s3
+// GFX11: v_dot8_i32_iu4 v5, v1, v2, s3           ; encoding: [0x05,0x40,0x18,0xcc,0x01,0x05,0x0e,0x18]
diff --git a/llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s b/llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s
index 5915cbc011863a..fadd24283d211f 100644
--- a/llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s
+++ b/llvm/test/MC/AMDGPU/gfx12_asm_vop3p_aliases.s
@@ -5,3 +5,9 @@ v_pk_min_f16 v0, v1, v2
 
 v_pk_max_f16 v0, v1, v2
 // GFX12: v_pk_max_num_f16 v0, v1, v2             ; encoding: [0x00,0x40,0x1c,0xcc,0x01,0x05,0x02,0x18]
+
+v_dot4_i32_i8 v5, v1, v2, s3
+// GFX12: v_dot4_i32_iu8 v5, v1, v2, s3           ; encoding: [0x05,0x40,0x16,0xcc,0x01,0x05,0x0e,0x18]
+
+v_dot8_i32_i4 v5, v1, v2, s3
+// GFX12: v_dot8_i32_iu4 v5, v1, v2, s3           ; encoding: [0x05,0x40,0x18,0xcc,0x01,0x05,0x0e,0x18]
diff --git a/llvm/test/MC/AMDGPU/gfx12_err.s b/llvm/test/MC/AMDGPU/gfx12_err.s
index d8578d87279d1a..d55b86b54ec7d8 100644
--- a/llvm/test/MC/AMDGPU/gfx12_err.s
+++ b/llvm/test/MC/AMDGPU/gfx12_err.s
@@ -22,20 +22,6 @@ v_cvt_f16_u16_e64_dpp v5, s1 row_shl:1 row_mask:0xf bank_mask:0xf
 v_dual_mul_f32 v0, v0, v2 : : v_dual_mul_f32 v1, v1, v3
 // GFX12-ERR: [[@LINE-1]]:{{[0-9]+}}: error: unknown token in expression
 
-// On GFX12, v_dot8_i32_i4 is a valid SP3 alias for v_dot8_i32_iu4.
-// However, we intentionally leave it unimplemented because on other
-// processors v_dot8_i32_i4 denotes an instruction of a different
-// behaviour, which is considered potentially dangerous.
-v_dot8_i32_i4 v0, v1, v2, v3
-// GFX12-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
-// On GFX12, v_dot4_i32_i8 is a valid SP3 alias for v_dot4_i32_iu8.
-// However, we intentionally leave it unimplemented because on other
-// processors v_dot4_i32_i8 denotes an instruction of a different
-// behaviour, which is considered potentially dangerous.
-v_dot4_i32_i8 v0, v1, v2, v3
-// GFX12-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
-
 v_dot4c_i32_i8 v0, v1, v2
 // GFX12-ERR: :[[@LINE-1]]:{{[0-9]+}}: error: instruction not supported on this GPU
 

@jayfoad
Copy link
Contributor Author

jayfoad commented Dec 6, 2024

@Sisyph @mbrkusanin any thoughts about this? My immediate motivation is to make it simpler to do automated testing against SP3.

Copy link
Collaborator

@rampitec rampitec left a comment

Choose a reason for hiding this comment

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

That's always a bad idea to repurpose instructions' names, but it is already done. LGTM for the purpose of SP3 compatibility.

@DadSchoorse
Copy link

wouldn't it make sense to set neg_lo:[1,1,0] by default for these opcodes? Then they would behave like gfx9/10 opcodes with the same names.

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.

LGTM. I'm having a hard time even picturing a scenario where having these aliases would confuse someone. Emitting textual assembly on an old target and reassemble it on gfx11? Porting inline asm?

@jayfoad
Copy link
Contributor Author

jayfoad commented Dec 9, 2024

wouldn't it make sense to set neg_lo:[1,1,0] by default for these opcodes? Then they would behave like gfx9/10 opcodes with the same names.

Maybe, if the goal was to provide backwards compatibility of handwritten assembly code. But that's really not a goal of the LLVM assembler. The only reason for implementing these aliases is for compatibility with the proprietary assembler.

@jayfoad jayfoad merged commit 1004496 into llvm:main Dec 9, 2024
11 checks passed
@jayfoad jayfoad deleted the v-dot4-i32-i8 branch December 9, 2024 11:48
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Feb 3, 2025
) (#282)

Previously we decided not to support these aliases because in other
generations they are real instructions with different behavior. Now I am
inclined to support them anyway for compatibility with SP3.

Co-authored-by: Jay Foad <[email protected]>
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.

5 participants