On Mon, Jan 19, 2026 at 02:26:41PM +0100, Georg-Johann Lay wrote:
> Am 14.01.26 um 14:55 schrieb Stefan Schulze Frielinghaus:
> > From: Stefan Schulze Frielinghaus <[email protected]>
> > 
> > I have put the check into cant_combine_insn_p().  However, I still
> > haven't convinced myself that this is a full fledged solution.  Even the
> > current behaviour for constraints associated a single register class
> > seems like only partially solved to me.  For the current case it would
> > solve the problem because we are substituting into a reg-reg move.
> > However, my current understanding is that I could actually make use of
> > hard registers in arbitrary patterns which wouldn't be dealt with.
> > 
> > With this patch I'm conservative when hard register constraints are
> > involved and skip any combination.
> > 
> > In light of not having real world examples were it breaks also for
> > constraints associated a single register class and being in stage 4, I'm
> > limiting this to hard register constraints for now.
> > 
> > @Jeff ok for mainline assuming that bootstrap and regtest are successful
> > for x86_64 and s390?
> > 
> > @Johann if you have a larger test than what you provided in the PR,
> > could you give this patch a try?  I can confirm that with the machine
> > description patch provided in the PR, the example from the PR compiles
> > fine if this patch is applied.  If you still see failures in more
> > complex examples you might also want to apply
> > https://gcc.gnu.org/pipermail/gcc-patches/2026-January/705494.html
> > https://gcc.gnu.org/pipermail/gcc-patches/2026-January/705495.html
> > Note, I haven't tested hard register constraints in clobbers so far, and
> > would rather assume that they do not work as expected.  Once I find some
> > time, I will look into this.
> 
> Hi Stefan,
> 
> I reran the tests, and there are still some ICEs.
> 
> GCC is:
> 
> commit 1367db23a5626499468d79b3c12a9b1859e7b819 (HEAD -> master, 
> origin/master, origin/HEAD)
> Author: Tobias Burnus <[email protected]>
> Date:   Mon Jan 19 12:17:59 2026 +0100
> 
> Tests ran with x.diff. z.diff doesn't have an effect on the outcome.

Hmm neither x.diff nor z.diff contain the patch you are currently
replying to, i.e., the combine patch is missing.  Did you still apply
the combine patch?

