Skip to content

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 llvm: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 llvm:mc Machine (object) code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants