On 3/19/24 12:48 AM, Andrew Waterman wrote:
On Mon, Mar 18, 2024 at 5:28 PM Vineet Gupta <vine...@rivosinc.com> wrote:
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.
There's a subset of embedded use cases where an interrupt service
routine continues on the same stack as the interrupted thread,
requiring sp to always have an ABI-compliant value (i.e. 16B aligned,
and with no important data on the stack at an address below sp).
Although not all use cases care about this property, it seems more
straightforward to maintain the invariant everywhere, rather than
selectively enforce it.
Just to be clear, the changes don't misalign the stack pointer at all.
They merely have the potential to create *another* pointer into the
stack which may or may not be aligned. Which is totally normal, it's no
different than taking the address of a char on the stack.
jeff