Re: [PATCH 3/4] [RISC-V] resolve confilct between zcmp multi push/pop and shrink-wrap-separate

[Jeff Law <jeffreyalaw@gmail.com>]

On 6/6/23 23:52, Fei Gao wrote:
> Disable zcmp multi push/pop if shrink-wrap-separate is active.
> 
> So in -Os that prefers smaller code size, by default shrink-wrap-separate
> is disabled while zcmp multi push/pop is enabled.
> 
> And in -O2 and others that prefers speed, by default shrink-wrap-separate
> is enabled while zcmp multi push/pop is disabled. To force enabling zcmp multi
> push/pop in this case, -fno-shrink-wrap-separate has to be explictly given.
> 
> The following TC shows the issues in -O2 before this patch with both
> shrink-wrap-separate and zcmp multi push/pop active.
> 1. duplicated store of s regs.
> 2. cm.push pushes ra, s0-s11 in reverse order than what normal
>     prologue does, causing stack corruption and failure to resotre s regs.
> 
> TC: zcmp_shrink_wrap_separate.c included in this patch.
> 
> output asm before this patch:
> calc_func:
> 	cm.push	{ra, s0-s3}, -32
> 	...
> 	beq	a5,zero,.L2
> 	...
> .L2:
> 	...
> 	sw	s1,20(sp) //issue here
> 	sw	s3,12(sp) //issue here
> 	...
> 	sw	s2,16(sp) //issue here
> 
> output asm after this patch:
> calc_func:
> 	addi	sp,sp,-32
> 	sw	s0,24(sp)
> 	...
> 	beq	a5,zero,.L2
> 	...
> .L2:
> 	...
> 	sw	s1,20(sp)
> 	sw	s3,12(sp)
> 	...
> 	sw	s2,16(sp)
> gcc/ChangeLog:
> 
>          * config/riscv/riscv.cc
>          (riscv_avoid_shrink_wrapping_separate): wrap the condition check in
>          riscv_avoid_shrink_wrapping_separate.
>          (riscv_avoid_multi_push): avoid multi push if shrink_wrapping_separate
>            is active.
>          (riscv_get_separate_components): call riscv_avoid_shrink_wrapping_separate
>          * shrink-wrap.cc (try_shrink_wrapping_separate): call
>            use_shrink_wrapping_separate.
>          (use_shrink_wrapping_separate):wrap the condition
>            check in use_shrink_wrapping_separate
>          * shrink-wrap.h (use_shrink_wrapping_separate): add to extern
> 
> gcc/testsuite/ChangeLog:
> 
>          * gcc.target/riscv/zcmp_shrink_wrap_separate.c: New test.
>          * gcc.target/riscv/zcmp_shrink_wrap_separate2.c: New test.
I know Kito asked for this to be broken up into target dependent vs 
target independent changes, that's a good ask.

Can't we utilize the get_separate_components hook to accomplish what 
you're trying to do?  ie, put the logic to avoid shrink wrapping for 
this case within the existing risc-v hook?

jeff