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
