Am 29.05.26 um 15:31 schrieb Stefan Schulze Frielinghaus:
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.
Hi Stefan,
Thanks for the details analysis.
With you patch applied, does it work since the insn uses constraint
modifier "%" ?
Or because the insn provides an alternative with swapped hard-reg
constraints?
Johann
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
