> On 23 Aug 2026, at 05:21, Jeffrey Law <[email protected]> wrote: > > > > On 8/13/2026 3:02 AM, Kyrylo Tkachov wrote: >> >>> On 12 Aug 2026, at 19:02, Jeffrey Law <[email protected]> wrote: >>> >>> >>> >>> On 8/9/2026 1:25 PM, [email protected] wrote: >>>> From: Kyrylo Tkachov <[email protected]> >>>> >>>> If-conversion emits its replacement sequence at the end of the test block. >>>> Expanding a conditional move there can require a fresh comparison, which >>>> writes a condition-code register. end_ifcvt_sequence already rejects a >>>> sequence that would destroy the condition code tested by the branch, but >>>> cc_in_cond only reports that register when the branch reads it directly. >>>> A branch such as AArch64 CBZ tests a general register, so the guard was >>>> inert and an unrelated live condition code could be destroyed. >>>> >>>> For the testcase at -O2 on aarch64, late-combine sinks a cset into the >>>> join block, which leaves the flags live across the branch: >>>> >>>> bb2: cmp w3, 1 // sets cc >>>> cset w4, ls >>>> cbz w1, .L2 // does not touch cc >>>> bb5: cinc w2, w2, ls // reads cc >>>> >>>> ce2 then if-converted bb3 and bb4 and inserted "cmp w1, 0" ahead of the >>>> branch, so cinc read the wrong flags: >>>> >>>> cmp w3, 1 >>>> cset w4, ls >>>> cmp w1, 0 // clobbers the live flags >>>> csel w2, w2, w3, eq >>>> cinc w2, w2, ls // reads cmp w1, 0 >>>> >>>> rtl.h documents that ports in the "lowered" form, which includes aarch64 >>>> before register allocation, may keep the flags live between instructions, >>>> so if-conversion has to respect that. Reject a generated sequence that >>>> writes a condition-code register while it is live on exit from the test >>>> block. Condition codes are recognised by mode class, as in cc_in_cond, >>>> which covers ports that do not define TARGET_FLAGS_REGNUM. DF liveness is >>>> already up to date here and the same paths query it for pseudos. >>>> >>>> noce_convert_multiple_sets validates its sequence itself rather than >>>> through end_ifcvt_sequence, and reaches noce_emit_cmove in the same way, >>>> so it gets the same check. >>>> >>>> PR126747 is the same defect reached from a different direction. At -Os the >>>> multiplication overflow idiom becomes one .MUL_OVERFLOW, so both arms read >>>> a >>>> single cset, and late-combine folds it into the second one: >>>> >>>> bb2: cmp xzr, x0, lsr 32 // sets cc >>>> cset w3, ne >>>> cbz w0, .L4 // does not touch cc >>>> bb5: cinc w0, w0, ne // reads cc >>>> >>>> ce2 if-converted bb3 the same way and "cmp w0, 0" landed ahead of the >>>> cinc, so >>>> foo (1, 1) returned 1 instead of 0. >>>> >>>> Bootstrapped and tested on aarch64-none-linux. >>>> Ok for trunk? >>>> Thanks, >>>> Kyrill >>>> >>>> gcc/ChangeLog: >>>> >>>> PR rtl-optimization/126501 >>>> PR rtl-optimization/126747 >>>> * ifcvt.cc (noce_clobbers_live_cc_p): New function. >>>> (end_ifcvt_sequence): Use it to reject sequences that clobber a >>>> condition-code register that is live out of the test block. >>>> (noce_convert_multiple_sets): Likewise. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR rtl-optimization/126501 >>>> PR rtl-optimization/126747 >>>> * gcc.c-torture/execute/pr126501.c: New test. >>>> * gcc.c-torture/execute/pr126747.c: New test. >>>> >>>> Signed-off-by: Kyrylo Tkachov <[email protected]> >>> So the concern I have is the test for whether or not a register is a flags >>> register. My worry is targets which don't have flags registers, but >>> instead put them in GPRs. In that case we're going to get MODE_INT back, >>> not MODE_CC. Note that this is potentially meaningfully different than >>> cc_in_cond where we're taking the mode from an operand of a larger RTX. >>> That can well give us a hard register in a mode other than reg_raw_mode. >> This is all a bit arcane stuff :) >> I’ve done a bit of digging and extended the CC register recording to handle >> the fixed registers advertised by the backends. I think that should do the >> right thing? >> Attached patch implements that. > Arcane is par for the course in much of the RTL code :-) > > It looks reasonable to me at this point. I recommend you commit and we give > it some soak time before we push any of the other ifcvt stuff that's queued > up (from you and myself).
Thanks for the reviews. I’ve pushed it. Kyrill > > jeff
