Re: [PATCH 2/2] ifcvt: Do not clobber a live condition-code register [PR126501, PR126747]

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.

Jeff