> Notice that x.diff is only a very small fraction of avr insns that
> would use HRCs.
> 
> Tests that now fail, but worked before (7 tests):
> 
> atmega128-sim: gcc: gcc.c-torture/execute/pr94412.c   -O1  (test for excess 
> errors)
> atmega128-sim: gcc: gcc.c-torture/execute/pr94412.c   -O2  (test for excess 
> errors)
> atmega128-sim: gcc: gcc.c-torture/execute/pr94412.c   -O2 -flto 
> -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
> atmega128-sim: gcc: gcc.c-torture/execute/pr94412.c   -O3 -g  (test for 
> excess errors)
> atmega128-sim: gcc: gcc.c-torture/execute/pr94412.c   -Os  (test for excess 
> errors)
> atmega128-sim: gcc: gcc.dg/loop-invariant-2.c (test for excess errors)
> atmega128-sim: gcc: gcc.dg/tree-ssa/ssa-lim-23.c (test for excess errors)
> 
> The ICEs are like
> 
> 
> /home/gjl/gnu/source/gcc-x/gcc/testsuite/gcc.c-torture/execute/pr94412.c: In 
> function 'bar':
> /home/gjl/gnu/source/gcc-x/gcc/testsuite/gcc.c-torture/execute/pr94412.c:15:1:
>  error: unable to find a register to spill
> /home/gjl/gnu/source/gcc-x/gcc/testsuite/gcc.c-torture/execute/pr94412.c:15:1:
>  error: this is the insn:
> (insn 12 28 26 2 (parallel [
>             (set (subreg:HI (reg:SI 73 [orig:52 _3 ] [52]) 0)
>                 (udiv:HI (reg:HI 74 [58])
>                     (reg:HI 75 [orig:53 _9 ] [53])))
>             (set (reg:HI 76 [57])
>                 (umod:HI (reg:HI 74 [58])
>                     (reg:HI 75 [orig:53 _9 ] [53])))
>             (clobber (reg:HI 77 [67]))
>             (clobber (reg:QI 78 [68]))
>         ]) 
> "/home/gjl/gnu/source/gcc-x/gcc/testsuite/gcc.c-torture/execute/pr94412.c":14:12
>  604 {udivmodhi4}
>      (expr_list:REG_UNUSED (reg:QI 78 [68])
>         (expr_list:REG_UNUSED (reg:HI 77 [67])
>             (expr_list:REG_UNUSED (reg:HI 76 [57])
>                 (expr_list:REG_DEAD (reg:HI 75 [orig:53 _9 ] [53])
>                     (expr_list:REG_DEAD (reg:HI 74 [58])
>                         (nil)))))))
> during RTL pass: reload
> /home/gjl/gnu/source/gcc-x/gcc/testsuite/gcc.c-torture/execute/pr94412.c:15:1:
>  internal compiler error: in lra_split_hard_reg_for, at lra-assigns.cc:1882
> 0x1e4823c internal_error(char const*, ...)
>       ../../../source/gcc-x/gcc/diagnostic-global-context.cc:787
> 0x8bdd15 fancy_abort(char const*, int, char const*)
>       ../../../source/gcc-x/gcc/diagnostics/context.cc:1805
> 0x7597ff _fatal_insn(char const*, rtx_def const*, char const*, int, char 
> const*)
>       ../../../source/gcc-x/gcc/rtl-error.cc:108
> 0xe39dc6 lra_split_hard_reg_for(bool)
>       ../../../source/gcc-x/gcc/lra-assigns.cc:1882
> 0xe338b6 lra(_IO_FILE*, int)
>       ../../../source/gcc-x/gcc/lra.cc:2535
> 0xde954f do_reload
>       ../../../source/gcc-x/gcc/ira.cc:6076
> 0xde954f execute
>       ../../../source/gcc-x/gcc/ira.cc:6264
> 
> As far as I understand, HRC won't work with scratches,
> but almost all avr insns that would want to use HRCs have
> hard reg clobbers. Such insns represent libgcc calls with
> a reg footprint smaller than the ABI, and such functions
> usually clobber at least some regs.
> 
> What would happen to hard-reg clobbers under HRC? Wrong code?

I haven't looked into this and cannot tell right away.

