Skip to content

Conversation

@broxigarchen
Copy link
Contributor

@broxigarchen broxigarchen commented Dec 12, 2024

Support true16 format for v_cndmask_b16 in MC and CodeGen in true16 and fake16 flow.

Since we are replacing v_cndmask_b16 to v_cndmask_b16_t16/fake16, we have to at least update the fake16 codeGen to get codeGen test passing. For this case, we have to update the true16 and with fake16 together, otherwise some of the true16 tests will fail

@broxigarchen broxigarchen force-pushed the main-merge-true16-vop3-mc-more-instructions-8 branch from 466ed96 to cec4aa4 Compare December 12, 2024 19:55
@broxigarchen broxigarchen changed the title [AMDGPU][True16][MC] true16 for v_cndmask_b16 [AMDGPU][True16] true16 for v_cndmask_b16 Dec 12, 2024
@broxigarchen broxigarchen force-pushed the main-merge-true16-vop3-mc-more-instructions-8 branch 2 times, most recently from a9ed03a to fd996b4 Compare December 13, 2024 18:21
@broxigarchen broxigarchen changed the title [AMDGPU][True16] true16 for v_cndmask_b16 [AMDGPU][True16][MC][CodeGen] true16 for v_cndmask_b16 Dec 13, 2024
@broxigarchen broxigarchen marked this pull request as ready for review December 13, 2024 21:12
@llvmbot
Copy link
Member

llvmbot commented Dec 13, 2024

@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-mc

Author: Brox Chen (broxigarchen)

Changes

Support true16 format for v_cndmask_b16 in MC and CodeGen in true16/fake16 flow.

Since we are replacing v_cndmask_b16 to v_cndmask_b16_t16/fake16, we have to at least update the fake16 codeGen to get codeGen test passing. For this case, we have to update the true16 and with fake16 together, otherwise some of the true16 tests will fail


Patch is 635.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119736.diff

41 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp (+4-4)
  • (modified) llvm/lib/Target/AMDGPU/SIInstructions.td (+20-2)
  • (modified) llvm/lib/Target/AMDGPU/VOP2Instructions.td (+23-3)
  • (modified) llvm/test/CodeGen/AMDGPU/GlobalISel/saddsat.ll (+68-67)
  • (modified) llvm/test/CodeGen/AMDGPU/GlobalISel/ssubsat.ll (+95-92)
  • (modified) llvm/test/CodeGen/AMDGPU/bf16.ll (+765-732)
  • (modified) llvm/test/CodeGen/AMDGPU/dagcombine-fmul-sel.ll (+49-51)
  • (modified) llvm/test/CodeGen/AMDGPU/extract-subvector-16bit.ll (+80-76)
  • (modified) llvm/test/CodeGen/AMDGPU/extract_vector_elt-f16.ll (+46-46)
  • (modified) llvm/test/CodeGen/AMDGPU/fmax_legacy.f16.ll (+54-51)
  • (modified) llvm/test/CodeGen/AMDGPU/fmin_legacy.f16.ll (+54-51)
  • (modified) llvm/test/CodeGen/AMDGPU/fmul-2-combine-multi-use.ll (+14-13)
  • (modified) llvm/test/CodeGen/AMDGPU/fmul-to-ldexp.ll (+184-184)
  • (modified) llvm/test/CodeGen/AMDGPU/fneg-combines.f16.ll (+15-16)
  • (modified) llvm/test/CodeGen/AMDGPU/fneg-modifier-casting.ll (+52-49)
  • (modified) llvm/test/CodeGen/AMDGPU/fract-match.ll (+14-14)
  • (modified) llvm/test/CodeGen/AMDGPU/insert_vector_elt.v2i16.ll (+72-71)
  • (modified) llvm/test/CodeGen/AMDGPU/llvm.maximum.f16.ll (+206-215)
  • (modified) llvm/test/CodeGen/AMDGPU/llvm.minimum.f16.ll (+206-215)
  • (modified) llvm/test/CodeGen/AMDGPU/llvm.round.ll (+11-11)
  • (modified) llvm/test/CodeGen/AMDGPU/lround.ll (+4-4)
  • (modified) llvm/test/CodeGen/AMDGPU/maximumnum.ll (+44-42)
  • (modified) llvm/test/CodeGen/AMDGPU/minimumnum.ll (+44-42)
  • (modified) llvm/test/CodeGen/AMDGPU/select-fabs-fneg-extract.f16.ll (+95-95)
  • (modified) llvm/test/CodeGen/AMDGPU/select-fabs-fneg-extract.v2f16.ll (+343-330)
  • (modified) llvm/test/CodeGen/AMDGPU/select-flags-to-fmin-fmax.ll (+132-120)
  • (modified) llvm/test/CodeGen/AMDGPU/select.f16.ll (+366-374)
  • (modified) llvm/test/CodeGen/AMDGPU/v_cndmask.ll (+2-2)
  • (modified) llvm/test/CodeGen/AMDGPU/valu-mask-write-hazard.mir (+3-3)
  • (modified) llvm/test/MC/AMDGPU/gfx11_asm_vop3.s (+100-73)
  • (modified) llvm/test/MC/AMDGPU/gfx11_asm_vop3_dpp16.s (+107-80)
  • (modified) llvm/test/MC/AMDGPU/gfx11_asm_vop3_dpp8.s (+56-29)
  • (modified) llvm/test/MC/AMDGPU/gfx12_asm_vop3.s (+100-73)
  • (modified) llvm/test/MC/AMDGPU/gfx12_asm_vop3_dpp16.s (+111-92)
  • (modified) llvm/test/MC/AMDGPU/gfx12_asm_vop3_dpp8.s (+57-38)
  • (modified) llvm/test/MC/Disassembler/AMDGPU/gfx11_dasm_vop3.txt (+70-25)
  • (modified) llvm/test/MC/Disassembler/AMDGPU/gfx11_dasm_vop3_dpp16.txt (+82-29)
  • (modified) llvm/test/MC/Disassembler/AMDGPU/gfx11_dasm_vop3_dpp8.txt (+46-11)
  • (modified) llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vop3.txt (+70-25)
  • (modified) llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vop3_dpp16.txt (+74-27)
  • (modified) llvm/test/MC/Disassembler/AMDGPU/gfx12_dasm_vop3_dpp8.txt (+38-9)
