Richard Sandiford <richard.sandif...@arm.com> writes:
> Andrew Pinski <quic_apin...@quicinc.com> writes:
>> While looking into PR 112454, I found the cost for
>> `(if_then_else (cmp) (const_int 1) (reg))` was being recorded as 8
>> (or `COSTS_N_INSNS (2)`) but it should have been 4 (or `COSTS_N_INSNS (1)`).
>> This improves the cost by not adding the cost of `(const_int 1)` to
>> the total cost.
>>
>> It does not does not fix PR 112454 as that requires other changes to forwprop
>> the `(const_int 1)` earlier than combine.
>>
>> Bootstrapped and tested on aarch64-linux-gnu with no regressions.
>>
>> gcc/ChangeLog:
>>
>> * config/aarch64/aarch64.cc (aarch64_if_then_else_costs):
>> Don't add the cost of `1` or `-1`.
>>
>> Signed-off-by: Andrew Pinski <quic_apin...@quicinc.com>
>> ---
>> gcc/config/aarch64/aarch64.cc | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
>> index f6f6f94bf43..63241c5aaa5 100644
>> --- a/gcc/config/aarch64/aarch64.cc
>> +++ b/gcc/config/aarch64/aarch64.cc
>> @@ -11642,9 +11642,16 @@ aarch64_if_then_else_costs (rtx op0, rtx op1, rtx
>> op2, int *cost, bool speed)
>> /* CSINV/NEG with zero extend + const 0 (*csinv3_uxtw_insn3). */
>> op1 = XEXP (inner, 0);
>> }
>> -
>> - *cost += rtx_cost (op1, VOIDmode, IF_THEN_ELSE, 1, speed);
>> - *cost += rtx_cost (op2, VOIDmode, IF_THEN_ELSE, 2, speed);
>> + if (op2 == constm1_rtx || op2 == const1_rtx)
>> + *cost += rtx_cost (op1, VOIDmode, IF_THEN_ELSE, 1, speed);
>> + else if (op1 == constm1_rtx || op1 == const1_rtx)
>> + *cost += rtx_cost (op2, VOIDmode, IF_THEN_ELSE, 2, speed);
>
> It looks like this is really an extra option on top of the previous
> if-else chain, since it only applies when OP1 and OP2 are still the
> operands of the if_then_else. So how about:
>
> else if (op1 == constm1_rtx || op1 == const1_rtx)
> {
> /* Use CSINV. */
eh, of course I meant CSINV or CSINC...
> *cost += rtx_cost (op2, VOIDmode, IF_THEN_ELSE, 2, speed);
> return true;
> }
> else if (op2 == constm1_rtx || op2 == const1_rtx)
> {
> /* Use CSINV. */
> *cost += rtx_cost (op1, VOIDmode, IF_THEN_ELSE, 1, speed);
> return true;
> }
>
> leaving the code to fall through to:
>
> *cost += rtx_cost (op1, VOIDmode, IF_THEN_ELSE, 1, speed);
> *cost += rtx_cost (op2, VOIDmode, IF_THEN_ELSE, 2, speed);
> return true;
>
> as it does currently. OK in that form if you agree.
>
> Let me know if you don't. But in that case:
>
>> + else
>> + {
>> + *cost += rtx_cost (op1, VOIDmode, IF_THEN_ELSE, 1, speed);
>> + *cost += rtx_cost (op2, VOIDmode, IF_THEN_ELSE, 1, speed);
>
> should be 2, speed
>
>> + }
>> +
>
> Thanks,
> Richard
