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

[Jeff Law <jeffreyalaw@gmail.com>]

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