On 2023-12-11 13:38 Jeff Law <[email protected]> wrote:
>
>
>
>On 12/5/23 01:12, Fei Gao wrote:
>> op=[PLUS, MINUS, IOR, XOR, ASHIFT, ASHIFTRT, LSHIFTRT, ROTATE, ROTATERT, AND]
>>
>> Co-authored-by: Xiao Zeng<[email protected]>
>>
>> gcc/ChangeLog:
>>
>> * ifcvt.cc (noce_cond_zero_shift_op_supported): check if OP is
>>shift like operation
>> (noce_cond_zero_binary_op_supported): restructure & call
>>noce_cond_zero_shift_op_supported
>> (noce_bbs_ok_for_cond_zero_arith): add support for const_int
>> (noce_try_cond_zero_arith): add support for x=c ? (y op const_int)
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/zicond_ifcvt_opt.c: add TCs for x=c ? (y op
>>const_int) : y
>> @@ -3089,7 +3111,18 @@ noce_try_cond_zero_arith (struct noce_if_info
>> *if_info)
>> return false;
>> }
>>
>> - *to_replace = target;
>> + if (CONST_INT_P (*to_replace))
>> + {
>> + if (noce_cond_zero_shift_op_supported (bin_code))
>> + *to_replace = gen_rtx_SUBREG (E_QImode, target, 0);
>> + else if (SUBREG_P (bin_op0))
>> + *to_replace = gen_rtx_SUBREG (GET_MODE (bin_op0), target, 0);
>> + else
>> + *to_replace = target;
>Not all targets use QImode for their shift counts, so you can't just
>force that argument to QImode.
Thanks for your info. I haven't understood the "complex" you mentioned
regarding subreg until now.
>
>The way this works in our internal tree is that we re-expand the binary
>operation rather than replacing bits of existing RTL. That allows the
>expanders to do the right thing automatically for the target WRT
>handling of things like the mode of the shift count. In fact, I don't
>see how you can ever do replacement of a constant with a register with
>the current scheme since the original constant will be modeless, so you
>never know what mode to use.
Letting the expander to handle const_int case seems a target general solution.
BR,
Fei
>
>
>
>Jeff