Re: [PATCH] RISC-V: Fix misaligned stack offset for interrupt function

[Kito Cheng]

Committed to trunk, thanks!

Re: [PATCH] RISC-V: Fix misaligned stack offset for interrupt function

[Kito Cheng]

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 :)

Re: [PATCH] RISC-V: Fix misaligned stack offset for interrupt function

[Fei Gao]

On 2023-12-25 16:45  Kito Cheng  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

Re: [PATCH] RISC-V: Fix misaligned stack offset for interrupt function

[Jeff Law]

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

[PATCH] RISC-V: Fix misaligned stack offset for interrupt function

[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 000..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