Message ID | 20231205081248.2106-5-gaofei@eswincomputing.com |
---|---|
State | New |
Headers | show |
Series | [1/5,V3,ifcvt] optimize x=c ? (y op z) : y by RISC-V Zicond like insns | expand |
On 12/5/23 01:12, Fei Gao wrote: > SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered > to support SImode in 64-bit machine. > > Co-authored-by: Xiao Zeng<zengxiao@eswincomputing.com> > > gcc/ChangeLog: > > * ifcvt.cc (noce_cond_zero_binary_op_supported): add support for extension > (noce_bbs_ok_for_cond_zero_arith): likewise > (noce_try_cond_zero_arith): support extension of LSHIFTRT case > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/zicond_ifcvt_opt.c: add TCs for extension So I think this needs to defer to gcc-15. But even so I think getting some review on the effort is useful. > --- > gcc/ifcvt.cc | 16 ++- > .../gcc.target/riscv/zicond_ifcvt_opt.c | 126 +++++++++++++++++- > 2 files changed, 139 insertions(+), 3 deletions(-) > > diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc > index b84be53ec5c..306497a8e37 100644 > --- a/gcc/ifcvt.cc > +++ b/gcc/ifcvt.cc > @@ -2934,6 +2934,10 @@ noce_cond_zero_binary_op_supported (rtx op) > { > enum rtx_code opcode = GET_CODE (op); > > + /* Strip SIGN_EXTEND or ZERO_EXTEND if any. */ > + if (opcode == SIGN_EXTEND || opcode == ZERO_EXTEND) > + opcode = GET_CODE (XEXP (op, 0)); So it seems to me like that you need to record what the extension was so that you can re-apply it to the result. > @@ -3114,7 +3122,11 @@ noce_try_cond_zero_arith (struct noce_if_info *if_info) > if (CONST_INT_P (*to_replace)) > { > if (noce_cond_zero_shift_op_supported (bin_code)) > - *to_replace = gen_rtx_SUBREG (E_QImode, target, 0); > + { > + *to_replace = gen_rtx_SUBREG (E_QImode, target, 0); > + if (GET_CODE (a) == ZERO_EXTEND && bin_code == LSHIFTRT) > + PUT_CODE (a, SIGN_EXTEND); > + } This doesn't look correct (ignoring the SUBREG issues with patch #4 in this series). When we looked at this internally the conclusion was we needed to first strip the extension, recording what kind of extension it was, then reapply the same extension to the result of the now conditional operation. And it's independent of SUBREG handling. Jeff
On 2023-12-11 13:46 Jeff Law <jeffreyalaw@gmail.com> wrote: > > > >On 12/5/23 01:12, Fei Gao wrote: >> SIGN_EXTEND, ZERO_EXTEND and SUBREG has been considered >> to support SImode in 64-bit machine. >> >> Co-authored-by: Xiao Zeng<zengxiao@eswincomputing.com> >> >> gcc/ChangeLog: >> >> * ifcvt.cc (noce_cond_zero_binary_op_supported): add support for extension >> (noce_bbs_ok_for_cond_zero_arith): likewise >> (noce_try_cond_zero_arith): support extension of LSHIFTRT case >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/zicond_ifcvt_opt.c: add TCs for extension >So I think this needs to defer to gcc-15. But even so I think getting >some review on the effort is useful. > > >> --- >> gcc/ifcvt.cc | 16 ++- >> .../gcc.target/riscv/zicond_ifcvt_opt.c | 126 +++++++++++++++++- >> 2 files changed, 139 insertions(+), 3 deletions(-) >> >> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc >> index b84be53ec5c..306497a8e37 100644 >> --- a/gcc/ifcvt.cc >> +++ b/gcc/ifcvt.cc >> @@ -2934,6 +2934,10 @@ noce_cond_zero_binary_op_supported (rtx op) >> { >> enum rtx_code opcode = GET_CODE (op); >> >> + /* Strip SIGN_EXTEND or ZERO_EXTEND if any. */ >> + if (opcode == SIGN_EXTEND || opcode == ZERO_EXTEND) >> + opcode = GET_CODE (XEXP (op, 0)); >So it seems to me like that you need to record what the extension was so >that you can re-apply it to the result. > >> @@ -3114,7 +3122,11 @@ noce_try_cond_zero_arith (struct noce_if_info *if_info) >> if (CONST_INT_P (*to_replace)) >> { >> if (noce_cond_zero_shift_op_supported (bin_code)) >> - *to_replace = gen_rtx_SUBREG (E_QImode, target, 0); >> + { >> + *to_replace = gen_rtx_SUBREG (E_QImode, target, 0); >> + if (GET_CODE (a) == ZERO_EXTEND && bin_code == LSHIFTRT) >> + PUT_CODE (a, SIGN_EXTEND); >> + } >This doesn't look correct (ignoring the SUBREG issues with patch #4 in >this series). Agree there's issue here for const_int case as you mentioned in [PATCH 4/5] [ifcvt] optimize x=c ? (y op const_int) : y. > >When we looked at this internally the conclusion was we needed to first >strip the extension, recording what kind of extension it was, then >reapply the same extension to the result of the now conditional >operation. And it's independent of SUBREG handling. Ignoring the const_int case, we can reuse the RTL pattern and replace the z(SUBREG pr REG) in INSN_A(x=y op z) without recording what kind of extension it was. New patch will be sent to gcc15. BR, Fei > > >Jeff
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc index b84be53ec5c..306497a8e37 100644 --- a/gcc/ifcvt.cc +++ b/gcc/ifcvt.cc @@ -2934,6 +2934,10 @@ noce_cond_zero_binary_op_supported (rtx op) { enum rtx_code opcode = GET_CODE (op); + /* Strip SIGN_EXTEND or ZERO_EXTEND if any. */ + if (opcode == SIGN_EXTEND || opcode == ZERO_EXTEND) + opcode = GET_CODE (XEXP (op, 0)); + if (opcode == PLUS || opcode == MINUS || opcode == IOR || opcode == XOR || opcode == AND || noce_cond_zero_shift_op_supported (opcode)) return true; @@ -3000,7 +3004,11 @@ noce_bbs_ok_for_cond_zero_arith (struct noce_if_info *if_info, rtx *common_ptr, if (!(noce_cond_zero_binary_op_supported (a) && REG_P (b))) return false; - bin_exp = a; + /* Strip sign_extend if any. */ + if (GET_CODE (a) == SIGN_EXTEND || GET_CODE (a) == ZERO_EXTEND) + bin_exp = XEXP (a, 0); + else + bin_exp = a; /* Canonicalize x = (z op y) : y to x = (y op z) : y */ op1 = get_base_reg (XEXP (bin_exp, 1)); @@ -3114,7 +3122,11 @@ noce_try_cond_zero_arith (struct noce_if_info *if_info) if (CONST_INT_P (*to_replace)) { if (noce_cond_zero_shift_op_supported (bin_code)) - *to_replace = gen_rtx_SUBREG (E_QImode, target, 0); + { + *to_replace = gen_rtx_SUBREG (E_QImode, target, 0); + if (GET_CODE (a) == ZERO_EXTEND && bin_code == LSHIFTRT) + PUT_CODE (a, SIGN_EXTEND); + } else if (SUBREG_P (bin_op0)) *to_replace = gen_rtx_SUBREG (GET_MODE (bin_op0), target, 0); else diff --git a/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c b/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c index 85743e1734c..53206d76e9f 100644 --- a/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c +++ b/gcc/testsuite/gcc.target/riscv/zicond_ifcvt_opt.c @@ -615,6 +615,69 @@ test_RotateR_eqz (unsigned long x, unsigned long y, unsigned long z, return x; } +int +test_ADD_ceqz_int (int x, int y, int z, int c) +{ + if (c) + x = y + z; + else + x = y; + return x; +} + +int +test_ShiftLeft_eqz_int (int x, int y, int z, int c) +{ + if (c) + x = y << z; + else + x = y; + return x; +} + +int +test_ShiftR_eqz_int (int x, int y, int z, int c) +{ + if (c) + x = y >> z; + else + x = y; + return x; +} + +unsigned int +test_ShiftR_logical_eqz_int (unsigned int x, unsigned int y, unsigned int z, + unsigned int c) +{ + if (c) + x = y >> z; + else + x = y; + return x; +} + +unsigned int +test_RotateL_eqz_int (unsigned int x, unsigned int y, unsigned int z, + unsigned int c) +{ + if (c) + x = (y << z) | (y >> (32 - z)); + else + x = y; + return x; +} + +unsigned int +test_RotateR_eqz_int (unsigned int x, unsigned int y, unsigned int z, + unsigned int c) +{ + if (c) + x = (y >> z) | (y << (32 - z)); + else + x = y; + return x; +} + long test_ADD_ceqz_imm (long x, long y, long c) { @@ -1225,6 +1288,67 @@ test_RotateR_eqz_imm (unsigned long x, unsigned long y, unsigned long c) x = y; return x; } + +int +test_ADD_ceqz_imm_int (int x, int y, int c) +{ + if (c) + x = y + 11; + else + x = y; + return x; +} + +int +test_ShiftLeft_eqz_imm_int (int x, int y, int c) +{ + if (c) + x = y << 11; + else + x = y; + return x; +} + +int +test_ShiftR_eqz_imm_int (int x, int y, int c) +{ + if (c) + x = y >> 11; + else + x = y; + return x; +} + +unsigned int +test_ShiftR_logical_eqz_imm_int (unsigned int x, unsigned int y, unsigned int c) +{ + if (c) + x = y >> 11; + else + x = y; + return x; +} + +unsigned int +test_RotateL_eqz_imm_int (unsigned int x, unsigned int y, unsigned int c) +{ + if (c) + x = (y << 11) | (y >> (32 - 11)); + else + x = y; + return x; +} + +unsigned int +test_RotateR_eqz_imm_int (unsigned int x, unsigned int y, unsigned int c) +{ + if (c) + x = (y >> 11) | (y << (32 - 11)); + else + x = y; + return x; +} + long test_AND_ceqz (long x, long y, long z, long c) { @@ -1544,5 +1668,5 @@ test_AND_eqz_x_2_imm_reverse_bin_oprands (long x, long c) x = 11 & x; return x; } -/* { dg-final { scan-assembler-times {czero\.eqz} 82 } } */ +/* { dg-final { scan-assembler-times {czero\.eqz} 94 } } */ /* { dg-final { scan-assembler-times {czero\.nez} 72 } } */