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. 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
