On 7/11/26 00:32, Jeffrey Law wrote:
>
> On 6/30/2026 12:19 PM, Radosav Krunic wrote:
>> It tunes ifcvt parameters so that we get if-conversion in more cases.
>>
>> gcc/
>>
>> * config/mips/mips.cc (mips_rtx_costs): Reduce cost of
>> if_then_else pattern.
>> (mips_max_noce_ifcvt_seq_cost): New function. Decrease
>> maximum permissible cost for the unconditional sequence which
>> should be generated during if-conversion (for all non-r6
>> targets). This disables if-conversion for non-r6 targets in
>> branch-cost-1.c test.
>> (mips_noce_conversion_profitable_p): New function.
>> (TARGET_MAX_NOCE_IFCVT_SEQ_COST): Define hook.
>> (TARGET_NOCE_CONVERSION_PROFITABLE_P): Define hook.
>>
>> gcc/testsuite/
>>
>> * gcc.target/mips/branch-cost-1.c: Disable for -Os.
> Note this is MIPS specific, so in an ideal world someone more familiar
> with MIPS targets would step in with an ACK or list of things to fix.
> In the absence of that feedback...
>
>
>> gcc/config/mips/mips.cc | 56 +++++++++++++++++++
>> gcc/testsuite/gcc.target/mips/branch-cost-1.c | 2 +-
>> 2 files changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
>> index 277ec419826..81a41b8ee3a 100644
>> --- a/gcc/config/mips/mips.cc
>> +++ b/gcc/config/mips/mips.cc
>> @@ -67,6 +67,7 @@ along with GCC; see the file COPYING3. If not see
>> #include "rtl-iter.h"
>> #include "flags.h"
>> #include "opts.h"
>> +#include "ifcvt.h"
>>
>> /* This file should be included last. */
>> #include "target-def.h"
>> @@ -4795,6 +4796,12 @@ mips_rtx_costs (rtx x, machine_mode mode, int
>> outer_code,
>> break;
>> }
>> return false;
>> +
>> + case IF_THEN_ELSE:
>> + if (reg_or_0_operand (XEXP (x, 1), VOIDmode)
>> + || reg_or_0_operand (XEXP (x, 2), VOIDmode))
>> + *total = 0;
>> + return false;
> Formatting looks wrong here. Make sure the case IF_THEN_ELSE lines up
> with other cases in that switch. The IF should be indented two spaces
> relative to the case label and so-on following GNU style guidelines.
>
>>
>>
>> +
>> +/* Return true if SEQ is a good candidate as a replacement for the
>> + if-convertible sequence described in IF_INFO. */
>> +
>> +static bool
>> +mips_noce_conversion_profitable_p (rtx_insn *seq, struct
>> noce_if_info *if_info)
>> +{
>> + struct noce_if_info mips_if_info = *if_info;
>> +
>> + for (rtx_insn *insn = seq; insn; insn = NEXT_INSN (insn))
>> + {
>> + /* Compensate target-specific costs by incrementing original_cost
>> + (mips_if_info.original_cost) and max_seq_costif
>> (mips_if_info.ax_seq_costif) */
>> + break;
>> + }
>> +
>> + return default_noce_conversion_profitable_p(seq, &mips_if_info);
> So is this hook definition really doing anything useful? It doesn't
> really do anything with the insns in the sequence and just seems to hand
> back off to the generic variant. Am I missing something? Formatting
> nit. if we're keeping this code there's always supposed to be a space
> between the function name and the open paren for its arguments.
>
> Basically it seems sensible from an overall structure standpoint. I
> can't really comment on the cost model as I haven't done meaningful work
> on mips in probably 20 years at this point. THe biggest issue I see is
> the conversion_profitable_hook seems pointless and should probably just
> be removed unless there's a followup which is going to improve it
> further.
>
> jeff
Hi Jeff,
Thanks for the comments and sorry for the delayed response.
I'll fix the formatting and remove the conversion_profitable_hook, as it is
unnecessary in v4.
Best regards,
Radosav
