Re: [PATCH v2] RISC-V: Save and restore FCSR in interrupt functions to avoid program errors.

2023-06-19 Thread Jeff Law via Gcc-patches




On 6/14/23 01:57, Jin Ma wrote:

In order to avoid interrupt functions to change the FCSR, it needs to be saved
and restored at the beginning and end of the function.

gcc/ChangeLog:

* config/riscv/riscv.cc (riscv_compute_frame_info): Allocate frame for 
FCSR.
(riscv_for_each_saved_reg): Save and restore FCSR in interrupt 
functions.
* config/riscv/riscv.md (riscv_frcsr): New patterns.
(riscv_fscsr): Likewise.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/interrupt-fcsr-1.c: New test.
* gcc.target/riscv/interrupt-fcsr-2.c: New test.
* gcc.target/riscv/interrupt-fcsr-3.c: New test.

Thanks.  I pushed this to the trunk.
jeff


[PATCH v2] RISC-V: Save and restore FCSR in interrupt functions to avoid program errors.

2023-06-14 Thread Jin Ma via Gcc-patches
In order to avoid interrupt functions to change the FCSR, it needs to be saved
and restored at the beginning and end of the function.

gcc/ChangeLog:

* config/riscv/riscv.cc (riscv_compute_frame_info): Allocate frame for 
FCSR.
(riscv_for_each_saved_reg): Save and restore FCSR in interrupt 
functions.
* config/riscv/riscv.md (riscv_frcsr): New patterns.
(riscv_fscsr): Likewise.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/interrupt-fcsr-1.c: New test.
* gcc.target/riscv/interrupt-fcsr-2.c: New test.
* gcc.target/riscv/interrupt-fcsr-3.c: New test.
---
 gcc/config/riscv/riscv.cc | 48 +--
 gcc/config/riscv/riscv.md | 13 +
 .../gcc.target/riscv/interrupt-fcsr-1.c   | 15 ++
 .../gcc.target/riscv/interrupt-fcsr-2.c   | 15 ++
 .../gcc.target/riscv/interrupt-fcsr-3.c   | 14 ++
 5 files changed, 102 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/interrupt-fcsr-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/interrupt-fcsr-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/interrupt-fcsr-3.c

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index dd5361c2bd2..9d71e5c9f72 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -5095,12 +5095,15 @@ riscv_compute_frame_info (void)
 
   frame = >machine->frame;
 
-  /* In an interrupt function, if we have a large frame, then we need to
- save/restore t0.  We check for this before clearing the frame struct.  */
+  /* In an interrupt function, there are two cases in which t0 needs to be 
used:
+ 1, If we have a large frame, then we need to save/restore t0.  We check 
for
+ this before clearing the frame struct.
+ 2, Need to save and restore some CSRs in the frame.  */
   if (cfun->machine->interrupt_handler_p)
 {
   HOST_WIDE_INT step1 = riscv_first_stack_step (frame, frame->total_size);
-  if (! POLY_SMALL_OPERAND_P ((frame->total_size - step1)))
+  if (! POLY_SMALL_OPERAND_P ((frame->total_size - step1))
+ || (TARGET_HARD_FLOAT || TARGET_ZFINX))
interrupt_save_prologue_temp = true;
 }
 
@@ -5147,6 +5150,17 @@ riscv_compute_frame_info (void)
}
 }
 
+  /* In an interrupt function, we need extra space for the initial saves of 
CSRs.  */
+  if (cfun->machine->interrupt_handler_p
+  && ((TARGET_HARD_FLOAT && frame->fmask)
+ || (TARGET_ZFINX
+ /* Except for RISCV_PROLOGUE_TEMP_REGNUM.  */
+ && (frame->mask & ~(1 << RISCV_PROLOGUE_TEMP_REGNUM)
+/* Save and restore FCSR.  */
+/* TODO: When P or V extensions support interrupts, some of their CSRs
+   may also need to be saved and restored.  */
+x_save_size += riscv_stack_align (1 * UNITS_PER_WORD);
+
   /* At the bottom of the frame are any outgoing stack arguments. */
   offset = riscv_stack_align (crtl->outgoing_args_size);
   /* Next are local stack variables. */
@@ -5392,6 +5406,34 @@ riscv_for_each_saved_reg (poly_int64 sp_offset, 
riscv_save_restore_fn fn,
}
}
 
+  /* In an interrupt function, save and restore some necessary CSRs in the 
stack
+to avoid changes in CSRs.  */
+  if (regno == RISCV_PROLOGUE_TEMP_REGNUM
+ && cfun->machine->interrupt_handler_p
+ && ((TARGET_HARD_FLOAT  && cfun->machine->frame.fmask)
+ || (TARGET_ZFINX
+ && (cfun->machine->frame.mask & ~(1 << 
RISCV_PROLOGUE_TEMP_REGNUM)
+   {
+ unsigned int fcsr_size = GET_MODE_SIZE (SImode);
+ if (!epilogue)
+   {
+ riscv_save_restore_reg (word_mode, regno, offset, fn);
+ offset -= fcsr_size;
+ emit_insn (gen_riscv_frcsr (RISCV_PROLOGUE_TEMP (SImode)));
+ riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
+ offset, riscv_save_reg);
+   }
+ else
+   {
+ riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
+ offset - fcsr_size, riscv_restore_reg);
+ emit_insn (gen_riscv_fscsr (RISCV_PROLOGUE_TEMP (SImode)));
+ riscv_save_restore_reg (word_mode, regno, offset, fn);
+ offset -= fcsr_size;
+   }
+ continue;
+   }
+
   riscv_save_restore_reg (word_mode, regno, offset, fn);
 }
 
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index d8e935cb934..565e8cd27cd 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -78,6 +78,8 @@ (define_c_enum "unspecv" [
   UNSPECV_GPR_RESTORE
 
   ;; Floating-point unspecs.
+  UNSPECV_FRCSR
+  UNSPECV_FSCSR
   UNSPECV_FRFLAGS
   UNSPECV_FSFLAGS
   UNSPECV_FSNVSNAN
@@ -3056,6 +3058,17 @@ (define_insn "gpr_restore_return"
   ""
   "")
 
+(define_insn "riscv_frcsr"
+  [(set (match_operand:SI 0 "register_operand" "=r")
+