> 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? > > If passes are able to clean things up so that there's a single set we can > often use other sequences to generate more efficient code than a generalized > conditional move. > > There may be examples of this in bugzilla. I've certainly seen it happen on > multiple occasions. I’ll gather some aarch64 stats for SPEC2026 and I’m hoping Lili will help with x86 evaluation so that we don’t over-if-convert. > > And just to be clear, this patch hasn't been forgotten, it's just more > complex than most, so taking longer to work through. On a positive note I > started including it in my tester with Sunday's runs. I've seen a come > quirks pop since then, but nothing I can concretely say is due to the > if-conversion patch though. 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 ;) Kyrill > > Jeff
