Re: [RFC] ifcvt: Account for parallelism when costing noce sequences
Kyrylo Tkachov <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
Hello, > On 12 Aug 2026, at 12:39, wangjue <[email protected]> wrote: > > From: juewang <[email protected]> > > seq_cost treats every instruction in a replacement sequence as serial. > For superscalar targets this can overestimate the cost of independent > register operations and reject profitable if-conversion. > I like the idea, and it makes sense. One thing I’ve been hitting with my if-conversion work recently is that if-conversion increasing the dependency chain length is what’s hurting wide cores. Adding more instructions that can go parallel is not a problem otherwise. > Estimate the dependency level of simple, single-cycle register operations > and cost each level using the target issue rate. Keep the existing serial > cost for size optimization and for sequences that need resource or alias > analysis. > I notice that mips has a pretty elaborate mips_seq_time mechanism. I wouldn’t advocate using it, just found it interesting when looking around. > gcc/ChangeLog: > > * ifcvt.cc (noce_parallel_cost_node): New. > (noce_parallel_seq_cost): New. > (default_noce_conversion_profitable_p): Use it. > --- > gcc/ifcvt.cc | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 69 insertions(+), 1 deletion(-) > > diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc > index 5ea25f8fbe7..25874f006e6 100644 > --- a/gcc/ifcvt.cc > +++ b/gcc/ifcvt.cc > @@ -803,6 +803,74 @@ noce_reversed_cond_code (struct noce_if_info *if_info) > return reversed_comparison_code (if_info->cond, if_info->jump); > } > > +/* A destination and its dependency level in a noce sequence. */ > + > +struct noce_parallel_cost_node > +{ > + rtx dest; > + unsigned int level; > +}; > + > +/* Estimate the cost of SEQ using the target issue rate for independent, > + single-cycle register operations. */ > + > +static unsigned int > +noce_parallel_seq_cost (rtx_insn *seq, bool speed_p) > +{ > + unsigned int serial_cost = seq_cost (seq, speed_p); > + > + if (!speed_p) > + return serial_cost; > + > + unsigned int issue_rate = targetm.sched.issue_rate (); You first need to check that targetm.sched.issue_rate is non-null. I wonder if the schedule issue_rate is too coarse. The midend also uses reassociation width as an estimate of CPU parallelism. On aarch64 at least we describe different widths for int, fp, and vector reassociation. Have you considered using that hook instead? Thanks, Kyrill > + if (issue_rate <= 1) > + return serial_cost; > + > + auto_vec<noce_parallel_cost_node> nodes; > + auto_vec<unsigned int> insns_per_level; > + > + for (rtx_insn *insn = seq; insn; insn = NEXT_INSN (insn)) > + { > + if (!NONDEBUG_INSN_P (insn)) > + continue; > + > + rtx set = single_set (insn); > + if (!set) > + return serial_cost; > + > + rtx dest = SET_DEST (set); > + rtx src = SET_SRC (set); > + > + if (!REG_P (dest) > + || contains_mem_rtx_p (src) > + || side_effects_p (src) > + || may_trap_p (src) > + || set_rtx_cost (set, speed_p) != COSTS_N_INSNS (1)) > + return serial_cost; > + > + unsigned int level = 0; > + for (unsigned int i = 0; i < nodes.length (); ++i) > + if (reg_overlap_mentioned_p (nodes[i].dest, src)) > + level = MAX (level, nodes[i].level + 1); > + > + if (insns_per_level.length () <= level) > + insns_per_level.safe_grow_cleared (level + 1, true); > + ++insns_per_level[level]; > + > + noce_parallel_cost_node node = { dest, level }; > + nodes.safe_push (node); > + } > + > + if (nodes.is_empty ()) > + return serial_cost; > + > + unsigned int parallel_cost = 0; > + for (unsigned int i = 0; i < insns_per_level.length (); ++i) > + parallel_cost += COSTS_N_INSNS (CEIL (insns_per_level[i], issue_rate)); > + > + return MIN (serial_cost, parallel_cost); > +} > + > /* Return true if SEQ is a good candidate as a replacement for the > if-convertible sequence described in IF_INFO. > This is the default implementation that targets can override > @@ -815,7 +883,7 @@ default_noce_conversion_profitable_p (rtx_insn *seq, > bool speed_p = if_info->speed_p; > > /* Cost up the new sequence. */ > - unsigned int cost = seq_cost (seq, speed_p); > + unsigned int cost = noce_parallel_seq_cost (seq, speed_p); > > if (cost <= if_info->original_cost) > return true; > -- > 2.34.1 >