diff --git a/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp b/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
index 5207201e14c091..6baef137df5e16 100644
--- a/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp
@@ -3007,8 +3007,8 @@ bool GCNHazardRecognizer::fixVALUMaskWriteHazard(MachineInstr *MI) {
     switch (I.getOpcode()) {
     case AMDGPU::V_ADDC_U32_e32:
     case AMDGPU::V_ADDC_U32_dpp:
-    case AMDGPU::V_CNDMASK_B16_e32:
-    case AMDGPU::V_CNDMASK_B16_dpp:
+    case AMDGPU::V_CNDMASK_B16_fake16_e32:
+    case AMDGPU::V_CNDMASK_B16_fake16_dpp:
     case AMDGPU::V_CNDMASK_B32_e32:
     case AMDGPU::V_CNDMASK_B32_dpp:
     case AMDGPU::V_DIV_FMAS_F32_e64:
@@ -3023,8 +3023,8 @@ bool GCNHazardRecognizer::fixVALUMaskWriteHazard(MachineInstr *MI) {
              HazardReg == AMDGPU::VCC_HI;
     case AMDGPU::V_ADDC_U32_e64:
     case AMDGPU::V_ADDC_U32_e64_dpp:
-    case AMDGPU::V_CNDMASK_B16_e64:
-    case AMDGPU::V_CNDMASK_B16_e64_dpp:
+    case AMDGPU::V_CNDMASK_B16_fake16_e64:
+    case AMDGPU::V_CNDMASK_B16_fake16_e64_dpp:
     case AMDGPU::V_CNDMASK_B32_e64:
     case AMDGPU::V_CNDMASK_B32_e64_dpp:
     case AMDGPU::V_SUBB_U32_e64:
diff --git a/llvm/lib/Target/AMDGPU/SIInstructions.td b/llvm/lib/Target/AMDGPU/SIInstructions.td
index bc25d75131cc35..bf8935d0812be8 100644
--- a/llvm/lib/Target/AMDGPU/SIInstructions.td
+++ b/llvm/lib/Target/AMDGPU/SIInstructions.td
@@ -1245,11 +1245,29 @@ class VOPSelectPat <ValueType vt> : GCNPat <
   (vt (select i1:$src0, vt:$src1, vt:$src2)),
   (V_CNDMASK_B32_e64 0, VSrc_b32:$src2, 0, VSrc_b32:$src1, SSrc_i1:$src0)
 >;
+class VOPSelectPat_t16 <ValueType vt> : GCNPat <
+  (vt (select i1:$src0, vt:$src1, vt:$src2)),
+  (V_CNDMASK_B16_t16_e64 0, VSrcT_b16:$src2, 0, VSrcT_b16:$src1, SSrc_i1:$src0)
+>;
+class VOPSelectPat_fake16 <ValueType vt> : GCNPat <
+  (vt (select i1:$src0, vt:$src1, vt:$src2)),
+  (V_CNDMASK_B16_fake16_e64 0, VSrc_b16:$src2, 0, VSrc_b16:$src1, SSrc_i1:$src0)
+>;
 
 def : VOPSelectModsPat <i32>;
 def : VOPSelectModsPat <f32>;
-def : VOPSelectPat <f16>;
-def : VOPSelectPat <i16>;
+let True16Predicate = NotHasTrue16BitInsts in {
+  def : VOPSelectPat <f16>;
+  def : VOPSelectPat <i16>;
+} // End True16Predicate = NotHasTrue16BitInsts
+let True16Predicate = UseRealTrue16Insts in {
+  def : VOPSelectPat_t16 <f16>;
+  def : VOPSelectPat_t16 <i16>;
+} // End True16Predicate = UseRealTrue16Insts
+let True16Predicate = UseFakeTrue16Insts in {
+  def : VOPSelectPat_fake16 <f16>;
+  def : VOPSelectPat_fake16 <i16>;
+} // End True16Predicate = UseFakeTrue16Insts
 
 let AddedComplexity = 1 in {
 def : GCNPat <
diff --git a/llvm/lib/Target/AMDGPU/VOP2Instructions.td b/llvm/lib/Target/AMDGPU/VOP2Instructions.td
index 384fec0079a5d9..d9e4fcc53f0c4b 100644
--- a/llvm/lib/Target/AMDGPU/VOP2Instructions.td
+++ b/llvm/lib/Target/AMDGPU/VOP2Instructions.td
@@ -714,6 +714,26 @@ class VOP2e_SGPR<list<ValueType> ArgVT> : VOPProfile<ArgVT> {
 def VOP2e_I32_I32_I32_I1 : VOP2e_SGPR<[i32, i32, i32, i1]>;
 def VOP2e_I16_I16_I16_I1 : VOP2e_SGPR<[i16, i16, i16, i1]>;
 // V_CNDMASK_B16 is VOP3 only
+def VOP2e_I16_I16_I16_I1_true16 : VOP2e_SGPR<[i16, i16, i16, i1]> {
+  let IsTrue16 = 1;
+  let IsRealTrue16 = 1;
+  let HasOpSel = 1;
+  let DstRC64 = getVALUDstForVT<DstVT, 1, 1>.ret;
+  let Src0RC64 = getVOP3SrcForVT<Src0VT, 1/*IsTrue16*/>.ret;
+  let Src1RC64 = getVOP3SrcForVT<Src1VT, 1/*IsTrue16*/>.ret;
+  let Src2RC64 = getVOP3SrcForVT<Src2VT, 1/*IsTrue16*/>.ret;
+  let Src0Mod = getSrc0Mod<f16, DstVT, 1/*IsTrue16*/, 0/*IsFake16*/>.ret;
+  let Src1Mod = getSrcMod<f16, 1/*IsTrue16*/, 0/*IsFake16*/>.ret;
+  let HasSrc2Mods = 0;
+  let InsVOP3OpSel = getInsVOP3Base<Src0RC64, Src1RC64,
+                    Src2RC64, NumSrcArgs,
+                    HasClamp, 1/*HasModifiers*/, 0/*HasSrc2Mods*/, HasOMod,
+                    Src0Mod, Src1Mod, Src2Mod, 1/*HasOpSel*/>.ret;
+  let Src0VOP3DPP = VGPRSrc_16;
+  let Src1VOP3DPP = getVOP3DPPSrcForVT<Src1VT, 0/*IsFake16*/>.ret;
+  let Src0ModVOP3DPP = getSrc0ModVOP3DPP<f16, DstVT, 0/*IsFake16*/>.ret;
+  let Src1ModVOP3DPP = getSrcModVOP3DPP<f16, 0/*IsFake16*/>.ret;
+}
 def VOP2e_I16_I16_I16_I1_fake16 : VOP2e_SGPR<[i16, i16, i16, i1]> {
   let IsTrue16 = 1;
   let DstRC64 = getVALUDstForVT<DstVT>.ret;
@@ -765,8 +785,8 @@ def VOP_WRITELANE : VOPProfile<[i32, i32, i32, i32]> {
 // VOP2 Instructions
 //===----------------------------------------------------------------------===//
 
-let SubtargetPredicate = isGFX11Plus in
-defm V_CNDMASK_B16 : VOP2eInst <"v_cndmask_b16", VOP2e_I16_I16_I16_I1_fake16>;
+defm V_CNDMASK_B16_t16 : VOP2eInst <"v_cndmask_b16_t16", VOP2e_I16_I16_I16_I1_true16>;
+defm V_CNDMASK_B16_fake16 : VOP2eInst <"v_cndmask_b16_fake16", VOP2e_I16_I16_I16_I1_fake16>;
 defm V_CNDMASK_B32 : VOP2eInst_VOPD <"v_cndmask_b32", VOP2e_I32_I32_I32_I1, 0x9, "v_cndmask_b32">;
 let SubtargetPredicate = HasMadMacF32Insts, isReMaterializable = 1 in
 def V_MADMK_F32 : VOP2_Pseudo <"v_madmk_f32", VOP_MADMK_F32, []>;
@@ -1835,7 +1855,7 @@ defm V_FMAMK_F16           : VOP2Only_Real_MADK_t16_and_fake16_gfx11_gfx12<0x037
 defm V_FMAAK_F16           : VOP2Only_Real_MADK_t16_and_fake16_gfx11_gfx12<0x038, "v_fmaak_f16">;
 
 // VOP3 only.
-defm V_CNDMASK_B16         : VOP3Only_Realtriple_gfx11_gfx12<0x25d>;
+defm V_CNDMASK_B16         : VOP3Only_Realtriple_t16_and_fake16_gfx11_gfx12<0x25d, "v_cndmask_b16">;
 defm V_LDEXP_F32           : VOP3Only_Realtriple_gfx11_gfx12<0x31c>;
 defm V_BFM_B32             : VOP3Only_Realtriple_gfx11_gfx12<0x31d>;
 defm V_BCNT_U32_B32        : VOP3Only_Realtriple_gfx11_gfx12<0x31e>;
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/saddsat.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/saddsat.ll
index e289ee759da158..e27d4372d87be4 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/saddsat.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/saddsat.ll
@@ -5294,15 +5294,15 @@ define amdgpu_ps i128 @s_saddsat_i128(i128 inreg %lhs, i128 inreg %rhs) {
 ; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, s0
 ; GFX11-NEXT:    s_and_b32 s0, 1, s10
 ; GFX11-NEXT:    s_cmp_eq_u64 s[6:7], 0
-; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s2
+; GFX11-NEXT:    v_cmp_ne_u32_e64 s0, 0, s0
 ; GFX11-NEXT:    s_cselect_b32 s1, 1, 0
-; GFX11-NEXT:    v_cmp_ne_u32_e64 vcc_lo, 0, s0
+; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s2
 ; GFX11-NEXT:    s_and_b32 s1, 1, s1
-; GFX11-NEXT:    v_cmp_ne_u32_e64 s0, 0, s1
-; GFX11-NEXT:    v_cndmask_b32_e32 v0, v1, v0, vcc_lo
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, v2, 0, s0
-; GFX11-NEXT:    v_mov_b32_e32 v2, s5
+; GFX11-NEXT:    v_cmp_ne_u32_e64 s1, 0, s1
+; GFX11-NEXT:    v_cndmask_b16 v0, v1, v0, s0
 ; GFX11-NEXT:    s_ashr_i32 s0, s9, 31
+; GFX11-NEXT:    v_cndmask_b16 v1, v2, 0, s1
+; GFX11-NEXT:    v_mov_b32_e32 v2, s5
 ; GFX11-NEXT:    s_add_i32 s1, s0, 0x80000000
 ; GFX11-NEXT:    v_xor_b32_e32 v0, v1, v0
 ; GFX11-NEXT:    v_dual_mov_b32 v1, s4 :: v_dual_and_b32 v0, 1, v0
@@ -5447,20 +5447,20 @@ define amdgpu_ps <4 x float> @saddsat_i128_sv(i128 inreg %lhs, i128 %rhs) {
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v1, vcc_lo, s1, v1, vcc_lo
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v4, vcc_lo, s2, v2, vcc_lo
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v5, vcc_lo, s3, v3, vcc_lo
-; GFX11-NEXT:    v_cmp_gt_u64_e32 vcc_lo, s[0:1], v[0:1]
-; GFX11-NEXT:    v_cndmask_b32_e64 v6, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_gt_i64_e32 vcc_lo, s[2:3], v[4:5]
-; GFX11-NEXT:    v_cndmask_b32_e64 v7, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_gt_i64_e32 vcc_lo, 0, v[2:3]
-; GFX11-NEXT:    v_cndmask_b32_e64 v8, 0, 1, vcc_lo
+; GFX11-NEXT:    v_cmp_gt_u64_e64 s0, s[0:1], v[0:1]
+; GFX11-NEXT:    v_cmp_gt_i64_e64 s1, 0, v[2:3]
 ; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, s[2:3], v[4:5]
-; GFX11-NEXT:    v_cndmask_b32_e32 v6, v7, v6, vcc_lo
-; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, 0, v[2:3]
+; GFX11-NEXT:    v_cndmask_b32_e64 v6, 0, 1, s0
+; GFX11-NEXT:    v_cmp_gt_i64_e64 s0, s[2:3], v[4:5]
+; GFX11-NEXT:    v_cndmask_b32_e64 v7, 0, 1, s0
+; GFX11-NEXT:    v_cmp_eq_u64_e64 s0, 0, v[2:3]
+; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s1
+; GFX11-NEXT:    v_cndmask_b16 v3, v7, v6, vcc_lo
+; GFX11-NEXT:    v_cndmask_b16 v2, v2, 0, s0
+; GFX11-NEXT:    v_xor_b32_e32 v2, v2, v3
 ; GFX11-NEXT:    v_ashrrev_i32_e32 v3, 31, v5
-; GFX11-NEXT:    v_cndmask_b32_e64 v2, v8, 0, vcc_lo
-; GFX11-NEXT:    v_xor_b32_e32 v2, v2, v6
-; GFX11-NEXT:    v_add_nc_u32_e32 v6, 0x80000000, v3
 ; GFX11-NEXT:    v_and_b32_e32 v2, 1, v2
+; GFX11-NEXT:    v_add_nc_u32_e32 v6, 0x80000000, v3
 ; GFX11-NEXT:    v_cmp_ne_u32_e32 vcc_lo, 0, v2
 ; GFX11-NEXT:    v_cndmask_b32_e32 v1, v1, v3, vcc_lo
 ; GFX11-NEXT:    v_cndmask_b32_e32 v0, v0, v3, vcc_lo
@@ -5606,21 +5606,22 @@ define amdgpu_ps <4 x float> @saddsat_i128_vs(i128 %lhs, i128 inreg %rhs) {
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v5, vcc_lo, s1, v1, vcc_lo
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v6, vcc_lo, s2, v2, vcc_lo
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v7, vcc_lo, s3, v3, vcc_lo
-; GFX11-NEXT:    v_cmp_lt_u64_e32 vcc_lo, v[4:5], v[0:1]
+; GFX11-NEXT:    v_cmp_lt_u64_e64 s0, v[4:5], v[0:1]
 ; GFX11-NEXT:    s_cmp_eq_u64 s[2:3], 0
+; GFX11-NEXT:    s_cselect_b32 s1, 1, 0
+; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, v[6:7], v[2:3]
+; GFX11-NEXT:    v_cndmask_b32_e64 v0, 0, 1, s0
+; GFX11-NEXT:    v_cmp_lt_i64_e64 s0, v[6:7], v[2:3]
+; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, s0
+; GFX11-NEXT:    s_and_b32 s0, 1, s1
 ; GFX11-NEXT:    v_cmp_lt_i64_e64 s1, s[2:3], 0
-; GFX11-NEXT:    s_cselect_b32 s0, 1, 0
-; GFX11-NEXT:    s_and_b32 s0, 1, s0
-; GFX11-NEXT:    v_cndmask_b32_e64 v0, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_lt_i64_e32 vcc_lo, v[6:7], v[2:3]
-; GFX11-NEXT:    v_cndmask_b32_e64 v8, 0, 1, s1
 ; GFX11-NEXT:    v_cmp_ne_u32_e64 s0, 0, s0
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, v[6:7], v[2:3]
+; GFX11-NEXT:    v_cndmask_b16 v0, v1, v0, vcc_lo
+; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s1
+; GFX11-NEXT:    v_cndmask_b16 v1, v2, 0, s0
 ; GFX11-NEXT:    v_ashrrev_i32_e32 v2, 31, v7
-; GFX11-NEXT:    v_dual_cndmask_b32 v0, v1, v0 :: v_dual_add_nc_u32 v3, 0x80000000, v2
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, v8, 0, s0
 ; GFX11-NEXT:    v_xor_b32_e32 v0, v1, v0
+; GFX11-NEXT:    v_add_nc_u32_e32 v3, 0x80000000, v2
 ; GFX11-NEXT:    v_and_b32_e32 v0, 1, v0
 ; GFX11-NEXT:    v_cmp_ne_u32_e32 vcc_lo, 0, v0
 ; GFX11-NEXT:    v_cndmask_b32_e32 v1, v5, v2, vcc_lo
@@ -5846,33 +5847,33 @@ define <2 x i128> @v_saddsat_v2i128(<2 x i128> %lhs, <2 x i128> %rhs) {
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v9, vcc_lo, v1, v9, vcc_lo
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v16, vcc_lo, v2, v10, vcc_lo
 ; GFX11-NEXT:    v_add_co_ci_u32_e32 v17, vcc_lo, v3, v11, vcc_lo
-; GFX11-NEXT:    v_cmp_lt_u64_e32 vcc_lo, v[8:9], v[0:1]
-; GFX11-NEXT:    v_cndmask_b32_e64 v0, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_lt_i64_e32 vcc_lo, v[16:17], v[2:3]
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, vcc_lo
+; GFX11-NEXT:    v_cmp_lt_u64_e64 s0, v[8:9], v[0:1]
+; GFX11-NEXT:    v_cmp_gt_i64_e64 s1, 0, v[10:11]
 ; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, v[16:17], v[2:3]
-; GFX11-NEXT:    v_cndmask_b32_e32 v0, v1, v0, vcc_lo
-; GFX11-NEXT:    v_cmp_gt_i64_e32 vcc_lo, 0, v[10:11]
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, vcc_lo
-; GFX11-NEXT:    v_add_co_u32 v12, vcc_lo, v4, v12
-; GFX11-NEXT:    v_add_co_ci_u32_e32 v13, vcc_lo, v5, v13, vcc_lo
-; GFX11-NEXT:    v_add_co_ci_u32_e32 v18, vcc_lo, v6, v14, vcc_lo
-; GFX11-NEXT:    v_add_co_ci_u32_e32 v19, vcc_lo, v7, v15, vcc_lo
-; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, 0, v[10:11]
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, v1, 0, vcc_lo
-; GFX11-NEXT:    v_cmp_lt_u64_e32 vcc_lo, v[12:13], v[4:5]
+; GFX11-NEXT:    v_cndmask_b32_e64 v0, 0, 1, s0
+; GFX11-NEXT:    v_cmp_lt_i64_e64 s0, v[16:17], v[2:3]
+; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s1
+; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, s0
+; GFX11-NEXT:    v_cmp_eq_u64_e64 s0, 0, v[10:11]
+; GFX11-NEXT:    v_add_co_u32 v10, s1, v4, v12
+; GFX11-NEXT:    v_add_co_ci_u32_e64 v11, s1, v5, v13, s1
+; GFX11-NEXT:    v_add_co_ci_u32_e64 v12, s1, v6, v14, s1
+; GFX11-NEXT:    v_cndmask_b16 v0, v1, v0, vcc_lo
+; GFX11-NEXT:    v_cndmask_b16 v1, v2, 0, s0
+; GFX11-NEXT:    v_cmp_lt_u64_e64 s0, v[10:11], v[4:5]
+; GFX11-NEXT:    v_add_co_ci_u32_e64 v13, s1, v7, v15, s1
+; GFX11-NEXT:    v_cmp_gt_i64_e64 s1, 0, v[14:15]
 ; GFX11-NEXT:    v_xor_b32_e32 v0, v1, v0
-; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_lt_i64_e32 vcc_lo, v[18:19], v[6:7]
-; GFX11-NEXT:    v_cndmask_b32_e64 v3, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_gt_i64_e32 vcc_lo, 0, v[14:15]
-; GFX11-NEXT:    v_cndmask_b32_e64 v4, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, v[18:19], v[6:7]
-; GFX11-NEXT:    v_ashrrev_i32_e32 v6, 31, v19
-; GFX11-NEXT:    v_cndmask_b32_e32 v1, v3, v2, vcc_lo
-; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, 0, v[14:15]
+; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s0
+; GFX11-NEXT:    v_cmp_lt_i64_e64 s0, v[12:13], v[6:7]
+; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, v[12:13], v[6:7]
+; GFX11-NEXT:    v_cndmask_b32_e64 v4, 0, 1, s1
+; GFX11-NEXT:    v_ashrrev_i32_e32 v6, 31, v13
+; GFX11-NEXT:    v_cndmask_b32_e64 v3, 0, 1, s0
+; GFX11-NEXT:    v_cmp_eq_u64_e64 s0, 0, v[14:15]
 ; GFX11-NEXT:    v_add_nc_u32_e32 v7, 0x80000000, v6
-; GFX11-NEXT:    v_cndmask_b32_e64 v2, v4, 0, vcc_lo
+; GFX11-NEXT:    v_cndmask_b16 v1, v3, v2, vcc_lo
+; GFX11-NEXT:    v_cndmask_b16 v2, v4, 0, s0
 ; GFX11-NEXT:    v_xor_b32_e32 v1, v2, v1
 ; GFX11-NEXT:    v_ashrrev_i32_e32 v2, 31, v17
 ; GFX11-NEXT:    v_and_b32_e32 v0, 1, v0
@@ -5882,10 +5883,10 @@ define <2 x i128> @v_saddsat_v2i128(<2 x i128> %lhs, <2 x i128> %rhs) {
 ; GFX11-NEXT:    v_cmp_ne_u32_e64 s0, 0, v3
 ; GFX11-NEXT:    v_cndmask_b32_e32 v1, v9, v2, vcc_lo
 ; GFX11-NEXT:    v_dual_cndmask_b32 v2, v16, v2 :: v_dual_cndmask_b32 v3, v17, v4
-; GFX11-NEXT:    v_cndmask_b32_e64 v4, v12, v6, s0
-; GFX11-NEXT:    v_cndmask_b32_e64 v5, v13, v6, s0
-; GFX11-NEXT:    v_cndmask_b32_e64 v6, v18, v6, s0
-; GFX11-NEXT:    v_cndmask_b32_e64 v7, v19, v7, s0
+; GFX11-NEXT:    v_cndmask_b32_e64 v4, v10, v6, s0
+; GFX11-NEXT:    v_cndmask_b32_e64 v5, v11, v6, s0
+; GFX11-NEXT:    v_cndmask_b32_e64 v6, v12, v6, s0
+; GFX11-NEXT:    v_cndmask_b32_e64 v7, v13, v7, s0
 ; GFX11-NEXT:    s_setpc_b64 s[30:31]
   %result = call <2 x i128> @llvm.sadd.sat.v2i128(<2 x i128> %lhs, <2 x i128> %rhs)
   ret <2 x i128> %result
@@ -6243,16 +6244,16 @@ define amdgpu_ps <2 x i128> @s_saddsat_v2i128(<2 x i128> inreg %lhs, <2 x i128>
 ; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, s0
 ; GFX11-NEXT:    s_and_b32 s0, 1, s18
 ; GFX11-NEXT:    s_cmp_eq_u64 s[10:11], 0
-; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s2
+; GFX11-NEXT:    v_cmp_ne_u32_e64 s0, 0, s0
 ; GFX11-NEXT:    s_cselect_b32 s1, 1, 0
-; GFX11-NEXT:    v_cmp_ne_u32_e64 vcc_lo, 0, s0
+; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s2
 ; GFX11-NEXT:    s_and_b32 s1, 1, s1
 ; GFX11-NEXT:    s_ashr_i32 s10, s17, 31
-; GFX11-NEXT:    v_cmp_ne_u32_e64 s0, 0, s1
+; GFX11-NEXT:    v_cmp_ne_u32_e64 s1, 0, s1
 ; GFX11-NEXT:    s_add_i32 s11, s10, 0x80000000
-; GFX11-NEXT:    v_cndmask_b32_e32 v0, v1, v0, vcc_lo
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, v2, 0, s0
+; GFX11-NEXT:    v_cndmask_b16 v0, v1, v0, s0
 ; GFX11-NEXT:    s_add_u32 s0, s4, s12
+; GFX11-NEXT:    v_cndmask_b16 v1, v2, 0, s1
 ; GFX11-NEXT:    s_addc_u32 s1, s5, s13
 ; GFX11-NEXT:    s_addc_u32 s2, s6, s14
 ; GFX11-NEXT:    v_cmp_lt_u64_e64 s4, s[0:1], s[4:5]
@@ -6268,17 +6269,18 @@ define amdgpu_ps <2 x i128> @s_saddsat_v2i128(<2 x i128> inreg %lhs, <2 x i128>
 ; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s4
 ; GFX11-NEXT:    s_and_b32 s4, 1, s12
 ; GFX11-NEXT:    s_cmp_eq_u64 s[14:15], 0
-; GFX11-NEXT:    v_cndmask_b32_e64 v3, 0, 1, s6
+; GFX11-NEXT:    v_cmp_ne_u32_e64 s4, 0, s4
 ; GFX11-NEXT:    s_cselect_b32 s5, 1, 0
-; GFX11-NEXT:    v_cmp_ne_u32_e64 vcc_lo, 0, s4
+; GFX11-NEXT:    v_cndmask_b32_e64 v3, 0, 1, s6
 ; GFX11-NEXT:    s_and_b32 s5, 1, s5
-; GFX11-NEXT:    v_cmp_ne_u32_e64 s4, 0, s5
-; GFX11-NEXT:    v_cndmask_b32_e32 v1, v2, v1, vcc_lo
-; GFX11-NEXT:    v_cndmask_b32_e64 v2, v3, 0, s4
+; GFX11-NEXT:    v_cmp_ne_u32_e64 s5, 0, s5
+; GFX11-NEXT:    v_cndmask_b16 v1, v2, v1, s4
+; GFX11-NEXT:    s_ashr_i32 s4, s3, 31
+; GFX11-NEXT:    s_add_i32 s0, s4, 0x80000000
+; GFX11-NEXT:    v_cndmask_b16 v2, v3, 0, s5
 ; GFX11-NEXT:    v_mov_b32_e32 v3, s8
 ; GFX11-NEXT:    v_cmp_ne_u32_e32 vcc_lo, 0, v0
 ; GFX11-NEXT:    v_mov_b32_e32 v0, s16
-; GFX11-NEXT:    s_ashr_i32 s4, s3, 31
 ; GFX11-NEXT:    v_xor_b32_e32 v1, v2, v1
 ; GFX11-NEXT:    v_mov_b32_e32 v4, s9
 ; GFX11-NEXT:    v_mov_b32_e32 v2, s17
@@ -6287,7 +6289,6 @@ define amdgpu_ps <2 x i128> @s_saddsat_v2i128(<2 x i128> inreg %lhs, <2 x i128>
 ; GFX11-NEXT:    v_and_b32_e32 v1, 1, v1
 ; GFX11-NEXT:    v_cndmask_b32_e64 v4, v4, s10, vcc_lo
 ; GFX11-NEXT:    v_cndmask_b32_e64 v2, v2, s11, vcc_lo
-; GFX11-NEXT:    s_add_i32 s0, s4, 0x80000000
 ; GFX11-NEXT:    v_cmp_ne_u32_e32 vcc_lo, 0, v1
 ; GFX11-NEXT:    v_mov_b32_e32 v1, s2
 ; GFX11-NEXT:    v_readfirstlane_b32 s1, v4
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/ssubsat.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/ssubsat.ll
index 43ebe156eb2a28..af96da1bb25adf 100644
--- a/llvm/test/CodeGen/AMDGPU/GlobalISel/ssubsat.ll
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/ssubsat.ll
@@ -5297,28 +5297,28 @@ define amdgpu_ps i128 @s_ssubsat_i128(i128 inreg %lhs, i128 inreg %rhs) {
 ; GFX11-NEXT:    s_sub_u32 s8, s0, s4
 ; GFX11-NEXT:    s_subb_u32 s9, s1, s5
 ; GFX11-NEXT:    s_subb_u32 s10, s2, s6
-; GFX11-NEXT:    v_cmp_lt_u64_e64 s0, s[8:9], s[0:1]
 ; GFX11-NEXT:    s_subb_u32 s11, s3, s7
+; GFX11-NEXT:    v_cmp_lt_u64_e64 s0, s[8:9], s[0:1]
 ; GFX11-NEXT:    s_cmp_eq_u64 s[10:11], s[2:3]
+; GFX11-NEXT:    v_cmp_lt_i64_e64 s1, s[10:11], s[2:3]
+; GFX11-NEXT:    v_cmp_gt_u64_e64 s2, s[4:5], 0
 ; GFX11-NEXT:    s_cselect_b32 s12, 1, 0
 ; GFX11-NEXT:    v_cndmask_b32_e64 v0, 0, 1, s0
-; GFX11-NEXT:    v_cmp_lt_i64_e64 s0, s[10:11], s[2:3]
-; GFX11-NEXT:    v_cmp_gt_u64_e64 s2, s[4:5], 0
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, s0
 ; GFX11-NEXT:    s_and_b32 s0, 1, s12
 ; GFX11-NEXT:    s_cmp_eq_u64 s[6:7], 0
 ; GFX11-NEXT:    v_cndmask_b32_e64 v2, 0, 1, s2
 ; GFX11-NEXT:    v_cmp_gt_i64_e64 s2, s[6:7], 0
-; GFX11-NEXT:    v_cmp_ne_u32_e64 vcc_lo, 0, s0
+; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, s1
 ; GFX11-NEXT:    s_cselect_b32 s1, 1, 0
-; GFX11-NEXT:    s_ashr_i32 s0, s11, 31
+; GFX11-NEXT:    v_cmp_ne_u32_e64 s0, 0, s0
 ; GFX11-NEXT:    s_and_b32 s1, 1, s1
 ; GFX11-NEXT:    v_cndmask_b32_e64 v3, 0, 1, s2
-; GFX11-NEXT:    v_cndmask_b32_e32 v0, v1, v0, vcc_lo
-; GFX11-NEXT:    v_cmp_ne_u32_e64 vcc_lo, 0, s1
+; GFX11-NEXT:    v_cmp_ne_u32_e64 s1, 0, s1
+; GFX11-NEXT:    v_cndmask_b16 v0, v1, v0, s0
+; GFX11-NEXT:    s_ashr_i32 s0, s11, 31
+; GFX11-NEXT:    v_cndmask_b16 v1, v3, v2, s1
+; GFX11-NEXT:    v_dual_mov_b32 v2, s9 :: v_dual_mov_b32 v3, s11
 ; GFX11-NEXT:    s_add_i32 s1, s0, 0x80000000
-; GFX11-NEXT:    v_dual_cndmask_b32 v1, v3, v2 :: v_dual_mov_b32 v2, s9
-; GFX11-NEXT:    v_mov_b32_e32 v3, s11
 ; GFX11-NEXT:    v_xor_b32_e32 v0, v1, v0
 ; GFX11-NEXT:    v_dual_mov_b32 v1, s8 :: v_dual_and_b32 v0, 1, v0
 ; GFX11-NEXT:    v_cmp_ne_u32_e32 vcc_lo, 0, v0
@@ -5470,25 +5470,26 @@ define amdgpu_ps <4 x float> @ssubsat_i128_sv(i128 inreg %lhs, i128 %rhs) {
 ; GFX11-NEXT:    v_sub_co_ci_u32_e32 v5, vcc_lo, s1, v1, vcc_lo
 ; GFX11-NEXT:    v_sub_co_ci_u32_e32 v6, vcc_lo, s2, v2, vcc_lo
 ; GFX11-NEXT:    v_sub_co_ci_u32_e32 v7, vcc_lo, s3, v3, vcc_lo
-; GFX11-NEXT:    v_cmp_gt_u64_e32 vcc_lo, s[0:1], v[4:5]
-; GFX11-NEXT:    v_cndmask_b32_e64 v8, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_gt_i64_e32 vcc_lo, s[2:3], v[6:7]
-; GFX11-NEXT:    v_cndmask_b32_e64 v9, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_lt_u64_e32 vcc_lo, 0, v[0:1]
-; GFX11-NEXT:    v_cndmask_b32_e64 v0, 0, 1, vcc_lo
-; GFX11-NEXT:    v_cmp_lt_i64_e32 vcc_lo, 0, v[2:3]
-; GFX11-NEXT:    v_cndmask_b32_e64 v1, 0, 1, vcc_lo
+; GFX11-NEXT:    v_cmp_gt_u64_e64 s0, s[0:1], v[4:5]
+; GFX11-NEXT:    v_cmp_lt_u64_e64 s1, 0, v[0:1]
 ; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, s[2:3], v[6:7]
-; GFX11-NEXT:    v_cndmask_b32_e32 v8, v9, v8, vcc_lo
-; GFX11-NEXT:    v_cmp_eq_u64_e32 vcc_lo, 0, v[2:3]
+; GFX11-NEXT:    v_cndmask_b32_e64 v8, 0, 1, s0
+; GFX11-NEXT:    v_cmp_gt_i64_e64 s0, s[2:3], v[6:7]
+; GFX11-NEXT:    v_cndmask_b32_e64 v0, 0, 1, s1
+; GFX11-NEXT:...
[truncated]

def : VOPSelectPat <f16>;
def : VOPSelectPat <i16>;
let True16Predicate = NotHasTrue16BitInsts in {
def : VOPSelectPat <f16>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do source modifiers work with V_CNDMASK_B16? We should have the source modifier patterns here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Matt. V_CNDMASK_B16 has a operand type of I16_I16_I16_I1 thus it does not has source modifier in Pre-GFX11. Do it make sense to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

This wasn't a question of how the instruction is encoded in tablegen, but does the underlying instruction support them. i.e. is tablegen wrong and it should have them

Copy link
Contributor Author

@broxigarchen broxigarchen Jan 3, 2025

Choose a reason for hiding this comment

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

I see. Sorry I was confused at the beginning.

In Pre-GFX11 i16/f16 are matched to v_cndmask_b32 and in post-gfx11 they are matched to v_cndmask_b16. Both instruction support omod/abs/neg/opsel srcmod. So yes I think we should have srcmod pattern for f16/i16 both pre-GFX11 and post-GFX11.

When I checked the old commit here d2a9739#diff-bfd398f33980e2dad30c39c83b325066633fe6c107982e6d1207d070580940d3R905 It seems set the srcmod to be zero for 16bits type. Is there a reason behind it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed it looks like cndmask_b32 and cndmask_b16 can support those modifiers. I'd recommend filing an issue and adding the support in a follow up patch if it is useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed an issue for this and will address it in later patch

@broxigarchen broxigarchen force-pushed the main-merge-true16-vop3-mc-more-instructions-8 branch from fd996b4 to 83cdd94 Compare January 3, 2025 00:20
@broxigarchen
Copy link
Contributor Author

Added a GFX12 runline for codegen test


let SubtargetPredicate = isGFX11Plus in
defm V_CNDMASK_B16 : VOP2eInst <"v_cndmask_b16", VOP2e_I16_I16_I16_I1_fake16>;
defm V_CNDMASK_B16_t16 : VOP2eInst <"v_cndmask_b16_t16", VOP2e_I16_I16_I16_I1_true16>;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have SubtargetPredicate and True16Predicate on these instructions. I guess the codegen works since the selection patterns have the predicates, but MC probably needs them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

def : VOPSelectPat <f16>;
def : VOPSelectPat <i16>;
let True16Predicate = NotHasTrue16BitInsts in {
def : VOPSelectPat <f16>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed it looks like cndmask_b32 and cndmask_b16 can support those modifiers. I'd recommend filing an issue and adding the support in a follow up patch if it is useful.

@broxigarchen broxigarchen force-pushed the main-merge-true16-vop3-mc-more-instructions-8 branch from 83cdd94 to 39803e1 Compare January 13, 2025 21:59
@broxigarchen broxigarchen force-pushed the main-merge-true16-vop3-mc-more-instructions-8 branch from 39803e1 to 8cc493c Compare January 13, 2025 22:01
@broxigarchen
Copy link
Contributor Author

rebased and resolve conflicts

@broxigarchen
Copy link
Contributor Author

Windows test passing. Linux failure seems to be a github server issue

@broxigarchen broxigarchen requested a review from Sisyph January 14, 2025 18:06
} // End True16Predicate = UseRealTrue16Insts
let True16Predicate = UseFakeTrue16Insts in {
def : VOPSelectPat_fake16 <f16>;
def : VOPSelectPat_fake16 <i16>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might want to continue using cndmask_b32 for Fake16. The reason being that we have VOPD cndmask_b32, but no VOPD cndmask_b16. So there is in fact a performance penalty, and no register saving upside that we have in real true16 mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Updated

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

@broxigarchen broxigarchen requested a review from kosarev January 16, 2025 21:19
@broxigarchen broxigarchen merged commit 8a0c2e7 into llvm:main Jan 16, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants