On 3/16/24 11:35 AM, Vineet Gupta wrote:
Noticed that new sum of two s12 splitter was generating following:
| 059930 <tempnam>:
| 59930: add sp,sp,-16
| 59934: lui t0,0xfffff
| 59938: sd s0,0(sp)
| 5993c: sd ra,8(sp)
| 59940: add sp,sp,t0
| 59944: add s0,sp,2047 <----
| 59948: mv a2,a0
| 5994c: mv a3,a1
| 59950: mv a0,sp
| 59954: li a4,1
| 59958: lui a1,0x1
| 5995c: add s0,s0,1 <---
| 59960: jal 59a3c
SP here becomes unaligned, even if transitively which is undesirable as
well as incorrect:
- ABI requires stack to be 8 byte aligned
- asm code looks weird and unexpected
- to the user it might falsely seem like a compiler bug even when not,
specially when staring at asm for debugging unrelated issue.
It's not ideal, but I think it's still ABI compliant as-is. If it
wasn't, then I suspect things like virtual origins in Ada couldn't be
made ABI compliant.
Fix this by using 2032+addend idiom when handling register operands
related to stack. This only affects positive S12 values, negative values
are already -2048 based.
Unfortunately implementation requires making a copy of splitter, since
it needs different varaints of predicate and constraint which cant be
done conditionally in the same MD pattern (constraint with restricted
range prevents LRA from allowing such insn despite new predicate)
With the patch, we get following correct code instead:
| ..
| 59944: add s0,sp,2032
| ..
| 5995c: add s0,s0,16
Alternately you could tighten the positive side of the range of the
splitter from patch 1/3 so that you could always use 2032 rather than
2047 on the first addi. ie instead of allowing 2048..4094, allow
2048..4064.
I don't have a strong opinion on that vs the direction you've gone here.
Jeff
