[v3,2/2] RISC-V: avoid LUI based const mat in alloca epilogue expansion
Checks
Context |
Check |
Description |
rivoscibot/toolchain-ci-rivos-lint |
success
|
Lint passed
|
rivoscibot/toolchain-ci-rivos-apply-patch |
success
|
Patch applied
|
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gcv-lp64d-multilib |
success
|
Build passed
|
rivoscibot/toolchain-ci-rivos-build--newlib-rv32imc_zba_zbb_zbc_zbs-ilp32d-non-multilib |
success
|
Build passed
|
rivoscibot/toolchain-ci-rivos-build--newlib-rv64gc-lp64d-multilib |
success
|
Build passed
|
rivoscibot/toolchain-ci-rivos-build--linux-rv64gcv-lp64d-multilib |
success
|
Build passed
|
rivoscibot/toolchain-ci-rivos-build--linux-rv32gc_zba_zbb_zbc_zbs-ilp32d-non-multilib |
success
|
Build passed
|
rivoscibot/toolchain-ci-rivos-build--linux-rv64gc_zba_zbb_zbc_zbs-lp64d-non-multilib |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Testing passed
|
rivoscibot/toolchain-ci-rivos-test |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
This is testsuite clean however there's a dwarf quirk which I want to
run by the experts. The test that was tripping CI has following
fragment:
Before patch | After Patch
------------------------------------------------------
li t0,-4096 | addi sp,s0,-2048
addi t0,t0,560 | .cfi_def_cfa 2, 2048 <- #1
add sp,s0,t0 | addi sp,sp,-1488
.cfi_def_cfa 2, 3536 | .cfi_def_cfa_offset 3536 <- #2
addi sp,sp,1504 | addi sp,sp,1504
.cfi_def_cfa_offset 2032 | .cfi_def_cfa_offset 2032 <- #3
The dwarf insn #1 and #3 seem ok, however #2 seems dubious to me.
---
This is continuing on the prev patch in function epilogue expansion.
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_expand_epilogue): Handle offset
being sum of two S12.
Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
---
gcc/config/riscv/riscv.cc | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
Comments
On 5/20/24 5:32 PM, Vineet Gupta wrote:
> This is testsuite clean however there's a dwarf quirk which I want to
> run by the experts. The test that was tripping CI has following
> fragment:
>
> Before patch | After Patch
> ------------------------------------------------------
> li t0,-4096 | addi sp,s0,-2048
> addi t0,t0,560 | .cfi_def_cfa 2, 2048 <- #1
> add sp,s0,t0 | addi sp,sp,-1488
> .cfi_def_cfa 2, 3536 | .cfi_def_cfa_offset 3536 <- #2
> addi sp,sp,1504 | addi sp,sp,1504
> .cfi_def_cfa_offset 2032 | .cfi_def_cfa_offset 2032 <- #3
>
> The dwarf insn #1 and #3 seem ok, however #2 seems dubious to me.
What about it seems dubious? We need a CFA adjustment on each insn that
modifies the stack pointer so that we can unwind at any arbitrary point.
The first adjustment says the prior frame is at sp + 2048. Then it's at
sp + 3536. Then after the final insn the prior frame is at sp+2032.
Jeff
On 5/20/24 20:54, Jeff Law wrote:
> On 5/20/24 5:32 PM, Vineet Gupta wrote:
>> This is testsuite clean however there's a dwarf quirk which I want to
>> run by the experts. The test that was tripping CI has following
>> fragment:
>>
>> Before patch | After Patch
>> ------------------------------------------------------
>> li t0,-4096 | addi sp,s0,-2048
>> addi t0,t0,560 | .cfi_def_cfa 2, 2048 <- #1
>> add sp,s0,t0 | addi sp,sp,-1488
>> .cfi_def_cfa 2, 3536 | .cfi_def_cfa_offset 3536 <- #2
>> addi sp,sp,1504 | addi sp,sp,1504
>> .cfi_def_cfa_offset 2032 | .cfi_def_cfa_offset 2032 <- #3
>>
>> The dwarf insn #1 and #3 seem ok, however #2 seems dubious to me.
> What about it seems dubious?
My discomfort at claiming I understand dwarf, despite debugging/fixing
the ARC Linux port's in kernel dwarf unwinder :-)
> We need a CFA adjustment on each insn that
> modifies the stack pointer so that we can unwind at any arbitrary point.
Of course.
> The first adjustment says the prior frame is at sp + 2048. Then it's at
> sp + 3536. Then after the final insn the prior frame is at sp+2032.
Yeah I got confused with second one since once it gets anchored to SP
from S0, but you are right it is farther from base CFA now.
-Vineet
On 5/20/24 5:32 PM, Vineet Gupta wrote:
> This is testsuite clean however there's a dwarf quirk which I want to
> run by the experts. The test that was tripping CI has following
> fragment:
>
> Before patch | After Patch
> ------------------------------------------------------
> li t0,-4096 | addi sp,s0,-2048
> addi t0,t0,560 | .cfi_def_cfa 2, 2048 <- #1
> add sp,s0,t0 | addi sp,sp,-1488
> .cfi_def_cfa 2, 3536 | .cfi_def_cfa_offset 3536 <- #2
> addi sp,sp,1504 | addi sp,sp,1504
> .cfi_def_cfa_offset 2032 | .cfi_def_cfa_offset 2032 <- #3
>
> The dwarf insn #1 and #3 seem ok, however #2 seems dubious to me.
>
> ---
>
> This is continuing on the prev patch in function epilogue expansion.
>
> gcc/ChangeLog:
> * config/riscv/riscv.cc (riscv_expand_epilogue): Handle offset
> being sum of two S12.
OK.
jeff
@@ -8111,7 +8111,10 @@ riscv_expand_epilogue (int style)
need_barrier_p = false;
poly_int64 adjust_offset = -frame->hard_frame_pointer_offset;
+ rtx dwarf_adj = gen_int_mode (adjust_offset, Pmode);
rtx adjust = NULL_RTX;
+ bool sum_of_two_s12 = false;
+ HOST_WIDE_INT one, two;
if (!adjust_offset.is_constant ())
{
@@ -8123,14 +8126,23 @@ riscv_expand_epilogue (int style)
}
else
{
- if (!SMALL_OPERAND (adjust_offset.to_constant ()))
+ HOST_WIDE_INT adj_off_value = adjust_offset.to_constant ();
+ if (SMALL_OPERAND (adj_off_value))
+ {
+ adjust = GEN_INT (adj_off_value);
+ }
+ else if (SUM_OF_TWO_S12_ALGN (adj_off_value))
+ {
+ riscv_split_sum_of_two_s12 (adj_off_value, &one, &two);
+ dwarf_adj = adjust = GEN_INT (one);
+ sum_of_two_s12 = true;
+ }
+ else
{
riscv_emit_move (RISCV_PROLOGUE_TEMP (Pmode),
- GEN_INT (adjust_offset.to_constant ()));
+ GEN_INT (adj_off_value));
adjust = RISCV_PROLOGUE_TEMP (Pmode);
}
- else
- adjust = GEN_INT (adjust_offset.to_constant ());
}
insn = emit_insn (
@@ -8138,14 +8150,21 @@ riscv_expand_epilogue (int style)
adjust));
rtx dwarf = NULL_RTX;
- rtx cfa_adjust_value = gen_rtx_PLUS (
- Pmode, hard_frame_pointer_rtx,
- gen_int_mode (-frame->hard_frame_pointer_offset, Pmode));
+ rtx cfa_adjust_value = gen_rtx_PLUS (Pmode, hard_frame_pointer_rtx,
+ dwarf_adj);
rtx cfa_adjust_rtx = gen_rtx_SET (stack_pointer_rtx, cfa_adjust_value);
dwarf = alloc_reg_note (REG_CFA_ADJUST_CFA, cfa_adjust_rtx, dwarf);
+
RTX_FRAME_RELATED_P (insn) = 1;
REG_NOTES (insn) = dwarf;
+
+ if (sum_of_two_s12)
+ {
+ insn = emit_insn (gen_add3_insn (stack_pointer_rtx, stack_pointer_rtx,
+ GEN_INT (two)));
+ RTX_FRAME_RELATED_P (insn) = 1;
+ }
}
if (use_restore_libcall || use_multi_pop)