On Tue, Jun 2, 2026 at 8:14 AM Jakub Jelinek <[email protected]> wrote:
>
> On Fri, May 29, 2026 at 09:47:45AM +0200, Jakub Jelinek wrote:
> > I've debugged this now on the 16 branch where my patch is not present.
> > The
> > (define_split
> > [(set (match_operand:SWI 0 "register_operand")
> > (plus:SWI (match_operand:SWI 1 "register_operand")
> > (match_operand:SWI 2 "<nonmemory_operand>")))]
> > "TARGET_APX_NF && reload_completed
> > && ix86_lea_for_add_ok (insn, operands)"
> > [(set (match_dup 0)
> > (plus:<LEAMODE> (match_dup 1) (match_dup 2)))]
> > {
> > if (<MODE>mode != <LEAMODE>mode)
> > {
> > operands[0] = gen_lowpart (<LEAMODE>mode, operands[0]);
> > operands[1] = gen_lowpart (<LEAMODE>mode, operands[1]);
> > operands[2] = gen_lowpart (<LEAMODE>mode, operands[2]);
> > }
> > })
> > splitter there is matched, including ix86_lea_for_add_ok on
> > (insn 12 10 13 2 (set (reg:DI 2 cx [108])
> > (plus:DI (reg:DI 0 ax [orig:104 s ] [104])
> > (reg:DI 5 di [ _2+8 ]))) "apx-nf-pr125469.c":10:5 288
> > {*adddi_1_nf}
> > (nil))
> > But then goes into:
> > rtx_insn *
> > gen_split_138 (rtx_insn *curr_insn ATTRIBUTE_UNUSED, rtx *operands)
> > {
> > if (dump_file)
> > fprintf (dump_file, "Splitting with gen_split_138 (i386.md:7136)\n");
> > start_sequence ();
> > #define FAIL return (end_sequence (), nullptr)
> > #define DONE return end_sequence ()
> > #line 7144 "../../gcc/config/i386/i386.md"
> > {
> > if (DImode != DImode)
> > {
> > operands[0] = gen_lowpart (DImode, operands[0]);
> > operands[1] = gen_lowpart (DImode, operands[1]);
> > operands[2] = gen_lowpart (DImode, operands[2]);
> > }
> > }
> > #undef DONE
> > #undef FAIL
> > static const uint8_t expand_encoding[] = {
> > 0x01, 0x1f, 0x01, 0x00, 0x3b, 0x12, 0x01, 0x01,
> > 0x01, 0x02
> > };
> > return complete_seq (expand_encoding, operands);
> > }
> > That returns
> > (insn 37 0 0 (set (reg:DI 2 cx [108])
> > (plus:DI (reg:DI 0 ax [orig:104 s ] [104])
> > (reg:DI 5 di [ _2+8 ]))) -1
> > (nil))
> > But then in try_split we have:
> > /* Avoid infinite loop if any insn of the result matches
> > the original pattern. */
> > insn_last = seq;
> > while (1)
> > {
> > if (INSN_P (insn_last)
> > && rtx_equal_p (PATTERN (insn_last), pat))
> > return trial;
> > which is a must, otherwise we'd try to split the newly created insn again
> > and again succeed in splitting it and so forth forever.
>
> I've dropped the ball on this sorry.
>
> I've tested following patch on the trunk on x86_64-linux and i686-linux,
> ok for trunk?
>
> 2026-06-02 Jakub Jelinek <[email protected]>
>
> PR target/125469
> * config/i386/i386.md (*add<mode>_1<nf_name>): Revert 2026-05-28
> change.
> (define_split from *add<mode>_1_nf to *lea<mode>): Set INSN_CODE
> to -1 if <LEAMODE> and <MODE> are the same.
LGTM, but perhaps the RTL maintainer (CC'd) should also look at the
patch if the approach is OK.
Thanks,
Uros.
>
>
> --- gcc/config/i386/i386.md.jj 2026-06-01 23:00:32.078971912 +0200
> +++ gcc/config/i386/i386.md 2026-06-01 23:04:41.017838244 +0200
> @@ -6760,10 +6760,7 @@ (define_insn "*add<mode>_1<nf_name>"
> if (TARGET_APX_NDD && <nf_applied>)
> return "%{nf%} add{<imodesuffix>}\t{%2, %1, %0|%0, %1, %2}";
> else
> - {
> - operands[3] = gen_rtx_PLUS (<MODE>mode, operands[1], operands[2]);
> - return "lea{<imodesuffix>}\t{%E3, %0|%0, %E3}";
> - }
> + return "#";
>
> case TYPE_INCDEC:
> if (operands[2] == const1_rtx)
> @@ -7262,6 +7259,11 @@ (define_split
> operands[1] = gen_lowpart (<LEAMODE>mode, operands[1]);
> operands[2] = gen_lowpart (<LEAMODE>mode, operands[2]);
> }
> + else
> + /* *lea<mode> and *add<mode>_1_nf have the same pattern, try_split
> + refuses to split an insn into itself, so just request
> + a new recog which will match *lea<mode> which comes first. */
> + INSN_CODE (curr_insn) = -1;
> })
>
> ;; Convert add to the lea pattern to avoid flags dependency.
>
>
> Jakub
>
