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
