The attached v3 tracks the use of cond_earliest as you suggested and adds its cost in default_noce_conversion_profitable_p.
Bootstrapped and regtested on x86 and p10, aarch64 still running. Regtested on riscv64. Regards Robin Before noce_find_if_block processes a block it sets up an if_info structure that holds the original costs. At that point the costs of the then/else blocks have not been added so we only care about the "if" cost. The code originally used BRANCH_COST for that but was then changed to COST_N_INSNS (2) - a compare and a jump. This patch computes the jump costs via insn_cost (if_info.jump, ...) under the assumption that the target takes BRANCH_COST into account when costing a jump instruction. In noce_convert_multiple_sets, we keep track of the need for the initial CC comparison. If we needed it for the generated sequence we add its cost in default_noce_conversion_profitable_p. gcc/ChangeLog: * ifcvt.cc (default_noce_conversion_profitable_p): Add cost of CC comparison. (noce_convert_multiple_sets_1): Set use_cond_earliest. (noce_process_if_block): Just use original cost. (noce_find_if_block): Use insn_cost (jump_insn). * ifcvt.h (struct noce_if_info): Add use_cond_earliest. --- gcc/ifcvt.cc | 37 ++++++++++++++++++++++--------------- gcc/ifcvt.h | 3 +++ 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc index 58ed42673e5..9b408eeb313 100644 --- a/gcc/ifcvt.cc +++ b/gcc/ifcvt.cc @@ -814,7 +814,16 @@ default_noce_conversion_profitable_p (rtx_insn *seq, /* Cost up the new sequence. */ unsigned int cost = seq_cost (seq, speed_p); - if (cost <= if_info->original_cost) + /* If the created sequence does not use cond_earliest (but the jump + does) add its cost to the original_cost here. */ + unsigned int cost_adjust = 0; + + if (if_info->jump != if_info->cond_earliest + && !if_info->use_cond_earliest) + cost_adjust = insn_cost (if_info->cond_earliest, + if_info->speed_p); + + if (cost <= if_info->original_cost + cost_adjust) return true; /* When compiling for size, we can make a reasonably accurately guess @@ -3780,6 +3789,7 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info, temp_dest = temp_dest2; if (!second_try && read_comparison) *last_needs_comparison = count; + if_info->use_cond_earliest = true; } else { @@ -3931,16 +3941,13 @@ noce_process_if_block (struct noce_if_info *if_info) to calculate a value for x. ??? For future expansion, further expand the "multiple X" rules. */ - /* First look for multiple SETS. The original costs already include - a base cost of COSTS_N_INSNS (2): one instruction for the compare - (which we will be needing either way) and one instruction for the - branch. When comparing costs we want to use the branch instruction - cost and the sets vs. the cmovs generated here. Therefore subtract - the costs of the compare before checking. - ??? Actually, instead of the branch instruction costs we might want - to use COSTS_N_INSNS (BRANCH_COST ()) as in other places. */ + /* First look for multiple SETS. + The original costs already include costs for the jump insn as well + as for a CC comparison if there is any. + If a target re-uses the existing CC comparison we keep track of that + and add the costs in default default_noce_conversion_profitable_p. */ - unsigned potential_cost = if_info->original_cost - COSTS_N_INSNS (1); + unsigned potential_cost = if_info->original_cost; unsigned old_cost = if_info->original_cost; if (!else_bb && HAVE_conditional_move @@ -4696,6 +4703,7 @@ noce_find_if_block (basic_block test_bb, edge then_edge, edge else_edge, gcc_assert (if_info.rev_cond == NULL_RTX || rev_cond_earliest == cond_earliest); if_info.cond_earliest = cond_earliest; + if_info.use_cond_earliest = false; if_info.jump = jump; if_info.then_else_reversed = then_else_reversed; if_info.speed_p = speed_p; @@ -4703,11 +4711,10 @@ noce_find_if_block (basic_block test_bb, edge then_edge, edge else_edge, = targetm.max_noce_ifcvt_seq_cost (then_edge); /* We'll add in the cost of THEN_BB and ELSE_BB later, when we check that they are valid to transform. We can't easily get back to the insn - for COND (and it may not exist if we had to canonicalize to get COND), - and jump_insns are always given a cost of 1 by seq_cost, so treat - both instructions as having cost COSTS_N_INSNS (1). */ - if_info.original_cost = COSTS_N_INSNS (2); - + for COND (and it may not exist if we had to canonicalize to get COND). + It is assumed that the costs of a jump insn are dependent on the + branch costs. */ + if_info.original_cost += insn_cost (if_info.jump, if_info.speed_p); /* Do the real work. */ diff --git a/gcc/ifcvt.h b/gcc/ifcvt.h index 37147f99129..67d4f6a502e 100644 --- a/gcc/ifcvt.h +++ b/gcc/ifcvt.h @@ -63,6 +63,9 @@ struct noce_if_info /* New insns should be inserted before this one. */ rtx_insn *cond_earliest; + /* Whether or not COND_EARLIEST is used in the resulting sequence. */ + bool use_cond_earliest; + /* Insns in the THEN and ELSE block. There is always just this one insns in those blocks. The insns are single_set insns. If there was no ELSE block, INSN_B is the last insn before -- 2.45.1