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