Hi! On 2024-01-29T11:34:05+0100, Tobias Burnus <tbur...@baylibre.com> wrote: > Andrew wrote off list: > "Vector reductions don't work on RDNA, as is, but they're > supposed to be disabled by the insn condition" > > This patch disables "fold_left_plus_<mode>", which is about > vectorization and in the code path shown in the backtrace. > I can also confirm manually that it fixes the ICE I saw and > also the ICE for the testfile that Richard's PR shows at the > end of his backtrace. (-O3 is needed to trigger the ICE.)
On top of that, OK to push the attached "GCN: Conditionalize 'define_expand "reduc_<fexpander>_scal_<mode>"' on '!TARGET_RDNA2_PLUS' [PR113615]"? Which of the 'assert's are worth keeping? Only tested 'vect.exp' for 'check-gcc-c' so far; full testing to run later. Please confirm I'm understanding this correctly: Andrew's original commit c7ec7bd1c6590cf4eed267feab490288e0b8d691 "amdgcn: add -march=gfx1030 EXPERIMENTAL" did this: (define_expand "reduc_<reduc_op>_scal_<mode>" [(set (match_operand:<SCALAR_MODE> 0 "register_operand") (unspec:<SCALAR_MODE> [(match_operand:V_ALL 1 "register_operand")] REDUC_UNSPEC))] - "" + "!TARGET_RDNA2" [later '!TARGET_RDNA2_PLUS'] { [...] This conditional, however, does *not* govern any explicit 'gen_reduc_plus_scal_<mode>', and therefore Tobias in commit 7cc2262ec9a410dc56d1c1c6b950c922e14f621d "gcn/gcn-valu.md: Disable fold_left_plus for TARGET_RDNA2_PLUS [PR113615]" had to replicate the '!TARGET_RDNA2_PLUS' condition: > @@ -4274,7 +4274,8 @@ (define_expand "fold_left_plus_<mode>" > [(match_operand:<SCALAR_MODE> 0 "register_operand") > (match_operand:<SCALAR_MODE> 1 "gcn_alu_operand") > (match_operand:V_FP 2 "gcn_alu_operand")] > - "can_create_pseudo_p () > + "!TARGET_RDNA2_PLUS > + && can_create_pseudo_p () > && (flag_openacc || flag_openmp > || flag_associative_math)" > { | rtx dest = operands[0]; | rtx scalar = operands[1]; | rtx vector = operands[2]; | rtx tmp = gen_reg_rtx (<SCALAR_MODE>mode); | | emit_insn (gen_reduc_plus_scal_<mode> (tmp, vector)); | [...] ..., and I thus now have to do similar for 'gen_reduc_<expander>_scal_<mode>' use in here: (define_expand "reduc_<fexpander>_scal_<mode>" [(match_operand:<SCALAR_MODE> 0 "register_operand") (fminmaxop:V_FP (match_operand:V_FP 1 "register_operand"))] - "" + "!TARGET_RDNA2_PLUS" { /* fmin/fmax are identical to smin/smax. */ emit_insn (gen_reduc_<expander>_scal_<mode> (operands[0], operands[1])); [...] Grüße Thomas
>From 1ca37da07f0fd3fa2e87fcbde9f2c2aadbe320dc Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <tschwi...@baylibre.com> Date: Fri, 16 Feb 2024 13:04:00 +0100 Subject: [PATCH] GCN: Conditionalize 'define_expand "reduc_<fexpander>_scal_<mode>"' on '!TARGET_RDNA2_PLUS' [PR113615] On top of commit c7ec7bd1c6590cf4eed267feab490288e0b8d691 "amdgcn: add -march=gfx1030 EXPERIMENTAL" conditionalizing 'define_expand "reduc_<reduc_op>_scal_<mode>"' on '!TARGET_RDNA2' (later: '!TARGET_RDNA2_PLUS'), we then did similar in commit 7cc2262ec9a410dc56d1c1c6b950c922e14f621d "gcn/gcn-valu.md: Disable fold_left_plus for TARGET_RDNA2_PLUS [PR113615]" to conditionalize 'define_expand "fold_left_plus_<mode>"' on '!TARGET_RDNA2_PLUS', but I found we also need to conditionalize the related 'define_expand "reduc_<fexpander>_scal_<mode>"' on '!TARGET_RDNA2_PLUS', to avoid ICEs like: [...]/gcc.dg/vect/pr108608.c: In function 'foo': [...]/gcc.dg/vect/pr108608.c:9:1: error: unrecognizable insn: (insn 34 33 35 2 (set (reg:V64DF 723) (unspec:V64DF [ (reg:V64DF 690 [ vect_m_11.20 ]) (const_int 1 [0x1]) ] UNSPEC_MOV_DPP_SHR)) -1 (nil)) during RTL pass: vregs Similar for 'gcc.dg/vect/vect-fmax-2.c', 'gcc.dg/vect/vect-fmin-2.c', and 'UNSPEC_SMAX_DPP_SHR' for 'gcc.dg/vect/vect-fmax-1.c', and 'UNSPEC_SMIN_DPP_SHR' for 'gcc.dg/vect/vect-fmin-1.c', when running 'vect.exp' for 'check-gcc-c'. PR target/113615 gcc/ * config/gcn/gcn-valu.md (define_expand "reduc_<fexpander>_scal_<mode>"): Conditionalize on '!TARGET_RDNA2_PLUS'. --- gcc/config/gcn/gcn-valu.md | 6 +++++- gcc/config/gcn/gcn.cc | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/gcc/config/gcn/gcn-valu.md b/gcc/config/gcn/gcn-valu.md index 59e27d0aed79..973a72e3fc41 100644 --- a/gcc/config/gcn/gcn-valu.md +++ b/gcc/config/gcn/gcn-valu.md @@ -4247,6 +4247,8 @@ REDUC_UNSPEC))] "!TARGET_RDNA2_PLUS" { + gcc_checking_assert (!TARGET_RDNA2_PLUS); + rtx tmp = gcn_expand_reduc_scalar (<MODE>mode, operands[1], <reduc_unspec>); @@ -4261,8 +4263,10 @@ [(match_operand:<SCALAR_MODE> 0 "register_operand") (fminmaxop:V_FP (match_operand:V_FP 1 "register_operand"))] - "" + "!TARGET_RDNA2_PLUS" { + gcc_checking_assert (!TARGET_RDNA2_PLUS); + /* fmin/fmax are identical to smin/smax. */ emit_insn (gen_reduc_<expander>_scal_<mode> (operands[0], operands[1])); DONE; diff --git a/gcc/config/gcn/gcn.cc b/gcc/config/gcn/gcn.cc index fce2d4d30c9d..8fa445deda53 100644 --- a/gcc/config/gcn/gcn.cc +++ b/gcc/config/gcn/gcn.cc @@ -5449,6 +5449,8 @@ char * gcn_expand_dpp_shr_insn (machine_mode mode, const char *insn, int unspec, int shift) { + gcc_checking_assert (!TARGET_RDNA2_PLUS); + static char buf[128]; const char *dpp; const char *vcc_in = ""; @@ -5510,6 +5512,8 @@ gcn_expand_dpp_shr_insn (machine_mode mode, const char *insn, rtx gcn_expand_reduc_scalar (machine_mode mode, rtx src, int unspec) { + gcc_checking_assert (!TARGET_RDNA2_PLUS); + machine_mode orig_mode = mode; machine_mode scalar_mode = GET_MODE_INNER (mode); int vf = GET_MODE_NUNITS (mode); -- 2.43.0