diff mbox series

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

Message ID 20230614075746.3918-1-jinma@linux.alibaba.com
State Committed
Commit dbcbc858c71f69da76d1f36d6bb5d72f2db11eda
Headers
Series [v2] RISC-V: Save and restore FCSR in interrupt functions to avoid program errors. |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

Jin Ma June 14, 2023, 7:57 a.m. UTC 
  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

Comments

Jeff Law June 19, 2023, 7:03 p.m. UTC | #1 
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
diff mbox series

Patch

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 = &cfun->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")
+	(unspec_volatile [(const_int 0)] UNSPECV_FRCSR))]
+  "TARGET_HARD_FLOAT || TARGET_ZFINX"
+  "frcsr\t%0")
+
+(define_insn "riscv_fscsr"
+  [(unspec_volatile [(match_operand:SI 0 "csr_operand" "rK")] UNSPECV_FSCSR)]
+  "TARGET_HARD_FLOAT || TARGET_ZFINX"
+  "fscsr\t%0")
+
 (define_insn "riscv_frflags"
   [(set (match_operand:SI 0 "register_operand" "=r")
 	(unspec_volatile [(const_int 0)] UNSPECV_FRFLAGS))]
diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-1.c b/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-1.c
new file mode 100644
index 00000000000..aaafb08a674
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-1.c
@@ -0,0 +1,15 @@ 
+/* Verify that fcsr instructions emitted.  */
+/* { dg-do compile } */
+/* { dg-require-effective-target hard_float } */
+/* { dg-options "-O" } */
+
+extern int foo (void);
+
+void __attribute__ ((interrupt))
+sub (void)
+{
+  foo ();
+}
+
+/* { dg-final { scan-assembler-times "frcsr\t" 1 } } */
+/* { dg-final { scan-assembler-times "fscsr\t" 1 } } */
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-2.c b/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-2.c
new file mode 100644
index 00000000000..ea22e6a2be4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-2.c
@@ -0,0 +1,15 @@ 
+/* Verify that fcsr instructions emitted.  */
+/* { dg-do compile } */
+/* { dg-require-effective-target hard_float } */
+/* { dg-options "-O" } */
+
+extern int foo (void);
+extern float interrupt_count;
+void __attribute__ ((interrupt))
+sub (void)
+{
+  interrupt_count++;
+}
+
+/* { dg-final { scan-assembler-times "frcsr\t" 1 } } */
+/* { dg-final { scan-assembler-times "fscsr\t" 1 } } */
diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-3.c b/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-3.c
new file mode 100644
index 00000000000..5e7eac4b770
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/interrupt-fcsr-3.c
@@ -0,0 +1,14 @@ 
+/* Verify that fcsr instructions are not emitted.  */
+/* { dg-do compile } */
+/* { dg-require-effective-target hard_float } */
+/* { dg-options "-O" } */
+
+extern int foo (void);
+
+void __attribute__ ((interrupt))
+sub (void)
+{
+}
+
+/* { dg-final { scan-assembler-not "frcsr\t" } } */
+/* { dg-final { scan-assembler-not "fscsr\t" } } */