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. > > Thx, > -Vineet
