You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "xiaoxiang781216 (via GitHub)" <gi...@apache.org> on 2023/06/17 07:52:54 UTC
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9486: riscv/fpu: Implement correct lazy-FPU functionality
xiaoxiang781216 commented on code in PR #9486:
URL: https://github.com/apache/nuttx/pull/9486#discussion_r1232983645
##########
arch/risc-v/include/irq.h:
##########
@@ -204,50 +204,56 @@
#endif
#ifdef CONFIG_ARCH_FPU
-# define REG_F0_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 0)
-# define REG_F1_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 1)
-# define REG_F2_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 2)
-# define REG_F3_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 3)
-# define REG_F4_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 4)
-# define REG_F5_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 5)
-# define REG_F6_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 6)
-# define REG_F7_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 7)
-# define REG_F8_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 8)
-# define REG_F9_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 9)
-# define REG_F10_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 10)
-# define REG_F11_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 11)
-# define REG_F12_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 12)
-# define REG_F13_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 13)
-# define REG_F14_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 14)
-# define REG_F15_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 15)
-# define REG_F16_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 16)
-# define REG_F17_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 17)
-# define REG_F18_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 18)
-# define REG_F19_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 19)
-# define REG_F20_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 20)
-# define REG_F21_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 21)
-# define REG_F22_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 22)
-# define REG_F23_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 23)
-# define REG_F24_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 24)
-# define REG_F25_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 25)
-# define REG_F26_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 26)
-# define REG_F27_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 27)
-# define REG_F28_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 28)
-# define REG_F29_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 29)
-# define REG_F30_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 30)
-# define REG_F31_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 31)
-# define REG_FCSR_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 32)
-
-# define FPU_XCPT_REGS (FPU_REG_SIZE * 33)
+# define REG_F0_NDX (FPU_REG_SIZE * 0)
+# define REG_F1_NDX (FPU_REG_SIZE * 1)
+# define REG_F2_NDX (FPU_REG_SIZE * 2)
+# define REG_F3_NDX (FPU_REG_SIZE * 3)
+# define REG_F4_NDX (FPU_REG_SIZE * 4)
+# define REG_F5_NDX (FPU_REG_SIZE * 5)
+# define REG_F6_NDX (FPU_REG_SIZE * 6)
+# define REG_F7_NDX (FPU_REG_SIZE * 7)
+# define REG_F8_NDX (FPU_REG_SIZE * 8)
+# define REG_F9_NDX (FPU_REG_SIZE * 9)
+# define REG_F10_NDX (FPU_REG_SIZE * 10)
+# define REG_F11_NDX (FPU_REG_SIZE * 11)
+# define REG_F12_NDX (FPU_REG_SIZE * 12)
+# define REG_F13_NDX (FPU_REG_SIZE * 13)
+# define REG_F14_NDX (FPU_REG_SIZE * 14)
+# define REG_F15_NDX (FPU_REG_SIZE * 15)
+# define REG_F16_NDX (FPU_REG_SIZE * 16)
+# define REG_F17_NDX (FPU_REG_SIZE * 17)
+# define REG_F18_NDX (FPU_REG_SIZE * 18)
+# define REG_F19_NDX (FPU_REG_SIZE * 19)
+# define REG_F20_NDX (FPU_REG_SIZE * 20)
+# define REG_F21_NDX (FPU_REG_SIZE * 21)
+# define REG_F22_NDX (FPU_REG_SIZE * 22)
+# define REG_F23_NDX (FPU_REG_SIZE * 23)
+# define REG_F24_NDX (FPU_REG_SIZE * 24)
+# define REG_F25_NDX (FPU_REG_SIZE * 25)
+# define REG_F26_NDX (FPU_REG_SIZE * 26)
+# define REG_F27_NDX (FPU_REG_SIZE * 27)
+# define REG_F28_NDX (FPU_REG_SIZE * 28)
+# define REG_F29_NDX (FPU_REG_SIZE * 29)
+# define REG_F30_NDX (FPU_REG_SIZE * 30)
+# define REG_F31_NDX (FPU_REG_SIZE * 31)
+# define REG_FCSR_NDX (FPU_REG_SIZE * 32)
+
+# define FPU_XCPT_NREGS (33)
+# define FPU_XCPT_REGS (FPU_REG_SIZE * FPU_XCPT_NREGS)
Review Comment:
change to:
```
# define FPU_XCPT_REGS (FPU_REG_SIZE * 33)
```
and remove line 241
##########
arch/risc-v/src/common/riscv_macros.S:
##########
@@ -101,11 +101,6 @@
.macro riscv_savefpu in
#ifdef CONFIG_ARCH_FPU
Review Comment:
could remove "#ifdef" check
##########
arch/risc-v/include/irq.h:
##########
@@ -204,50 +204,56 @@
#endif
#ifdef CONFIG_ARCH_FPU
-# define REG_F0_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 0)
-# define REG_F1_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 1)
-# define REG_F2_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 2)
-# define REG_F3_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 3)
-# define REG_F4_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 4)
-# define REG_F5_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 5)
-# define REG_F6_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 6)
-# define REG_F7_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 7)
-# define REG_F8_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 8)
-# define REG_F9_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 9)
-# define REG_F10_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 10)
-# define REG_F11_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 11)
-# define REG_F12_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 12)
-# define REG_F13_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 13)
-# define REG_F14_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 14)
-# define REG_F15_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 15)
-# define REG_F16_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 16)
-# define REG_F17_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 17)
-# define REG_F18_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 18)
-# define REG_F19_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 19)
-# define REG_F20_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 20)
-# define REG_F21_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 21)
-# define REG_F22_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 22)
-# define REG_F23_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 23)
-# define REG_F24_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 24)
-# define REG_F25_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 25)
-# define REG_F26_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 26)
-# define REG_F27_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 27)
-# define REG_F28_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 28)
-# define REG_F29_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 29)
-# define REG_F30_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 30)
-# define REG_F31_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 31)
-# define REG_FCSR_NDX (INT_XCPT_REGS + FPU_REG_SIZE * 32)
-
-# define FPU_XCPT_REGS (FPU_REG_SIZE * 33)
+# define REG_F0_NDX (FPU_REG_SIZE * 0)
+# define REG_F1_NDX (FPU_REG_SIZE * 1)
+# define REG_F2_NDX (FPU_REG_SIZE * 2)
+# define REG_F3_NDX (FPU_REG_SIZE * 3)
+# define REG_F4_NDX (FPU_REG_SIZE * 4)
+# define REG_F5_NDX (FPU_REG_SIZE * 5)
+# define REG_F6_NDX (FPU_REG_SIZE * 6)
+# define REG_F7_NDX (FPU_REG_SIZE * 7)
+# define REG_F8_NDX (FPU_REG_SIZE * 8)
+# define REG_F9_NDX (FPU_REG_SIZE * 9)
+# define REG_F10_NDX (FPU_REG_SIZE * 10)
+# define REG_F11_NDX (FPU_REG_SIZE * 11)
+# define REG_F12_NDX (FPU_REG_SIZE * 12)
+# define REG_F13_NDX (FPU_REG_SIZE * 13)
+# define REG_F14_NDX (FPU_REG_SIZE * 14)
+# define REG_F15_NDX (FPU_REG_SIZE * 15)
+# define REG_F16_NDX (FPU_REG_SIZE * 16)
+# define REG_F17_NDX (FPU_REG_SIZE * 17)
+# define REG_F18_NDX (FPU_REG_SIZE * 18)
+# define REG_F19_NDX (FPU_REG_SIZE * 19)
+# define REG_F20_NDX (FPU_REG_SIZE * 20)
+# define REG_F21_NDX (FPU_REG_SIZE * 21)
+# define REG_F22_NDX (FPU_REG_SIZE * 22)
+# define REG_F23_NDX (FPU_REG_SIZE * 23)
+# define REG_F24_NDX (FPU_REG_SIZE * 24)
+# define REG_F25_NDX (FPU_REG_SIZE * 25)
+# define REG_F26_NDX (FPU_REG_SIZE * 26)
+# define REG_F27_NDX (FPU_REG_SIZE * 27)
+# define REG_F28_NDX (FPU_REG_SIZE * 28)
+# define REG_F29_NDX (FPU_REG_SIZE * 29)
+# define REG_F30_NDX (FPU_REG_SIZE * 30)
+# define REG_F31_NDX (FPU_REG_SIZE * 31)
+# define REG_FCSR_NDX (FPU_REG_SIZE * 32)
+
+# define FPU_XCPT_NREGS (33)
+# define FPU_XCPT_REGS (FPU_REG_SIZE * FPU_XCPT_NREGS)
# define FPU_REG_FULL_SIZE (INT_REG_SIZE * FPU_REG_SIZE)
+# define FPU_XCPT_SIZE (FPU_REG_FULL_SIZE * FPU_XCPT_NREGS)
Review Comment:
change to:
```
define FPU_XCPT_SIZE (FPU_XCPT_REGS * FPU_XCPT_REGS)
```
and remove FPU_REG_FULL_SIZE.
##########
arch/risc-v/src/common/riscv_macros.S:
##########
@@ -217,11 +206,6 @@
.macro riscv_loadfpu out
#ifdef CONFIG_ARCH_FPU
Review Comment:
could remove "#ifdef" check too
##########
arch/risc-v/src/common/riscv_fpu.S:
##########
@@ -57,18 +61,95 @@
* Returned Value:
* This function does not return anything explicitly.
*
- ************************************************************************************/
+ ****************************************************************************/
.type riscv_fpuconfig, function
riscv_fpuconfig:
- li a0, MSTATUS_FS_INIT
- csrs CSR_STATUS, a0
+ li a0, MSTATUS_FS_INIT
+ csrs CSR_STATUS, a0
+
+ fsflags zero
+ fsrm zero
+
+ fence.i
+ ret
+
+/****************************************************************************
+ * Name: riscv_savefpu
+ *
+ * Description:
+ * Given the pointer to a register save area (in A0), save the state of the
+ * floating point registers.
+ *
+ * C Function Prototype:
+ * void riscv_savefpu(uintptr_t *regs, uintptr_t *fregs);
+ *
+ * Input Parameters:
+ * regs - A pointer to the integer registers that contain the status
+ * fregs - A pointer to the register save area in which to save the
+ * floating point registers
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+ .type riscv_savefpu, function
+
+riscv_savefpu:
+
+ REGLOAD t0, REG_INT_CTX(a0)
+ li t1, MSTATUS_FS
+ and t2, t0, t1
+ li t1, MSTATUS_FS_DIRTY
+ bne t2, t1, 1f
+ li t1, ~MSTATUS_FS
+ and t0, t0, t1
+ li t1, MSTATUS_FS_CLEAN
+ or t0, t0, t1
+ REGSTORE t0, REG_INT_CTX(a0)
+
+ riscv_savefpu a1
+
+1:
+ ret
+
+/****************************************************************************
+ * Name: riscv_restorefpu
+ *
+ * Description:
+ * Given the pointer to a register save area (in A0), restore the state of
+ * the floating point registers.
+ *
+ * C Function Prototype:
+ * void riscv_restorefpu(uintptr_t *regs, uintptr_t *fregs);
+ *
+ * Input Parameters:
+ * regs - A pointer to the integer registers that contain the status
+ * fregs - A pointer to the register save area containing the floating
+ * point registers.
+ *
+ * Returned Value:
+ * This function does not return anything explicitly. However, it is
+ * called from interrupt level assembly logic that assumes that r0 is
+ * preserved.
+ *
+ ****************************************************************************/
+
+ .type riscv_restorefpu, function
+
+riscv_restorefpu:
+
+ REGLOAD t0, REG_INT_CTX(a0)
+ li t1, MSTATUS_FS
+ and t2, t0, t1
Review Comment:
could change t2 to t0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org