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

Reply via email to