Am 05.08.25 um 15:58 schrieb Stefan Schulze Frielinghaus:
On Mon, Aug 04, 2025 at 01:04:36PM +0200, Georg-Johann Lay wrote:
Rewriting avr.md so it uses less explicit hard registers, I came
across the ICE below. I am using 2 patches as attached:
- A tentative fix for LRA https://gcc.gnu.org/PR121198
- A WIP to rewrite avr.md's divmodhi4 to use HRCs instead of
explicit hard regs. Outcome is that divmodhi4 will read
like this:
(define_insn_and_split "divmodhi4"
[(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"
[(scratch)]
{ DONE_ADD_CCC })
Then compile
void bar (int, char*);
void foo (int k)
{
bar (k / 5, "one");
}
$ avr-gcc spill.c -S -mmcu=avr4 -Os -da
spill.c: In function 'foo':
spill.c:6:1: error: unable to find a register to spill
6 | }
| ^
spill.c:6:1: error: this is the insn:
(insn 9 20 16 2 (parallel [
(set (reg:HI 53)
(div:HI (reg:HI 49 [ k ])
(reg:HI 54 [48])))
(set (reg:HI 55 [47])
(mod:HI (reg:HI 49 [ k ])
(reg:HI 54 [48])))
(clobber (reg:HI 56 [50]))
(clobber (reg:QI 57 [51]))
]) "spill.c":5:5 588 {divmodhi4}
(expr_list:REG_UNUSED (reg:QI 57 [51])
(expr_list:REG_UNUSED (reg:HI 56 [50])
(expr_list:REG_UNUSED (reg:HI 55 [47])
(expr_list:REG_DEAD (reg:HI 54 [48])
(expr_list:REG_DEAD (reg:HI 49 [ k ])
(nil)))))))
during RTL pass: reload
dump file: spill.c.326r.reload
spill.c:6:1: internal compiler error: in lra_split_hard_reg_for, at
lra-assigns.cc:1863
Some questions:
1) Is there anything wrong with the divmodhi4 pattern per se?
AFAIU match_dup is the right thing to do. HImode regs must start
in even registers, so that's fine too I think.
When the pattern itself is fine, I would open a PR for the ICE.
2) I had a look at the IRA dump. The assignments it comes up with
have nothing to do with the constraints:
divmodhi4 Regs before IRA
Op Reg IRA *=pseudo assigns
0 =r22:HI 24
1 r24:HI 49* 24
2 r22:HI 48* 18
3 =r24:HI 47* 18
4 =r26:HI 50* 20
5 =r21:QI 51* 30
That is: IRA comes up with assigns that have nothing to do with
the constraints of the insn. Doesn't that make it extra hard
for LRA to come up with the specified constraints?
IIUC, HRCs materialize in lra-constraints.cc, and IRA doesn't
know anything about them. Isn't that a perfect situation for
sub-optimal code? Since LRA will have to spill many of IRA's
choices, requiring a zoo off extra moves and frame locations
when reg pressure is high.
That is a nice finding. While implementing hard register constraints in
LRA I was looking for examples were it would have made sense to already
instruct IRA about preferred registers. Since everything could be fixed
up by LRA, I didn't want to prematurely implement something in IRA which
does not lead to better code and I never went on with that. My gut
feeling is that we see this especially on AVR since there values are
held in register pairs of more than two registers. So fixing it up
later is not that easy anymore if register pressure is high. I will
look more closely into this the coming days.
Cheers,
Stefan
Thank you for taking time. I opened https://gcc.gnu.org/PR121426 for it.
Johann
So I guess the problem is that IRA only works on the level of
register classes, and it cannot do anything about HRCs since
they don't have a register class in general?
Johann
diff --git a/gcc/lra-constraints.cc b/gcc/lra-constraints.cc
index 83f8fda3b52..0c09c9aa45b 100644
--- a/gcc/lra-constraints.cc
+++ b/gcc/lra-constraints.cc
@@ -2551,7 +2551,7 @@ process_alt_operands (int only_alternative)
{
int regno = decode_hard_reg_constraint (p);
gcc_assert (regno >= 0);
- cl = REGNO_REG_CLASS (regno);
+ cl = GENERAL_REGS;
CLEAR_HARD_REG_SET (hard_reg_constraint);
SET_HARD_REG_BIT (hard_reg_constraint, regno);
cl_filter = &hard_reg_constraint;
diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index 13493ad6891..496f70521ec 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -3773,32 +3773,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"
@@ -3816,32 +3798,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"
@@ -3853,8 +3817,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")])
