Re: [gcc-15 2/3] RISC-V: avoid LUI based const mat: keep stack offsets aligned

[Jeff Law <jeffreyalaw@gmail.com>]

On 3/19/24 12:48 AM, Andrew Waterman wrote:
> On Mon, Mar 18, 2024 at 5:28 PM Vineet Gupta <vineetg@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