* [PATCH] RISC-V: Fix misaligned stack offset for interrupt function
@ 2023-12-25 8:45 Kito Cheng
2023-12-28 15:58 ` Jeff Law
2023-12-29 7:56 ` Fei Gao
0 siblings, 2 replies; 5+ messages in thread
From: Kito Cheng @ 2023-12-25 8:45 UTC (permalink / raw)
To: gcc-patches, kito.cheng, palmer, jeffreyalaw; +Cc: Kito Cheng
`interrupt` function will backup fcsr register, but it fixed to SImode,
it's not big issue since fcsr only used 8 bits so far, however the
offset should still using UNITS_PER_WORD to prevent the stack offset
become non 8 byte aligned, it will cause problem for RV64.
gcc/ChangeLog:
* config/riscv/riscv.cc (riscv_for_each_saved_reg): Adjust the
offset of fcsr.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/interrupt-misaligned.c: New.
---
gcc/config/riscv/riscv.cc | 4 ++-
.../gcc.target/riscv/interrupt-misaligned.c | 29 +++++++++++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/interrupt-misaligned.c
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index d867c0a03f0..c2b24d3db5a 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -6790,7 +6790,9 @@ riscv_for_each_saved_reg (poly_int64 sp_offset, riscv_save_restore_fn fn,
|| (TARGET_ZFINX
&& (cfun->machine->frame.mask & ~(1 << RISCV_PROLOGUE_TEMP_REGNUM)))))
{
- unsigned int fcsr_size = GET_MODE_SIZE (SImode);
+ /* Always assume FCSR occupy UNITS_PER_WORD to prevent stack
+ offset misaligned later. */
+ unsigned int fcsr_size = UNITS_PER_WORD;
if (!epilogue)
{
riscv_save_restore_reg (word_mode, regno, offset, fn);
diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-misaligned.c b/gcc/testsuite/gcc.target/riscv/interrupt-misaligned.c
new file mode 100644
index 00000000000..b5f8e6c2bbe
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/interrupt-misaligned.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv64gc -mabi=lp64d -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } */
+
+/* Make sure no stack offset are misaligned.
+** interrupt:
+** ...
+** sd\tt0,40\(sp\)
+** frcsr\tt0
+** sw\tt0,32\(sp\)
+** sd\tt1,24\(sp\)
+** fsd\tft0,8\(sp\)
+** ...
+** lw\tt0,32\(sp\)
+** fscsr\tt0
+** ld\tt0,40\(sp\)
+** ld\tt1,24\(sp\)
+** fld\tft0,8\(sp\)
+** ...
+*/
+
+
+void interrupt(void) __attribute__((interrupt));
+void interrupt(void)
+{
+ asm volatile ("# clobber!":::"t0", "t1", "ft0");
+}
+
+/* { dg-final { check-function-bodies "**" "" } } */
--
2.40.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: Fix misaligned stack offset for interrupt function
2023-12-25 8:45 [PATCH] RISC-V: Fix misaligned stack offset for interrupt function Kito Cheng
@ 2023-12-28 15:58 ` Jeff Law
2024-01-04 8:02 ` Kito Cheng
2023-12-29 7:56 ` Fei Gao
1 sibling, 1 reply; 5+ messages in thread
From: Jeff Law @ 2023-12-28 15:58 UTC (permalink / raw)
To: Kito Cheng, gcc-patches, kito.cheng, palmer
On 12/25/23 01:45, Kito Cheng wrote:
> `interrupt` function will backup fcsr register, but it fixed to SImode,
> it's not big issue since fcsr only used 8 bits so far, however the
> offset should still using UNITS_PER_WORD to prevent the stack offset
> become non 8 byte aligned, it will cause problem for RV64.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv.cc (riscv_for_each_saved_reg): Adjust the
> offset of fcsr.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/interrupt-misaligned.c: New.
OK
jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: Fix misaligned stack offset for interrupt function
2023-12-25 8:45 [PATCH] RISC-V: Fix misaligned stack offset for interrupt function Kito Cheng
2023-12-28 15:58 ` Jeff Law
@ 2023-12-29 7:56 ` Fei Gao
2024-01-04 8:02 ` Kito Cheng
1 sibling, 1 reply; 5+ messages in thread
From: Fei Gao @ 2023-12-29 7:56 UTC (permalink / raw)
To: Kito Cheng, gcc-patches, Kito Cheng, Palmer Dabbelt, jeffreyalaw
Cc: Kito Cheng, jinma
On 2023-12-25 16:45 Kito Cheng <kito.cheng@sifive.com> wrote:
>+++ b/gcc/testsuite/gcc.target/riscv/interrupt-misaligned.c
>@@ -0,0 +1,29 @@
>+/* { dg-do compile } */
>+/* { dg-options "-O2 -march=rv64gc -mabi=lp64d -fno-schedule-insns -fno-schedule-insns2" } */
>+/* { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } */
>+
>+/* Make sure no stack offset are misaligned.
>+** interrupt:
>+** ...
>+** sd\tt0,40\(sp\)
>+** frcsr\tt0
>+** sw\tt0,32\(sp\)
>+** sd\tt1,24\(sp\)
>+** fsd\tft0,8\(sp\)
>+** ...
>+** lw\tt0,32\(sp\)
>+** fscsr\tt0
>+** ld\tt0,40\(sp\)
>+** ld\tt1,24\(sp\)
>+** fld\tft0,8\(sp\)
>+** ...
>+*/
Hi Kito
The fix is fine but maybe using s0 instead of t0 is better:
1. simpler codes.
2. less stack size
current implementaion:
>+** sd\tt0,40\(sp\)
>+** frcsr\tt0
>+** sw\tt0,32\(sp\) //save content of frcsr in stack
use s0:
>+** sd\tt0,40\(sp\)
>+** frcsr\ts0 //save content of frcsr in s0 instead of stack. If s0 is used as callee saved register, it will be saved again later by legacy codes .
Also adding this change in riscv_expand_prologue & epilogue would be consistent with current stack allocation logic.
I can try it if you think necessary.
BR
Fei
>+
>+
>+void interrupt(void) __attribute__((interrupt));
>+void interrupt(void)
>+{
>+ asm volatile ("# clobber!":::"t0", "t1", "ft0");
>+}
>+
>+/* { dg-final { check-function-bodies "**" "" } } */
>--
>2.40.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: Fix misaligned stack offset for interrupt function
2023-12-29 7:56 ` Fei Gao
@ 2024-01-04 8:02 ` Kito Cheng
0 siblings, 0 replies; 5+ messages in thread
From: Kito Cheng @ 2024-01-04 8:02 UTC (permalink / raw)
To: Fei Gao; +Cc: Kito Cheng, gcc-patches, Palmer Dabbelt, jeffreyalaw, jinma
Hi Fei:
> The fix is fine but maybe using s0 instead of t0 is better:
> 1. simpler codes.
> 2. less stack size
>
> current implementaion:
> >+** sd\tt0,40\(sp\)
> >+** frcsr\tt0
> >+** sw\tt0,32\(sp\) //save content of frcsr in stack
>
> use s0:
> >+** sd\tt0,40\(sp\)
> >+** frcsr\ts0 //save content of frcsr in s0 instead of stack. If s0 is used as callee saved register, it will be saved again later by legacy codes .
>
> Also adding this change in riscv_expand_prologue & epilogue would be consistent with current stack allocation logic.
>
> I can try it if you think necessary.
Yeah, that can optimize further, but I guess the ideal strategy is
check which register will save/restore at prologue/epilogue - and use
that no matter if it's s* or t* register, anyway, I think it's
interesting stuff to improve, so it would be appreciated if you can
try :)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RISC-V: Fix misaligned stack offset for interrupt function
2023-12-28 15:58 ` Jeff Law
@ 2024-01-04 8:02 ` Kito Cheng
0 siblings, 0 replies; 5+ messages in thread
From: Kito Cheng @ 2024-01-04 8:02 UTC (permalink / raw)
To: Jeff Law; +Cc: Kito Cheng, gcc-patches, palmer
Committed to trunk, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-04 8:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-25 8:45 [PATCH] RISC-V: Fix misaligned stack offset for interrupt function Kito Cheng
2023-12-28 15:58 ` Jeff Law
2024-01-04 8:02 ` Kito Cheng
2023-12-29 7:56 ` Fei Gao
2024-01-04 8:02 ` Kito Cheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).