Ping.
On Sun, Jan 18, 2026 at 10:13:41AM +0100, Stefan Schulze Frielinghaus wrote: > On Thu, Jan 15, 2026 at 09:40:21PM +0100, Stefan Schulze Frielinghaus wrote: > > I was almost about to push v2 and then realized there is a flaw in my > > design. Instead of checking whether the initial insns i3 to i0 are > > making use of hard register constraints, I have to check whether the > > resulting insn does. Or in other words, if an initial insn does make > > use of hard register constraints and the resulting insn does not, then a > > combination cannot introduce a conflict and is therefore ok. However, > > conversely, if all initial insns do not make use of hard register > > constraints, and therefore would pass the check of patch v2, the > > resulting insn could make use of hard register constraints, possibly > > resulting in a conflict. > > > > Long story short: do the check for hard register constraints for the > > resulting insn only. > > > > The PR was fixed with patch v2, since the initial as well as resulting > > insns make use of hard register constraints. > > > > I only did a quick test of v2 and it worked. It is getting late here > > and I will come back to this tomorrow. Just wanted to give an update. > > Giving this a second thought, I still believe that this is the right > fix. Patch v2 was not wrong but it didn't catch the real problem. That > was probably what's been bothering me the whole time. > > One might even check whether the initial insn into which a combination > is about to happen, had the same hard register constraints as the > resulting one, and allow a combination in this case. Since this would > have meant that prior combine, a hard register constraint must have been > satisfiable (or we had a problem before combine), and that in turn means > that after a combination, the hard register constraints would still be > satisfiable. Anyhow, this patch is about fixing errors/ICEs and not > about missed-optimizations and I would like to keep it simple while in > stage 4. > > Bootstrap and regtest are successful on s390x and x86_64. Tested PR via > a cross successfully. Ok for mainline? > > Cheers, > Stefan > > > > > I have seen that target i386 uses extract_insn() in > > TARGET_LEGITIMATE_COMBINED_INSN which is called directly after my > > changes. Instead of calling it twice in a row for the same insn, one > > could establish a contract for TARGET_LEGITIMATE_COMBINED_INSN where > > targets may assume that the insn has been already extracted. Maybe > > something for stage 1. > > > > -- >8 -- > > > > From: Stefan Schulze Frielinghaus <[email protected]> > > > > This fixes > > > > t.c:6:1: error: unable to find a register to spill > > 6 | } > > | ^ > > > > for target avr. In the PR we are given a patch which makes use of hard > > register constraints in the machine description for divmodhi4. Prior > > combine we have for the test from the PR > > > > (insn 7 6 8 2 (parallel [ > > (set (reg:HI 46 [ _1 ]) > > (div:HI (reg/v:HI 44 [ k ]) > > (reg:HI 48))) > > (set (reg:HI 47) > > (mod:HI (reg/v:HI 44 [ k ]) > > (reg:HI 48))) > > (clobber (scratch:HI)) > > (clobber (scratch:QI)) > > ]) "t.c":5:5 602 {divmodhi4} > > (expr_list:REG_DEAD (reg:HI 48) > > (expr_list:REG_DEAD (reg/v:HI 44 [ k ]) > > (expr_list:REG_UNUSED (reg:HI 47) > > (nil))))) > > (insn 8 7 9 2 (set (reg:HI 22 r22) > > (symbol_ref/f:HI ("*.LC0") [flags 0x2] <var_decl 0x3fff7950d10 > > *.LC0>)) "t.c":5:5 128 {*movhi_split} > > (nil)) > > (insn 9 8 10 2 (set (reg:HI 24 r24) > > (reg:HI 46 [ _1 ])) "t.c":5:5 128 {*movhi_split} > > (expr_list:REG_DEAD (reg:HI 46 [ _1 ]) > > (nil))) > > > > The patched instruction divmodhi4 constraints operand 2 (here pseudo > > 48) to hard register 22. Combine merges insn 7 into 9 by crossing a > > hard register assignment of register 22. > > > > (note 7 6 8 2 NOTE_INSN_DELETED) > > (insn 8 7 9 2 (set (reg:HI 22 r22) > > (symbol_ref/f:HI ("*.LC0") [flags 0x2] <var_decl 0x3fff7950d10 > > *.LC0>)) "t.c":5:5 128 {*movhi_split} > > (nil)) > > (insn 9 8 10 2 (parallel [ > > (set (reg:HI 24 r24) > > (div:HI (reg:HI 49 [ k ]) > > (reg:HI 48))) > > (set (reg:HI 47) > > (mod:HI (reg:HI 49 [ k ]) > > (reg:HI 48))) > > (clobber (scratch:HI)) > > (clobber (scratch:QI)) > > ]) "t.c":5:5 602 {divmodhi4} > > (expr_list:REG_DEAD (reg:HI 48) > > (expr_list:REG_DEAD (reg:HI 49 [ k ]) > > (nil)))) > > > > This leaves us with a conflict for pseudo 48 in the updated insn 9 since > > register 22 is live here. > > > > Fixed by pulling the sledge hammer and rejecting any resulting insn > > which makes use of hard register constraints. Ideally we would skip > > based on the fact whether a combination crosses a hard register > > assignment and the corresponding hard register is also referred by a > > single register constraint of the resulting insn. > > > > gcc/ChangeLog: > > > > * combine.cc (recog_for_combine_1): Reject insns which make use > > of hard register constraints. > > --- > > gcc/combine.cc | 38 ++++++++++++++++++++++++++++++++++---- > > 1 file changed, 34 insertions(+), 4 deletions(-) > > > > diff --git a/gcc/combine.cc b/gcc/combine.cc > > index 09e24347b34..fa8435ffd11 100644 > > --- a/gcc/combine.cc > > +++ b/gcc/combine.cc > > @@ -11570,12 +11570,42 @@ recog_for_combine_1 (rtx *pnewpat, rtx_insn > > *insn, rtx *pnotes, > > REG_NOTES (insn) = notes; > > INSN_CODE (insn) = insn_code_number; > > > > - /* Allow targets to reject combined insn. */ > > - if (!targetm.legitimate_combined_insn (insn)) > > + /* Do not accept an insn if hard register constraints are used. For > > + example, assume that the first insn is combined into the last one: > > + > > + r100=... > > + %5=... > > + r101=exp(r100) > > + > > + If the resulting insn has an operand which is constrained to hard > > + register %5, then this introduces a conflict since register %5 is live > > + at this point. Therefore, skip for now. This is a sledge hammer > > + approach. Ideally we would skip based on the fact whether a > > + combination crosses a hard register assignment and the corresponding > > + hard register is also referred by a single register constraint of the > > + resulting insn. */ > > + bool has_hard_reg_cstr = false; > > + extract_insn (insn); > > + for (int nop = recog_data.n_operands - 1; nop >= 0; --nop) > > + if (strchr (recog_data.constraints[nop], '{')) > > + { > > + has_hard_reg_cstr = true; > > + break; > > + } > > + > > + /* Don't accept hard register constraints. Allow targets to reject > > + combined insn. */ > > + if (has_hard_reg_cstr || !targetm.legitimate_combined_insn (insn)) > > { > > if (dump_file && (dump_flags & TDF_DETAILS)) > > - fputs ("Instruction not appropriate for target.", > > - dump_file); > > + { > > + if (has_hard_reg_cstr) > > + fputs ("Instruction makes use of hard register constraints.", > > + dump_file); > > + else > > + fputs ("Instruction not appropriate for target.", > > + dump_file); > > + } > > > > /* Callers expect recog_for_combine to strip > > clobbers from the pattern on failure. */ > > -- > > 2.52.0 > >
