On Fri, May 29, 2026 at 9:47 AM Jakub Jelinek <[email protected]> wrote: > > On Fri, May 29, 2026 at 08:16:13AM +0200, Uros Bizjak wrote: > > LEA is not fully compatible with ADD because %rsp can't be used as an > > index register (this is a general limitation of address expression > > which LEA inherits). As shown by Jakub, there are some problems when > > instructions are shadowing each other. In this particular case, the > > splitter at i386.md::7215 is not triggered for some reason, so we just > > skip the split and emit LEA by hand. > > 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. > So perhaps all we need to do in the splitter is: > --- gcc/config/i386/i386.md.jj 2026-05-20 11:51:10.378282182 +0200 > +++ gcc/config/i386/i386.md 2026-05-29 09:46:06.703661092 +0200 > @@ -7148,6 +7148,11 @@ > 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; > })
I remember that years ago I had a similar issue with the splitter. As a solution, I just emitted a new insn with the pattern of curr_insn, but looking at the source, where there is plenty of " INSN_CODE (insn) = -1;" I guess that your approach is the way to re-recognize an insn. Uros.
