[llvm] 16e1f8e - Revert "[AMDGPU] Select v_sat_pk_u8_i16"
via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 31 03:40:03 PDT 2023
Author: pvanhout
Date: 2023-03-31T12:39:49+02:00
New Revision: 16e1f8e970aad4c92b31b330daf7274e89cca98d
URL: https://github.com/llvm/llvm-project/commit/16e1f8e970aad4c92b31b330daf7274e89cca98d
DIFF: https://github.com/llvm/llvm-project/commit/16e1f8e970aad4c92b31b330daf7274e89cca98d.diff
LOG: Revert "[AMDGPU] Select v_sat_pk_u8_i16"
This reverts commit 64b45db34a0cd979dae9ca3016e9da517e57b987.
Reason: the patterns are wrong which can result in a miscompilation.
However, fixing the pattern is not trivial due to how i8 values
are handled, and due to the additional type-checking performed by
D147127: trunc/smax/smin are all defined as int ops in the DAG
despite them working on vectors too.
As this is not a much-needed pattern, I prefer reverting for now
until I can find time to properly rewrite the pattern.
Added:
Modified:
llvm/lib/Target/AMDGPU/AMDGPUInstructions.td
llvm/lib/Target/AMDGPU/SIInstructions.td
llvm/test/CodeGen/AMDGPU/v_sat_pk_u8_i16.ll
Removed:
################################################################################
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUInstructions.td b/llvm/lib/Target/AMDGPU/AMDGPUInstructions.td
index 4df9ab5cf41a0..a7a87e866c9b8 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstructions.td
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstructions.td
@@ -282,10 +282,6 @@ def srl_16 : PatFrag<
(ops node:$src0), (srl_oneuse node:$src0, (i32 16))
>;
-def clamp_s16_u8 : PatFrag<
- (ops node:$src),
- (i16 (AMDGPUsmed3 $src, (i16 0), (i16 255)))
->;
def hi_i16_elt : PatFrag<
(ops node:$src0), (i16 (trunc (i32 (srl_16 node:$src0))))
diff --git a/llvm/lib/Target/AMDGPU/SIInstructions.td b/llvm/lib/Target/AMDGPU/SIInstructions.td
index 5769f355a1d04..3821da0175769 100644
--- a/llvm/lib/Target/AMDGPU/SIInstructions.td
+++ b/llvm/lib/Target/AMDGPU/SIInstructions.td
@@ -10,10 +10,9 @@
// that are not yet supported remain commented out.
//===----------------------------------------------------------------------===//
-class GCNPat<dag pattern, dag result> : Pat<pattern, result>, GCNPredicateControl, GISelFlags;
+class GCNPat<dag pattern, dag result> : Pat<pattern, result>, GCNPredicateControl {
-let GIIgnoreCopies = 1 in
-class GCNPatIgnoreCopies<dag pattern, dag result> : GCNPat<pattern, result>;
+}
class UniformSextInreg<ValueType VT> : PatFrag<
(ops node:$src),
@@ -2930,31 +2929,6 @@ def : GCNPat <
(v2i16 (V_LSHL_OR_B32_e64 $src1, (i32 16), (i32 (V_AND_B32_e64 (i32 (V_MOV_B32_e32 (i32 0xffff))), $src0))))
>;
-multiclass V_SAT_PK_Pat<Instruction inst> {
- def: GCNPat<
- (v2i16 (DivergentBinFrag<build_vector> (clamp_s16_u8 i16:$lo), (clamp_s16_u8 i16:$hi))),
- (inst
- (V_LSHL_OR_B32_e64 VGPR_32:$hi, (S_MOV_B32 (i32 16)),
- (V_AND_B32_e64 VGPR_32:$lo, (S_MOV_B32 (i32 0xFFFF)))))
- >;
-
- def: GCNPatIgnoreCopies<
- (v2i16 (DivergentBinFrag<smin> (smax v2i16:$src, (build_vector (i16 0), (i16 0))), (build_vector (i16 255), (i16 255)))),
- (inst VGPR_32:$src)
- >;
-
- def: GCNPatIgnoreCopies<
- (v2i16 (DivergentBinFrag<smax> (smin v2i16:$src, (build_vector (i16 255), (i16 255))), (build_vector (i16 0), (i16 0)))),
- (inst VGPR_32:$src)
- >;
-}
-
-let OtherPredicates = [HasTrue16BitInsts] in
-defm : V_SAT_PK_Pat<V_SAT_PK_U8_I16_t16_e64>;
-
-let OtherPredicates = [NotHasTrue16BitInsts] in
-defm : V_SAT_PK_Pat<V_SAT_PK_U8_I16_e64>;
-
// With multiple uses of the shift, this will duplicate the shift and
// increase register pressure.
def : GCNPat <
diff --git a/llvm/test/CodeGen/AMDGPU/v_sat_pk_u8_i16.ll b/llvm/test/CodeGen/AMDGPU/v_sat_pk_u8_i16.ll
index 32eac94e0218b..4018eabd4657e 100644
--- a/llvm/test/CodeGen/AMDGPU/v_sat_pk_u8_i16.ll
+++ b/llvm/test/CodeGen/AMDGPU/v_sat_pk_u8_i16.ll
@@ -30,19 +30,21 @@ define <2 x i16> @basic_smax_smin(i16 %src0, i16 %src1) {
; GFX9-LABEL: basic_smax_smin:
; GFX9: ; %bb.0:
; GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX9-NEXT: v_and_b32_e32 v0, 0xffff, v0
-; GFX9-NEXT: v_lshl_or_b32 v0, v1, 16, v0
-; GFX9-NEXT: v_sat_pk_u8_i16_e32 v0, v0
+; GFX9-NEXT: v_mov_b32_e32 v2, 0xff
+; GFX9-NEXT: v_med3_i16 v0, v0, 0, v2
+; GFX9-NEXT: v_med3_i16 v1, v1, 0, v2
+; GFX9-NEXT: s_mov_b32 s4, 0x5040100
+; GFX9-NEXT: v_perm_b32 v0, v1, v0, s4
; GFX9-NEXT: s_setpc_b64 s[30:31]
;
; GFX11-LABEL: basic_smax_smin:
; GFX11: ; %bb.0:
; GFX11-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX11-NEXT: v_and_b32_e32 v0, 0xffff, v0
-; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
-; GFX11-NEXT: v_lshl_or_b32 v0, v1, 16, v0
-; GFX11-NEXT: v_sat_pk_u8_i16_e32 v0, v0
+; GFX11-NEXT: v_med3_i16 v0, v0, 0, 0xff
+; GFX11-NEXT: v_med3_i16 v1, v1, 0, 0xff
+; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GFX11-NEXT: v_perm_b32 v0, v1, v0, 0x5040100
; GFX11-NEXT: s_setpc_b64 s[30:31]
;
; GISEL-VI-LABEL: basic_smax_smin:
@@ -200,19 +202,21 @@ define <2 x i16> @basic_smin_smax(i16 %src0, i16 %src1) {
; GFX9-LABEL: basic_smin_smax:
; GFX9: ; %bb.0:
; GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX9-NEXT: v_and_b32_e32 v0, 0xffff, v0
-; GFX9-NEXT: v_lshl_or_b32 v0, v1, 16, v0
-; GFX9-NEXT: v_sat_pk_u8_i16_e32 v0, v0
+; GFX9-NEXT: v_mov_b32_e32 v2, 0xff
+; GFX9-NEXT: v_med3_i16 v0, v0, 0, v2
+; GFX9-NEXT: v_med3_i16 v1, v1, 0, v2
+; GFX9-NEXT: s_mov_b32 s4, 0x5040100
+; GFX9-NEXT: v_perm_b32 v0, v1, v0, s4
; GFX9-NEXT: s_setpc_b64 s[30:31]
;
; GFX11-LABEL: basic_smin_smax:
; GFX11: ; %bb.0:
; GFX11-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX11-NEXT: v_and_b32_e32 v0, 0xffff, v0
-; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
-; GFX11-NEXT: v_lshl_or_b32 v0, v1, 16, v0
-; GFX11-NEXT: v_sat_pk_u8_i16_e32 v0, v0
+; GFX11-NEXT: v_med3_i16 v0, v0, 0, 0xff
+; GFX11-NEXT: v_med3_i16 v1, v1, 0, 0xff
+; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GFX11-NEXT: v_perm_b32 v0, v1, v0, 0x5040100
; GFX11-NEXT: s_setpc_b64 s[30:31]
;
; GISEL-VI-LABEL: basic_smin_smax:
@@ -249,19 +253,21 @@ define <2 x i16> @basic_smin_smax_combined(i16 %src0, i16 %src1) {
; GFX9-LABEL: basic_smin_smax_combined:
; GFX9: ; %bb.0:
; GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX9-NEXT: v_and_b32_e32 v0, 0xffff, v0
-; GFX9-NEXT: v_lshl_or_b32 v0, v1, 16, v0
-; GFX9-NEXT: v_sat_pk_u8_i16_e32 v0, v0
+; GFX9-NEXT: v_mov_b32_e32 v2, 0xff
+; GFX9-NEXT: v_med3_i16 v0, v0, 0, v2
+; GFX9-NEXT: v_med3_i16 v1, v1, 0, v2
+; GFX9-NEXT: s_mov_b32 s4, 0x5040100
+; GFX9-NEXT: v_perm_b32 v0, v1, v0, s4
; GFX9-NEXT: s_setpc_b64 s[30:31]
;
; GFX11-LABEL: basic_smin_smax_combined:
; GFX11: ; %bb.0:
; GFX11-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX11-NEXT: v_and_b32_e32 v0, 0xffff, v0
-; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1) | instskip(NEXT) | instid1(VALU_DEP_1)
-; GFX11-NEXT: v_lshl_or_b32 v0, v1, 16, v0
-; GFX11-NEXT: v_sat_pk_u8_i16_e32 v0, v0
+; GFX11-NEXT: v_med3_i16 v0, v0, 0, 0xff
+; GFX11-NEXT: v_med3_i16 v1, v1, 0, 0xff
+; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GFX11-NEXT: v_perm_b32 v0, v1, v0, 0x5040100
; GFX11-NEXT: s_setpc_b64 s[30:31]
;
; GISEL-VI-LABEL: basic_smin_smax_combined:
@@ -296,17 +302,21 @@ define <2 x i16> @vec_smax_smin(<2 x i16> %src) {
; SDAG-VI-NEXT: v_or_b32_e32 v0, v0, v1
; SDAG-VI-NEXT: s_setpc_b64 s[30:31]
;
-; GFX9-LABEL: vec_smax_smin:
-; GFX9: ; %bb.0:
-; GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX9-NEXT: v_sat_pk_u8_i16_e32 v0, v0
-; GFX9-NEXT: s_setpc_b64 s[30:31]
+; SDAG-GFX9-LABEL: vec_smax_smin:
+; SDAG-GFX9: ; %bb.0:
+; SDAG-GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; SDAG-GFX9-NEXT: v_pk_max_i16 v0, v0, 0
+; SDAG-GFX9-NEXT: s_movk_i32 s4, 0xff
+; SDAG-GFX9-NEXT: v_pk_min_i16 v0, v0, s4 op_sel_hi:[1,0]
+; SDAG-GFX9-NEXT: s_setpc_b64 s[30:31]
;
; GFX11-LABEL: vec_smax_smin:
; GFX11: ; %bb.0:
; GFX11-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX11-NEXT: v_sat_pk_u8_i16_e32 v0, v0
+; GFX11-NEXT: v_pk_max_i16 v0, v0, 0
+; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GFX11-NEXT: v_pk_min_i16 v0, 0xff, v0 op_sel_hi:[0,1]
; GFX11-NEXT: s_setpc_b64 s[30:31]
;
; GISEL-VI-LABEL: vec_smax_smin:
@@ -320,6 +330,14 @@ define <2 x i16> @vec_smax_smin(<2 x i16> %src) {
; GISEL-VI-NEXT: v_min_i16_sdwa v0, v0, v2 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:DWORD
; GISEL-VI-NEXT: v_or_b32_e32 v0, v1, v0
; GISEL-VI-NEXT: s_setpc_b64 s[30:31]
+;
+; GISEL-GFX9-LABEL: vec_smax_smin:
+; GISEL-GFX9: ; %bb.0:
+; GISEL-GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GISEL-GFX9-NEXT: v_pk_max_i16 v0, v0, 0
+; GISEL-GFX9-NEXT: v_mov_b32_e32 v1, 0xff00ff
+; GISEL-GFX9-NEXT: v_pk_min_i16 v0, v0, v1
+; GISEL-GFX9-NEXT: s_setpc_b64 s[30:31]
%src.max = call <2 x i16> @llvm.smax.v2i16(<2 x i16> %src, <2 x i16> <i16 0, i16 0>)
%src.clamp = call <2 x i16> @llvm.smin.v2i16(<2 x i16> %src.max, <2 x i16> <i16 255, i16 255>)
ret <2 x i16> %src.clamp
@@ -462,17 +480,21 @@ define <2 x i16> @vec_smin_smax(<2 x i16> %src) {
; SDAG-VI-NEXT: v_or_b32_e32 v0, v0, v1
; SDAG-VI-NEXT: s_setpc_b64 s[30:31]
;
-; GFX9-LABEL: vec_smin_smax:
-; GFX9: ; %bb.0:
-; GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX9-NEXT: v_sat_pk_u8_i16_e32 v0, v0
-; GFX9-NEXT: s_setpc_b64 s[30:31]
+; SDAG-GFX9-LABEL: vec_smin_smax:
+; SDAG-GFX9: ; %bb.0:
+; SDAG-GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; SDAG-GFX9-NEXT: s_movk_i32 s4, 0xff
+; SDAG-GFX9-NEXT: v_pk_min_i16 v0, v0, s4 op_sel_hi:[1,0]
+; SDAG-GFX9-NEXT: v_pk_max_i16 v0, v0, 0
+; SDAG-GFX9-NEXT: s_setpc_b64 s[30:31]
;
; GFX11-LABEL: vec_smin_smax:
; GFX11: ; %bb.0:
; GFX11-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GFX11-NEXT: s_waitcnt_vscnt null, 0x0
-; GFX11-NEXT: v_sat_pk_u8_i16_e32 v0, v0
+; GFX11-NEXT: v_pk_min_i16 v0, 0xff, v0 op_sel_hi:[0,1]
+; GFX11-NEXT: s_delay_alu instid0(VALU_DEP_1)
+; GFX11-NEXT: v_pk_max_i16 v0, v0, 0
; GFX11-NEXT: s_setpc_b64 s[30:31]
;
; GISEL-VI-LABEL: vec_smin_smax:
@@ -486,6 +508,14 @@ define <2 x i16> @vec_smin_smax(<2 x i16> %src) {
; GISEL-VI-NEXT: v_max_i16_sdwa v0, v0, v2 dst_sel:WORD_1 dst_unused:UNUSED_PAD src0_sel:DWORD src1_sel:DWORD
; GISEL-VI-NEXT: v_or_b32_e32 v0, v1, v0
; GISEL-VI-NEXT: s_setpc_b64 s[30:31]
+;
+; GISEL-GFX9-LABEL: vec_smin_smax:
+; GISEL-GFX9: ; %bb.0:
+; GISEL-GFX9-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; GISEL-GFX9-NEXT: v_mov_b32_e32 v1, 0xff00ff
+; GISEL-GFX9-NEXT: v_pk_min_i16 v0, v0, v1
+; GISEL-GFX9-NEXT: v_pk_max_i16 v0, v0, 0
+; GISEL-GFX9-NEXT: s_setpc_b64 s[30:31]
%src.min = call <2 x i16> @llvm.smin.v2i16(<2 x i16> %src, <2 x i16> <i16 255, i16 255>)
%src.clamp = call <2 x i16> @llvm.smax.v2i16(<2 x i16> %src.min, <2 x i16> <i16 0, i16 0>)
ret <2 x i16> %src.clamp
More information about the llvm-commits
mailing list