On Thu, May 28, 2026 at 10:02:17PM +0200, Georg-Johann Lay wrote:
> This patch proceeds converting insns with explicit hard registers to
> hard-reg constraints, this time for FMUL, FMULS and FMULSU instructions
> resp. their libgcc calls when no MUL is supported.
>
> One question I have for Stefan:
> ===============================
>
> The patch handles two orthogonal cases:
>
> A) Operation is commutative (fmul, fmuls) or not (fmulsu).
>
> B) Core supports MUL (use fmul* instruction) or doesn't support
> MUL (generate transparent libgcc call).
>
> I have tried two approaches to model commutativity of the inputs
> in the libgcc case. These are the constraint alternatives with
> hard-regs:
>
> 1) Using the "%" constraint modifier like:
> "={r22}" ;; output
> "%{r24}" ;; 1st input
> "{r25}" ;; 2nd input
>
> 2) Using two constraint alternatives like:
> "={r22},{r22}" ;; output
> "{r24},{r25}" ;; 1st input
> "{r25},{r24}" ;; 2nd input
>
> Neither of which is working, i.e. RA always picks the first alternative.
>
> Are there plans to implement this?
>
> If yes, then what way to go? Using "%" is much more natural IMHO, and it
> allows for a neat, compact and intuitive description. I would retain
> the patch until the work is upstream, and then use the way as you
> recommend.
>
> If not, I would just remove any attempts that try to exploit commutative
> operations and add a respective comment.
>
> I have attached a test case fmul.c.
>
> $ avr-gcc fmul.c -S -Os -mmcu=avr4 # takes the MUL route.
> $ avr-gcc fmul.c -S -Os -mmcu=avr2 # takes the no-MUL route.
>
> It has this test case:
>
> uint16_t gun2 ()
> {
> uint8_t a, b;
> __asm (";; %0 %1" : "={r24}" (a), "={r25}" (b));
> return fmul (b, a);
> }
>
> which in the avr2 no-MUL path swaps r24 and r25, even though the
> operands of fmul commute and the patch tries to cater for that:
>
> gun2:
> /* #APP */
> ;; r24 r25
> /* #NOAPP */
> -> mov r18,r25 ; 20 [c=4 l=1] movqi_insn/0
> -> mov r25,r24 ; 21 [c=4 l=1] movqi_insn/0
> -> mov r24,r18 ; 22 [c=4 l=1] movqi_insn/0
> rcall __fmul ; 23 [c=8 l=1] *fmul.call/0
> mov r24,r22 ; 29 [c=4 l=1] movqi_insn/0
> mov r25,r23 ; 30 [c=4 l=1] movqi_insn/0
> /* epilogue start */
> ret ; 27 [c=0 l=1] return
The problem is not within the constraint modifier but rather how IRA
allocates. In your example we have
(insn 7 2 8 2 (parallel [
(set (reg:QI 46 [ a ])
(asm_operands:QI (";; %0 %1") ("={r24}") 0 []
[]
[] t.c:16))
(set (reg:QI 47 [ b ])
(asm_operands:QI (";; %0 %1") ("={r25}") 1 []
[]
[] t.c:16))
(clobber (reg:CC 36 cc))
]) "t.c":16:3 -1
(expr_list:REG_UNUSED (reg:CC 36 cc)
(nil)))
(insn 8 7 12 2 (parallel [
(set (reg:HI 45 [ <retval> ])
(unspec:HI [
(reg:QI 47 [ b ])
(reg:QI 46 [ a ])
] UNSPEC_FMUL))
(clobber (scratch:HI))
]) "t.c":10:10 1068 {fmul}
(expr_list:REG_DEAD (reg:QI 47 [ b ])
(expr_list:REG_DEAD (reg:QI 46 [ a ])
(nil))))
for which IRA allocates
Popping a0(r45,l0) -- assign reg 24
Popping a1(r47,l0) -- assign reg 24
Popping a2(r46,l0) -- assign reg 25
which then has to be fixed-up by LRA
7: {r51:QI=asm_operands;r52:QI=asm_operands;clobber cc:CC;}
REG_UNUSED cc:CC
Inserting insn reload after:
17: r47:QI=r52:QI
16: r46:QI=r51:QI
so we end up with swapped operands
(insn 7 2 17 2 (parallel [
(set (reg:QI 24 r24 [orig:46 a ] [46])
(asm_operands:QI (";; %0 %1") ("={r24}") 0 []
[]
[] t.c:16))
(set (reg:QI 25 r25 [orig:47 b ] [47])
(asm_operands:QI (";; %0 %1") ("={r25}") 1 []
[]
[] t.c:16))
(clobber (reg:CC 36 cc))
]) "t.c":16:3 -1
(nil))
(insn 17 7 16 2 (set (reg:QI 18 r18 [orig:47 b ] [47])
(reg:QI 25 r25 [orig:47 b ] [47])) "t.c":16:3 113 {movqi_insn_split}
(nil))
(insn 16 17 19 2 (set (reg:QI 25 r25 [orig:46 a ] [46])
(reg:QI 24 r24 [orig:46 a ] [46])) "t.c":16:3 113 {movqi_insn_split}
(nil))
(insn 19 16 8 2 (set (reg:QI 24 r24 [orig:47 b ] [47])
(reg:QI 18 r18 [orig:47 b ] [47])) "t.c":10:10 113 {movqi_insn_split}
(nil))
(insn 8 19 18 2 (parallel [
(set (reg:HI 22 r22 [orig:45 <retval> ] [45])
(unspec:HI [
(reg:QI 24 r24 [orig:47 b ] [47])
(reg:QI 25 r25 [orig:46 a ] [46])
] UNSPEC_FMUL))
(clobber (reg:HI 24 r24 [50]))
]) "t.c":10:10 1068 {fmul}
(nil))
I already have a fix for this (see attachment). I've only tested the
patch on s390 and see some fallout but conceptually that is the right
direction. However, this patch is already in my queue for months but I
hope to get back to this some time ...
Long story short, with the attached patch we have after IRA
Popping a0(r45,l0) -- assign reg 18
Popping a1(r47,l0) -- assign reg 25
Popping a2(r46,l0) -- assign reg 24
so that LRA doesn't have to fix-up anything which means we end up with
(insn 7 2 8 2 (parallel [
(set (reg:QI 24 r24 [orig:46 a ] [46])
(asm_operands:QI (";; %0 %1") ("={r24}") 0 []
[]
[] t.c:16))
(set (reg:QI 25 r25 [orig:47 b ] [47])
(asm_operands:QI (";; %0 %1") ("={r25}") 1 []
[]
[] t.c:16))
(clobber (reg:CC 36 cc))
]) "t.c":16:3 -1
(nil))
(insn 8 7 16 2 (parallel [
(set (reg:HI 22 r22 [orig:45 <retval> ] [45])
(unspec:HI [
(reg:QI 24 r24 [orig:46 a ] [46])
(reg:QI 25 r25 [orig:47 b ] [47])
] UNSPEC_FMUL))
(clobber (reg:HI 24 r24 [50]))
]) "t.c":10:10 1068 {fmul}
(nil))
Note, how the input operands swapped in insn 8. Again, the constraint
modifier works, it is just an unfortunate allocation by IRA which had to
be fixed up by LRA.
The wip patch follows constraints associated a single register class. I
will let you know once I have a patch properly tested and polished up.
Cheers,
Stefan
>From e47268addb35f990e686daafd640fe177b359aa3 Mon Sep 17 00:00:00 2001
From: Stefan Schulze Frielinghaus <[email protected]>
Date: Fri, 9 Jan 2026 11:17:59 +0100
Subject: [WIP] ira: Treat hard register constraints similarly as single reg
constraints
From: Stefan Schulze Frielinghaus <[email protected]>
Treat a hard register constraint the same way as a constraint associated
a single register class for process_single_reg_class_operands().
---
gcc/ira-lives.cc | 78 +++++++++++--------
.../gcc.target/s390/asm-hard-reg-1.c | 4 +-
2 files changed, 49 insertions(+), 33 deletions(-)
diff --git a/gcc/ira-lives.cc b/gcc/ira-lives.cc
index a6b3eff1ab8..4cb3dbc21f8 100644
--- a/gcc/ira-lives.cc
+++ b/gcc/ira-lives.cc
@@ -846,16 +846,16 @@ mark_hard_reg_early_clobbers (rtx_insn *insn, bool live_p)
}
/* Checks that CONSTRAINTS permits to use only one hard register. If
- it is so, the function returns the class of the hard register.
- Otherwise it returns NO_REGS. */
-static enum reg_class
+ it is so, the function returns the register. Otherwise it returns -1. */
+static int
single_reg_class (const char *constraints, rtx op, rtx equiv_const)
{
int c;
- enum reg_class cl, next_cl;
+ int regno, next_regno;
+ enum reg_class next_cl;
enum constraint_num cn;
- cl = NO_REGS;
+ regno = -1;
alternative_mask preferred = preferred_alternatives;
while ((c = *constraints))
{
@@ -867,7 +867,15 @@ single_reg_class (const char *constraints, rtx op, rtx
equiv_const)
switch (c)
{
case 'g':
- return NO_REGS;
+ return -1;
+
+ case '{':
+ next_regno = decode_hard_reg_constraint (constraints);
+ if (regno >= 0 && regno != next_regno)
+ return -1;
+ regno = next_regno;
+ break;
+
default:
/* ??? Is this the best way to handle memory constraints? */
@@ -876,21 +884,21 @@ single_reg_class (const char *constraints, rtx op, rtx
equiv_const)
|| insn_extra_special_memory_constraint (cn)
|| insn_extra_relaxed_memory_constraint (cn)
|| insn_extra_address_constraint (cn))
- return NO_REGS;
+ return -1;
if (constraint_satisfied_p (op, cn)
|| (equiv_const != NULL_RTX
&& CONSTANT_P (equiv_const)
&& constraint_satisfied_p (equiv_const, cn)))
- return NO_REGS;
+ return -1;
next_cl = reg_class_for_constraint (cn);
if (next_cl == NO_REGS)
break;
- if (cl == NO_REGS
- ? ira_class_singleton[next_cl][GET_MODE (op)] < 0
- : (ira_class_singleton[cl][GET_MODE (op)]
- != ira_class_singleton[next_cl][GET_MODE (op)]))
- return NO_REGS;
- cl = next_cl;
+ next_regno = ira_class_singleton[next_cl][GET_MODE (op)];
+ if (regno < 0
+ ? next_regno < 0
+ : regno != next_regno)
+ return -1;
+ regno = next_regno;
break;
case '0': case '1': case '2': case '3': case '4':
@@ -899,31 +907,30 @@ single_reg_class (const char *constraints, rtx op, rtx
equiv_const)
char *end;
unsigned long dup = strtoul (constraints, &end, 10);
constraints = end;
- next_cl
+ next_regno
= single_reg_class (recog_data.constraints[dup],
recog_data.operand[dup], NULL_RTX);
- if (cl == NO_REGS
- ? ira_class_singleton[next_cl][GET_MODE (op)] < 0
- : (ira_class_singleton[cl][GET_MODE (op)]
- != ira_class_singleton[next_cl][GET_MODE (op)]))
- return NO_REGS;
- cl = next_cl;
+ if (regno < 0
+ ? next_regno < 0
+ : regno != next_regno)
+ return -1;
+ regno = next_regno;
continue;
}
}
constraints += CONSTRAINT_LEN (c, constraints);
}
- return cl;
+ return regno;
}
/* The function checks that operand OP_NUM of the current insn can use
only one hard register. If it is so, the function returns the
- class of the hard register. Otherwise it returns NO_REGS. */
-static enum reg_class
+ hard register. Otherwise it returns -1. */
+static int
single_reg_operand_class (int op_num)
{
if (op_num < 0 || recog_data.n_alternatives == 0)
- return NO_REGS;
+ return -1;
return single_reg_class (recog_data.constraints[op_num],
recog_data.operand[op_num], NULL_RTX);
}
@@ -991,7 +998,7 @@ ira_implicitly_set_insn_hard_regs (HARD_REG_SET *set,
static void
process_single_reg_class_operands (bool in_p, int freq)
{
- int i, regno;
+ int i, regno, hard_regno;
unsigned int px;
enum reg_class cl;
rtx operand;
@@ -1006,8 +1013,8 @@ process_single_reg_class_operands (bool in_p, int freq)
if (! in_p && recog_data.operand_type[i] != OP_OUT
&& recog_data.operand_type[i] != OP_INOUT)
continue;
- cl = single_reg_operand_class (i);
- if (cl == NO_REGS)
+ hard_regno = single_reg_operand_class (i);
+ if (hard_regno < 0)
continue;
operand_a = NULL;
@@ -1020,6 +1027,11 @@ process_single_reg_class_operands (bool in_p, int freq)
{
enum reg_class aclass;
+ cl = NO_REGS;
+ int nregs = hard_regno_nregs (hard_regno, recog_data.operand_mode[i]);
+ for (int j = 0; j < nregs; ++j)
+ cl = reg_class_superunion[cl][REGNO_REG_CLASS (hard_regno + j)];
+
operand_a = ira_curr_regno_allocno_map[regno];
aclass = ALLOCNO_CLASS (operand_a);
if (ira_class_subset_p[cl][aclass])
@@ -1036,7 +1048,7 @@ process_single_reg_class_operands (bool in_p, int freq)
poly_int64 offset;
xmode = recog_data.operand_mode[i];
- xregno = ira_class_singleton[cl][xmode];
+ xregno = hard_regno;
gcc_assert (xregno >= 0);
ymode = ALLOCNO_MODE (operand_a);
offset = subreg_lowpart_offset (ymode, xmode);
@@ -1059,6 +1071,10 @@ process_single_reg_class_operands (bool in_p, int freq)
}
}
+ HARD_REG_SET hard_regs;
+ CLEAR_HARD_REG_SET (hard_regs);
+ add_to_hard_reg_set (&hard_regs, recog_data.operand_mode[i], hard_regno);
+
EXECUTE_IF_SET_IN_SPARSESET (objects_live, px)
{
ira_object_t obj = ira_object_id_map[px];
@@ -1068,8 +1084,8 @@ process_single_reg_class_operands (bool in_p, int freq)
/* We could increase costs of A instead of making it
conflicting with the hard register. But it works worse
because it will be spilled in reload in anyway. */
- OBJECT_CONFLICT_HARD_REGS (obj) |= reg_class_contents[cl];
- OBJECT_TOTAL_CONFLICT_HARD_REGS (obj) |= reg_class_contents[cl];
+ OBJECT_CONFLICT_HARD_REGS (obj) |= hard_regs;
+ OBJECT_TOTAL_CONFLICT_HARD_REGS (obj) |= hard_regs;
}
}
}
diff --git a/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
index 671c0ede6ef..53ac46eb40c 100644
--- a/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
+++ b/gcc/testsuite/gcc.target/s390/asm-hard-reg-1.c
@@ -75,8 +75,8 @@ test_out_1 (void)
** test_out_2:
** lgr (%r[0-9]+),%r2
** foo %r2
-** ark (%r[0-9]+),\1,%r2
-** lgfr %r2,\2
+** ar %r2,\1
+** lgfr %r2,%r2
** br %r14
*/
--
2.54.0
