On 3/16/24 13:21, Jeff Law wrote:
> | 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.
To be clear are u suggesting ADD sp, sp, 2047 is ABI compliant ?
I'd still like to avoid it as I'm sure someone will complain about it.
>> 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.
2033..4064 vs. 2048..4094
Yeah I was a bit split about this as well. Since you are OK with either,
I'll keep them as-is and perhaps add this observation to commitlog.
Thx,
-Vineet
