Hi all,
I evaluated two patches using SPEC CPU 2017 / 2026 on EMR and znver5 with both
-O2 and -Ofast.
No regressions were observed. Perf counters confirmed a genuine improvement
only in 734.vpr_r.
-O2 -Ofast
EMR +3.2% +3.9%
znver5 +2.1% +2.9%
The improvement comes entirely from "get_non_updateable_bb". Before these
patches, if-conversion was unable to convert the following code:
if (x < xmin)
xmin = x;
else if (x > xmax)
xmax = x;
This issue is also tracked as GCC PR126139:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126139 .
Best regards,
Lili
> -----Original Message-----
> From: Jeff Law <[email protected]>
> Sent: Tuesday, September 1, 2026 3:22 PM
> To: Kyrylo Tkachov <[email protected]>
> Cc: Cui, Lili <[email protected]>; [email protected]
> Subject: Re: [PATCH] ifcvt: Extend noce_convert_multiple_sets to diamonds
> [PR125557]
>
>
>
> On 9/1/26 12:54 AM, Kyrylo Tkachov wrote:
> >
> >> On 1 Sep 2026, at 08:45, Jeff Law <[email protected]>
> wrote:
> >>
> >>
> >>
> >> On 8/31/26 11:40 PM, Kyrylo Tkachov wrote:
> >>>> Sure, happy to -- once the discussion on the list has settled, I will
> >>>> evaluate
> it on x86_64 and send the numbers.
> >>>>
> >>>> First, pre-evaluating the other arm into temporaries makes the diamond
> case fall out very naturally. One thing I would like to suggest: the gate
> here
> requires one arm's live-out set to be a subset of the others, and I think that
> limit can simply be dropped.
> >>>>
> >>>> Three steps:
> >>>> 1. The else arm's computations, evaluated unconditionally into fresh
> temporaries.
> >>>> 2. one conditional move for each set of the then arm, selecting that
> >>>> set's
> value against the value the else arm gave the same register -- or against the
> register's incoming value, if the else arm leaves it alone.
> >>>> 3. one conditional move for each register that only the else arm sets,
> selecting the register's incoming value against the temporary from step 1.
> >>>>
> >>>> Your patch already implements steps 1 and 2; only step 3 needs to be
> added. It is the same kind of move as step 2, so both can share one loop --
> and once it is there, all the code that enforces the limit falls away: the
> containment test, the "primary arm sets >= 2" rule and the arm swap. I have a
> rough change on top of your patch attached.
> >>>>
> >>>> My general feeling is that we should let noce_convert_multiple_sets
> convert as much as it can and leave it to the cost model to decide whether a
> conversion is worth it, rather than restricting the shapes up front.
> >>>>
> >>>> There is a second reason I would like it to handle the general case: it
> brings noce_convert_multiple_sets close enough to
> cond_move_process_if_block, noce_convert_multiple_sets is the more
> capable of the two, so once it is no longer restricted, the cases
> cond_move_process_if_block handles today would be converted by it instead,
> and benefit from that. Not something for this patch -- it just makes that step
> easier.
> >>>>
> >>>>
> >>> Thanks for the feedback. I think these extensions can be done cleaner as a
> follow-up patch, so I’ve incorporated your patch into a patch 2/2 and sent out
> the two for review.
> >>> Thanks your help!
> >> It's probably worth noting that converting multiple sets, particularly
> >> during
> the first if-conversion pass may be a loss on some platforms, particularly
> those
> with weaker conditional move sequences (ex risc-v where a generalized
> conditional move is 3 insns). It is often instead better to wait until after
> various passes have cleaned things up and the block in question often ends up
> with a single set.
> > Thanks, is the 3-insn form somehow costed accordingly in the backend i.e. is
> there some RTL-visible property we can use to tame the heuristic?
> Yes/No. We actually over-cost it right now. Some in flight work will make
> those costs more accurate and we'll be even more willing to use the multi-set
> path than we are now.
>
> The cases I've seen are profitable to convert using the multi-set path,
> they're
> just more profitable to wait and convert later. Trying to discover the better
> sequences after the fact is quite painful, though we have had some success
> doing so with some horrid risc-v target patterns.
>
> > Thanks, much appreciated. I have some ifcvt cleanup patches that I’ve
> > been working on that are staged behind this, hopefully they’ll pay
> > back a small part of the extra complexity this one introduces ;)
> There's two pending I'm aware of. First is Jue Wang's work on improving
> costing of the sequence to account for multi-issue designs. The case that work
> targets is the 3 conditional move sequence in mcf. That's something like 12
> instructions on RISC-V, so 48 cost units. But on a 6+ wide design it's just 3
> cycles or 12 cost units. Right now the dramatic over-costing causes rejection
> of the sequence.
>
> Second is costing the various approaches, including conditional moves and
> selecting the lowest cost rather than the first that's cheaper than a
> branch. This is primarily to allow x86 to prefer the cmov path while other
> targets like RISC-V can prefer a conditional arithmetic style path. I don't
> have
> the PR handy, but it's a pretty clear case showing the "first cheaper than
> branching" approach needs a re-think.
>
> Jeff
