Re: Re: [PATCH 5/5] [ifcvt] optimize extension for x=c ? (y op z) : y by RISC-V Zicond like insns

["Fei Gao" <gaofei@eswincomputing.com>]

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