On Wed, 7 Jan 2026, Jakub Jelinek wrote:
> Hi!
>
> Back in April last year I've changed try_combine's condition when trying to
> split two independent sets by moving one of them to i2. Previously this was
> testing !modified_between_p (SET_DEST (setN), i2, i3) and I've changed it
> to SET_DEST (setN) != pc_rtx && !reg_used_between_p (SET_DEST (set1), i2, i3)
> on the assumption written in the r15-9131-g19ba913517b5e2a00 commit
> message:
> "The following patch replaces the modified_between_p
> tests with reg_used_between_p, my understanding is that
> modified_between_p is a subset of reg_used_between_p, so one
> doesn't need both."
> That assumption is wrong though, neither of these is a subset of the
> other and I don't see any APIs which test both. We need to avoid moving
> a set from i3 to i2 both in case where the REG (or SUBREG_REG of SUBREG or
> MEM or whatever else) is set/modified between i2 and i3 exclusive, as shown
> by the testcase in PR121773 (which I'm not including because my ARM neon
> knowledge is limited). We have i2 insn 18 and i3 insn 7 after the current
> try_combine modifications:
> (insn 18 5 19 2 (set (reg:SI 104 [ _6 ])
> (const_int 305419896 [0x12345678])) "include/arm_neon.h":7467:22 542
> {*arm_movsi_vfp}
> (expr_list:REG_EQUAL (const_int 305419896 [0x12345678])
> (nil)))
> (insn 19 18 21 2 (set (reg:SI 105 [ _6+4 ])
> (const_int 538968064 [0x20200000])) "include/arm_neon.h":7467:22 542
> {*arm_movsi_vfp}
> (nil))
> (insn 21 19 7 2 (set (reg:DI 101 [ _5 ])
> (const_int 0 [0])) "include/arm_neon.h":607:14 -1
> (nil))
> (insn 7 21 8 2 (parallel [
> (set (pc)
> (pc))
> (set (subreg:SI (reg:DI 101 [ _5 ]) 0)
> (const_int 610839792 [0x2468acf0]))
> ]) "include/arm_neon.h":607:14 17 {addsi3_compare_op1}
> (expr_list:REG_DEAD (reg:SI 104 [ _6 ])
> (nil)))
> The second set can't be moved to the i2 location, because (reg:DI 101)
> is modified in insn 21 and so if setting half of it to 610839792 is
> moved from insn 7 where it modifies what was previously 0 into a location
> where it overwrites something and is later overwritten in insn 21, we get
> different behavior.
> And the second case is mentioned in the PR119291 commit log:
> (insn 22 21 23 4 (set (reg:SI 104 [ _7 ])
> (const_int 0 [0])) "pr119291.c":25:15 96 {*movsi_internal}
> (nil))
> (insn 23 22 24 4 (set (reg/v:SI 117 [ e ])
> (reg/v:SI 116 [ e ])) 96 {*movsi_internal}
> (expr_list:REG_DEAD (reg/v:SI 116 [ e ])
> (nil)))
> (note 24 23 25 4 NOTE_INSN_DELETED)
> (insn 25 24 26 4 (parallel [
> (set (pc)
> (pc))
> (set (reg/v:SI 116 [ e ])
> (const_int 0 [0]))
> ]) "pr119291.c":28:13 977 {*negsi_2}
> (expr_list:REG_DEAD (reg:SI 104 [ _7 ])
> (nil)))
> i2 is insn 22, i3 is insn 25 after in progress modifications and the
> second set can't be moved to i2 location, because (reg/v:SI 116) is used
> in insn 23, so with it being set to 0 around insn 22 insn 23 will see
> a different value.
>
> So, I'm afraid we need both the modified_between_p and reg_used_between_p
> check. We don't need the SET_DEST (setN) != pc_rtx checks, those were
> added because modified_between_p (pc_rtx, i2, i3) returns true if start
> is not the same as end, but reg_used_between_p doesn't behave like that.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> and release branches?
OK.
Richard.
> 2026-01-06 Jakub Jelinek <[email protected]>
>
> PR rtl-optimization/119291
> PR rtl-optimization/121773
> * combine.cc (try_combine): Check that SET_DEST (setN) is neither
> modified_between_p nor reg_used_between_p instead of just not
> reg_used_between_p or pc_rtx.
>
> --- gcc/combine.cc.jj 2026-01-02 09:56:09.924340621 +0100
> +++ gcc/combine.cc 2026-01-06 19:34:48.689525955 +0100
> @@ -4028,7 +4028,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
> && !(GET_CODE (SET_DEST (set1)) == SUBREG
> && find_reg_note (i2, REG_DEAD,
> SUBREG_REG (SET_DEST (set1))))
> - && SET_DEST (set1) != pc_rtx
> + && !modified_between_p (SET_DEST (set1), i2, i3)
> && !reg_used_between_p (SET_DEST (set1), i2, i3)))
> /* If I3 is a jump, ensure that set0 is a jump so that
> we do not create invalid RTL. */
> @@ -4044,7 +4044,7 @@ try_combine (rtx_insn *i3, rtx_insn *i2,
> && !(GET_CODE (SET_DEST (set0)) == SUBREG
> && find_reg_note (i2, REG_DEAD,
> SUBREG_REG (SET_DEST (set0))))
> - && SET_DEST (set0) != pc_rtx
> + && !modified_between_p (SET_DEST (set0), i2, i3)
> && !reg_used_between_p (SET_DEST (set0), i2, i3)))
> /* If I3 is a jump, ensure that set1 is a jump so that
> we do not create invalid RTL. */
>
> Jakub
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
