diff mbox series

[3/3] WIP combine: Skip in case of single register constraints [PR121426]

Message ID 20260110161159.1073435-4-stefansf@linux.ibm.com
State New
Headers
Series Take single register constraints into account |

Commit Message

Stefan Schulze Frielinghaus Jan. 10, 2026, 4:11 p.m. UTC 
  From: Stefan Schulze Frielinghaus <stefansf@gcc.gnu.org>

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 skipping any logical link if a
single register constraint is possibly involved.  Ideally we would skip
based on the fact whether there is any usage of a hard register referred
by any single register constraint between INSN and USE_INSN during
combine.

gcc/ChangeLog:

	* combine.cc (has_single_register_constraint_p): New function.
	(create_log_links): Do not build LOG_LINKs between insns
	containing single register constraints.
---
 gcc/combine.cc | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

Comments

Jeffrey Law Jan. 13, 2026, 3:22 p.m. UTC | #1 
On 1/10/2026 9:11 AM, Stefan Schulze Frielinghaus wrote:
> From: Stefan Schulze Frielinghaus <stefansf@gcc.gnu.org>
>
> 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 skipping any logical link if a
> single register constraint is possibly involved.  Ideally we would skip
> based on the fact whether there is any usage of a hard register referred
> by any single register constraint between INSN and USE_INSN during
> combine.
>
> gcc/ChangeLog:
>
> 	* combine.cc (has_single_register_constraint_p): New function.
> 	(create_log_links): Do not build LOG_LINKs between insns
> 	containing single register constraints.
Yea, this is a lot like the problems we have with return values and more 
generally classes with a single hard register.  I would suggest looking 
at can't combine_insn_p which has the likely_spilled checks in it.
Jeff
diff mbox series

Patch

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 816324f4735..1ea99d1697c 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -76,6 +76,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "regs.h"
 #include "emit-rtl.h"
 #include "recog.h"
+#include "ira.h"
 #include "cgraph.h"
 #include "stor-layout.h"
 #include "cfgrtl.h"
@@ -1020,6 +1021,60 @@  can_combine_use_p (df_ref use)
   return true;
 }
 
+/* Return true if INSN has a single register constraint like a constraint
+   associated with a single register class or a hard register constraint.
+   Otherwise return false.  */
+static bool
+has_single_register_constraint_p (rtx_insn *insn)
+{
+  extract_insn (insn);
+  for (int nop = recog_data.n_operands - 1; nop >= 0; --nop)
+    {
+      int c;
+      const char *p = recog_data.constraints[nop];
+      for (; (c = *p); p += CONSTRAINT_LEN (c, p))
+	if (c == ',')
+	  ;
+	else if (c == '{')
+	  return true;
+      /* For the moment ignore constraints with single register classes since
+	 this would introduce missed optimizations.  Although, this literally
+	 means we ICE in certain cases.  For example, on powerpc64le tests
+
+	 FAIL: gcc.dg/cpp/_Pragma3.c (test for excess errors)
+	 FAIL: gcc.target/powerpc/fold-vec-extract-char.p7.c scan-assembler-times \\mrldicl\\M|\\mrlwinm\\M 3
+	 FAIL: gcc.target/powerpc/fold-vec-extract-short.p7.c scan-assembler-times \\mrldic\\M|\\mrlwinm\\M 3
+	 FAIL: gcc.target/powerpc/fusion-p10-ldcmpi.c scan-assembler-times lbz_cmpldi_cr0_QI_clobber_CCUNS_zero 4
+	 FAIL: gcc.target/powerpc/fusion-p10-ldcmpi.c scan-assembler-times lha_cmpdi_cr0_HI_clobber_CC_sign 16
+	 FAIL: gcc.target/powerpc/fusion-p10-ldcmpi.c scan-assembler-times lhz_cmpldi_cr0_HI_clobber_CCUNS_zero 4
+	 FAIL: gcc.target/powerpc/p9-splat-2.c scan-assembler-times lxvwsx 1
+	 FAIL: gcc.target/powerpc/p9-splat-2.c scan-assembler-times xscvdpspn 2
+	 FAIL: gcc.target/powerpc/p9-splat-2.c scan-assembler-times xxspltw 2
+	 FAIL: gcc.target/powerpc/prefix-ds-dq.c scan-assembler-times \\mlha\\M 1
+	 FAIL: gcc.target/powerpc/prefix-ds-dq.c scan-assembler-times \\mlhz\\M 1
+	 FAIL: gcc.target/powerpc/vector_float.c scan-assembler-times \\mrldimi\\M 2
+	 FAIL: gcc.target/powerpc/vsx-builtin-7.c scan-assembler-times xxpermdi 35
+
+	 would fail because constraint c is referring to the single count
+	 register.  This should be fixed by not using a sledge hammer as
+	 described below.  */
+#if 0
+	else
+	  {
+	    enum reg_class cl
+	      = reg_class_for_constraint (lookup_constraint (p));
+	    if (cl == NO_REGS)
+	      continue;
+	    machine_mode mode = recog_data.operand_mode[nop];
+	    int regno = ira_class_singleton[cl][mode];
+	    if (regno >= 0)
+	      return true;
+	  }
+#endif
+    }
+  return false;
+}
+
 /* Fill in log links field for all insns.  */
 
 static void
@@ -1051,6 +1106,9 @@  create_log_links (void)
 	  /* Log links are created only once.  */
 	  gcc_assert (!LOG_LINKS (insn));
 
+	  bool has_single_reg_cstr
+	    = has_single_register_constraint_p (insn);
+
 	  FOR_EACH_INSN_DEF (def, insn)
             {
               unsigned int regno = DF_REF_REGNO (def);
@@ -1079,6 +1137,23 @@  create_log_links (void)
 		  && asm_noperands (PATTERN (use_insn)) >= 0)
 		continue;
 
+	      /* Do not build LOG_LINKs between insns containing single
+		 register constraints.  For example, assume that in the first
+		 insn operand r100 of exp_a is constrained to hard register %5.
+
+		 r101=exp_a(r100)
+		 %5=...
+		 r102=exp_b(r101)
+
+		 Then combining the first insn into the last one creates a
+		 conflict for pseudo r100 since hard register %5 is live.
+		 Therefore, skip for now.  This is a sledge hammer approach.
+		 Ideally we would skip based on the fact whether there is any
+		 definition of a hard register used in a single register
+		 constraint between INSN and USE_INSN.  */
+	      if (has_single_reg_cstr)
+		continue;
+
 	      /* Don't add duplicate links between instructions.  */
 	      struct insn_link *links;
 	      FOR_EACH_LOG_LINK (links, use_insn)