> Is there any way we can avoid using pattern_cost here? Using it means
> that we can make use of targetm.insn_cost for the jump but circumvent
> it for the condition, giving a bit of a mixed metric.
>
> (I realise there are existing calls to pattern_cost in ifcvt.cc,
> but if possible I think we should try to avoid adding more.)
Yes, I believe there is. In addition, what I did with
if_info->cond wasn't what I intended to do.
The whole point of the exercise is that noce_convert_multiple_sets
can re-use the CC comparison that is already present (because it
is used in the jump pattern). Therefore I want to split costs
into a jump part and a CC-setting part so the final costing
decision for multiple sets can be:
insn_cost (jump) + n * insn_cost (set)
vs
n * insn_cost ("cmov")
Still, the original costs should be:
insn_cost (set_cc) + insn_cost (jump)
and with the split we can just remove insn_cost (set_cc) before
the multiple-set cost comparison and re-add it afterwards.
For non-CC targets this is not necessary.
So what I'd hope is better is to use
insn_cost (if_info.earliest_cond)
which is indeed the CC-set/comparison if it exists.
The attached v2 was bootstrapped and regtested on x86, aarch64 and
power10 and regtested on riscv64.
Regards
Robin
gcc/ChangeLog:
* ifcvt.cc (noce_process_if_block): Subtract condition pattern
cost if applicable.
(noce_find_if_block): Use insn_cost and pattern_cost for
original cost.
---
gcc/ifcvt.cc | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index 58ed42673e5..ebb838fd82c 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -3931,16 +3931,16 @@ 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. */
-
- unsigned potential_cost = if_info->original_cost - COSTS_N_INSNS (1);
+ /* 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.
+ We want to allow the backend to re-use the existing CC comparison
+ and therefore don't consider it for the cost comparison (as it is
+ then needed for both the jump as well as the cmov sequence). */
+
+ unsigned potential_cost = if_info->original_cost;
+ if (if_info->cond_earliest && if_info->jump != if_info->cond_earliest)
+ potential_cost -= insn_cost (if_info->cond_earliest, if_info->speed_p);
unsigned old_cost = if_info->original_cost;
if (!else_bb
&& HAVE_conditional_move
@@ -4703,11 +4703,12 @@ 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).
+ jump insn that is costed via insn_cost. It is assumed that the
+ costs of a jump insn are dependent on the branch costs. */
+ if (if_info.cond_earliest && if_info.jump != if_info.cond_earliest)
+ if_info.original_cost = insn_cost (if_info.cond_earliest, if_info.speed_p);
+ if_info.original_cost += insn_cost (if_info.jump, if_info.speed_p);
/* Do the real work. */
--
2.45.1
