On 2023-12-11 13:46 Jeff Law <jeffreya...@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<zengx...@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