> 
> Moreover, quite some insns rely on insn combine, e.g. the
> ones for mul with sign-, zero- or one-extended operands.
> AFAIU, HRCs are incompatible with insn combine, so that HRCs
> are no alternative to the current hard-reg operands?
> 
> There are almost no insns that don't need insn combine *and*
> don't have hard-reg clobbers, so that almost nothing of HRC
> is left to be usable in the avr backend :-(
> 
> It seems a bit strange that insn combine would interfere with
> RA.  Some problems are when insn combine (or other passes) are
> propagating hard regs into operands, but they don't match constraints
> and RA has to kick them out again, perhaps resulting in sub-optimal code.
> But in no case I saw RA ICEing.

That is because so far RA had a choice here when regular register
constraints were used with a register class with multiple registers.
With hard register constraints, however, you really pin a pseudo to a
particular hard register and if that is already live due to an
assignment to that hard register you error out because it cannot be
reloaded.  You run into the very same problem when you create single
register classes and assign them to new constraints and use those
instead of hard register constraints.

Cheers,
Stefan

> 
> 
> Johann
> 
> 
> > -- 8< --
> > 
> > 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 potential
> > combination if a hard register constraint is involved.  Ideally we would
> > skip based on the fact whether there is any usage of a hard register
> > referred by any hard register constraint between potentially combined
> > insns.
> > 
> > gcc/ChangeLog:
> > 
> >     * combine.cc (cant_combine_insn_p): Do not try to combine insns
> >     if hard register constraints are involved.
> > ---
> >   gcc/combine.cc | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> > 
> > diff --git a/gcc/combine.cc b/gcc/combine.cc
> > index 816324f4735..a4e7aff971d 100644
> > --- a/gcc/combine.cc
> > +++ b/gcc/combine.cc
> > @@ -2229,6 +2229,27 @@ cant_combine_insn_p (rtx_insn *insn)
> >     if (!NONDEBUG_INSN_P (insn))
> >       return true;
> > +  /* Do not try to combine insns which make use of hard 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 for the last insn.  
> > 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 potentially combined insns.  
> > */
> > +
> > +  extract_insn (insn);
> > +  for (int nop = recog_data.n_operands - 1; nop >= 0; --nop)
> > +    {
> > +      if (strchr (recog_data.constraints[nop], '{'))
> > +   return true;
> > +    }
> > +
> >     /* Never combine loads and stores involving hard regs that are likely
> >        to be spilled.  The register allocator can usually handle such
> >        reg-reg moves by tying.  If we allow the combiner to make

> commit 4725614255a2a5a428048db76ab931434c829758
> Author: Georg-Johann Lay <[email protected]>
> Date:   Tue Aug 5 14:54:35 2025 +0200
> 
>     htc: [u]divmod hi
> 
> diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
> index 30a02a4b6c9..ebfc16104c3 100644
> --- a/gcc/config/avr/avr.md
> +++ b/gcc/config/avr/avr.md
> @@ -3846,32 +3846,14 @@ (define_insn "*udivmodqi4_call"
>    [(set_attr "type" "xcall")])
>  
>  (define_insn_and_split "divmodhi4"
> -  [(set (match_operand:HI 0 "pseudo_register_operand")
> -        (div:HI (match_operand:HI 1 "pseudo_register_operand")
> -                (match_operand:HI 2 "pseudo_register_operand")))
> -   (set (match_operand:HI 3 "pseudo_register_operand")
> -        (mod:HI (match_dup 1) (match_dup 2)))
> -   (clobber (reg:QI 21))
> -   (clobber (reg:HI 22))
> -   (clobber (reg:HI 24))
> -   (clobber (reg:HI 26))]
> -  ""
> -  { gcc_unreachable(); }
> -  ""
> -  [(set (reg:HI 24) (match_dup 1))
> -   (set (reg:HI 22) (match_dup 2))
> -   (parallel [(set (reg:HI 22) (div:HI (reg:HI 24) (reg:HI 22)))
> -              (set (reg:HI 24) (mod:HI (reg:HI 24) (reg:HI 22)))
> -              (clobber (reg:HI 26))
> -              (clobber (reg:QI 21))])
> -   (set (match_dup 0) (reg:HI 22))
> -   (set (match_dup 3) (reg:HI 24))])
> -
> -(define_insn_and_split "*divmodhi4_call_split"
> -  [(set (reg:HI 22) (div:HI (reg:HI 24) (reg:HI 22)))
> -   (set (reg:HI 24) (mod:HI (reg:HI 24) (reg:HI 22)))
> -   (clobber (reg:HI 26))
> -   (clobber (reg:QI 21))]
> +  [(set (match_operand:HI 0 "register_operand"        "={r22}")
> +        (div:HI (match_operand:HI 1 "register_operand" "{r24}")
> +                (match_operand:HI 2 "register_operand" "{r22}")))
> +   (set (match_operand:HI 3 "register_operand"        "={r24}")
> +        (mod:HI (match_dup 1)
> +                (match_dup 2)))
> +   (clobber (match_scratch:HI 4                       "={r26}"))
> +   (clobber (match_scratch:QI 5                       "={r21}"))]
>    ""
>    "#"
>    "&& reload_completed"
> @@ -3889,32 +3871,14 @@ (define_insn "*divmodhi4_call"
>    [(set_attr "type" "xcall")])
>  
>  (define_insn_and_split "udivmodhi4"
> -  [(set (match_operand:HI 0 "pseudo_register_operand")
> -        (udiv:HI (match_operand:HI 1 "pseudo_register_operand")
> -                 (match_operand:HI 2 "pseudo_register_operand")))
> -   (set (match_operand:HI 3 "pseudo_register_operand")
> -        (umod:HI (match_dup 1) (match_dup 2)))
> -   (clobber (reg:QI 21))
> -   (clobber (reg:HI 22))
> -   (clobber (reg:HI 24))
> -   (clobber (reg:HI 26))]
> -  ""
> -  { gcc_unreachable(); }
> -  ""
> -  [(set (reg:HI 24) (match_dup 1))
> -   (set (reg:HI 22) (match_dup 2))
> -   (parallel [(set (reg:HI 22) (udiv:HI (reg:HI 24) (reg:HI 22)))
> -              (set (reg:HI 24) (umod:HI (reg:HI 24) (reg:HI 22)))
> -              (clobber (reg:HI 26))
> -              (clobber (reg:QI 21))])
> -   (set (match_dup 0) (reg:HI 22))
> -   (set (match_dup 3) (reg:HI 24))])
> -
> -(define_insn_and_split "*udivmodhi4_call_split"
> -  [(set (reg:HI 22) (udiv:HI (reg:HI 24) (reg:HI 22)))
> -   (set (reg:HI 24) (umod:HI (reg:HI 24) (reg:HI 22)))
> -   (clobber (reg:HI 26))
> -   (clobber (reg:QI 21))]
> +  [(set (match_operand:HI 0 "register_operand"         "={r22}")
> +        (udiv:HI (match_operand:HI 1 "register_operand" "{r24}")
> +                 (match_operand:HI 2 "register_operand" "{r22}")))
> +   (set (match_operand:HI 3 "register_operand"         "={r24}")
> +        (umod:HI (match_dup 1)
> +                 (match_dup 2)))
> +   (clobber (match_scratch:HI 4                        "={r26}"))
> +   (clobber (match_scratch:QI 5                        "={r21}"))]
>    ""
>    "#"
>    "&& reload_completed"
> @@ -3926,8 +3890,7 @@ (define_insn "*udivmodhi4_call"
>     (set (reg:HI 24) (umod:HI (reg:HI 24) (reg:HI 22)))
>     (clobber (reg:HI 26))
>     (clobber (reg:QI 21))
> -   (clobber (reg:CC REG_CC))
> -   ]
> +   (clobber (reg:CC REG_CC))]
>    "reload_completed"
>    "%~call __udivmodhi4"
>    [(set_attr "type" "xcall")])

> diff --git a/gcc/cse.cc b/gcc/cse.cc
> index a86fe307819..28175e2732b 100644
> --- a/gcc/cse.cc
> +++ b/gcc/cse.cc
> @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "backend.h"
>  #include "target.h"
>  #include "rtl.h"
> +#include "stmt.h"
>  #include "tree.h"
>  #include "cfghooks.h"
>  #include "df.h"
> @@ -31,6 +32,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "insn-config.h"
>  #include "regs.h"
>  #include "emit-rtl.h"
> +#include "ira.h"
>  #include "recog.h"
>  #include "cfgrtl.h"
>  #include "cfganal.h"
> @@ -6217,6 +6219,46 @@ invalidate_from_sets_and_clobbers (rtx_insn *insn)
>           invalidate (SET_DEST (y), VOIDmode);
>       }
>      }
> +
> +  /* Any single register constraint may introduce a conflict, if the 
> associated
> +     hard register is live.  For example:
> +
> +     r100=%1
> +     r101=42
> +     r102=exp(r101)
> +
> +     If the first operand r101 of exp is constrained to hard register %1, 
> then
> +     r100 cannot be trivially substituted by %1 in the following since %1 got
> +     clobbered.  Such conflicts may stem from single register classes as well
> +     as hard register constraints.  Since prior RA we do not know which
> +     alternative will be chosen, be conservative and consider any such hard
> +     register from any alternative as a potential clobber.  */
> +  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 == '{')
> +       {
> +         int regno = decode_hard_reg_constraint (p);
> +         machine_mode mode = recog_data.operand_mode[nop];
> +         invalidate_reg (gen_rtx_REG (mode, regno));
> +       }
> +     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)
> +           invalidate_reg (gen_rtx_REG (mode, regno));
> +       }
> +    }
>  }
>  
>  static rtx cse_process_note (rtx);
> diff --git a/gcc/testsuite/gcc.target/powerpc/asm-hard-reg-2.c 
> b/gcc/testsuite/gcc.target/powerpc/asm-hard-reg-2.c
> new file mode 100644
> index 00000000000..c0aad292acb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/asm-hard-reg-2.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +
> +double a;
> +double b (long double c) {
> +  long double d = 0;
> +  double e;
> +  __asm__ ("" : "=f" (a), "={fr2}" (e) : "{fr1}" (d));
> +  return c + c;
> +}

Reply via email to