You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/04/14 15:05:31 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request, #6073: arch/arm: Remove all lazy fpu related code
xiaoxiang781216 opened a new pull request, #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073
## Summary
since it is broken and ineffient, and then removed by:
```
commit dc961baaea17107109d94575e9bb5d9fb725b801
Author: chao.an <an...@xiaomi.com>
Date: Thu Apr 14 18:07:14 2022 +0800
arm/armv7-[a|r]: move fpu save/restore to assembly handler
Save/Restore FPU registers in C environment is dangerous practive,
which cannot guarantee the compiler won't generate the assembly code
with float point registers, especially in interrupt handling
Signed-off-by: chao.an <an...@xiaomi.com>
commit 8d66dbc0680cb81f91674ace814b2b8a94c49cd2
Author: chao.an <an...@xiaomi.com>
Date: Thu Apr 7 13:48:04 2022 +0800
arm/armv[7|8]-m: skip the fpu save/restore if stack frame is integer-only
Signed-off-by: chao.an <an...@xiaomi.com>
```
## Impact
No, remove the dead code
## Testing
Pass CI
--
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
[GitHub] [incubator-nuttx] pkarashchenko merged pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#issuecomment-1099583825
`ineffient` -> `inefficient` in commit message and PR description
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#discussion_r850559922
##########
arch/arm/src/lpc31xx/lpc31_decodeirq.c:
##########
@@ -99,6 +99,7 @@ uint32_t *arm_decodeirq(uint32_t *regs)
if (regs != CURRENT_REGS)
{
+#ifdef CONFIG_ARCH_ADDRENV
Review Comment:
Done.
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#issuecomment-1099801105
> `ineffient` -> `inefficient` in commit message and PR description
Done.
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#discussion_r852549217
##########
arch/arm/src/common/arm_internal.h:
##########
@@ -89,67 +89,10 @@
#define INTSTACK_SIZE (CONFIG_ARCH_INTERRUPTSTACK & ~STACK_ALIGN_MASK)
-/* Macros to handle saving and restoring interrupt state. In the current ARM
- * model, the state is always copied to and from the stack and TCB. In the
- * Cortex-M0/3 model, the state is copied from the stack to the TCB, but only
- * a referenced is passed to get the state from the TCB. Cortex-M4 is the
- * same, but may have additional complexity for floating point support in
- * some configurations.
- */
-
-#if defined(CONFIG_ARCH_ARMV6M) || defined(CONFIG_ARCH_ARMV7M) || \
- defined(CONFIG_ARCH_ARMV8M)
-
- /* If the floating point unit is present and enabled, then save the
- * floating point registers as well as normal ARM registers. This only
- * applies if "lazy" floating point register save/restore is used
- */
-
-# if defined(CONFIG_ARCH_FPU) && (defined(CONFIG_ARMV7M_LAZYFPU) || \
- defined(CONFIG_ARMV8M_LAZYFPU))
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS, arm_savefpu(regs))
-# else
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS)
-# endif
-# define arm_restorestate(regs) (CURRENT_REGS = regs)
-
-/* The Cortex-A and Cortex-R support the same mechanism, but only lazy
- * floating point register save/restore.
- */
-
-#elif defined(CONFIG_ARCH_ARMV7A) || defined(CONFIG_ARCH_ARMV7R)
-
- /* If the floating point unit is present and enabled, then save the
- * floating point registers as well as normal ARM registers.
- */
+/* Macros to handle saving and restoring interrupt state. */
-# if defined(CONFIG_ARCH_FPU)
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS, arm_savefpu(regs))
-# else
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS)
-# endif
-# define arm_restorestate(regs) (CURRENT_REGS = regs)
-
-/* Otherwise, for the ARM7 and ARM9. The state is copied in full from stack
- * to stack. This is not very efficient and should be fixed to match
- * Cortex-A5.
- */
-
-#else
-
- /* If the floating point unit is present and enabled, then save the
- * floating point registers as well as normal ARM registers. Only "lazy"
- * floating point save/restore is supported.
- */
-
-# if defined(CONFIG_ARCH_FPU)
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS, arm_savefpu(regs))
-# else
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS)
-# endif
-# define arm_restorestate(regs) (CURRENT_REGS = regs)
-
-#endif
+#define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS)
Review Comment:
Done.
--
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
[GitHub] [incubator-nuttx] anchao commented on pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
anchao commented on PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#issuecomment-1104692327
> I noticed that fpu_test with spresense:smp failed.
@masayuki2009 san, I have fix this issue, please review:
https://github.com/apache/incubator-nuttx/pull/6114
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#discussion_r850552891
##########
arch/arm/src/lpc31xx/lpc31_decodeirq.c:
##########
@@ -99,6 +99,7 @@ uint32_t *arm_decodeirq(uint32_t *regs)
if (regs != CURRENT_REGS)
{
+#ifdef CONFIG_ARCH_ADDRENV
Review Comment:
```suggestion
```
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#discussion_r851021614
##########
boards/arm/stm32/stm32f4discovery/configs/wifi/defconfig:
##########
@@ -20,7 +20,6 @@ CONFIG_ARCH_CHIP_STM32=y
CONFIG_ARCH_CHIP_STM32F407VG=y
CONFIG_ARCH_INTERRUPTSTACK=2048
CONFIG_ARCH_STACKDUMP=y
-CONFIG_ARMV7M_LAZYFPU=y
Review Comment:
@anchao will provide a new patch improving fpu test design:
1. Remove CONFIG_TESTING_OSTEST_FPUSIZE config
2. Merge related code from board/chip to arch common
Let's wait his PR.
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#discussion_r852160804
##########
boards/arm/stm32/stm32f4discovery/configs/wifi/defconfig:
##########
@@ -20,7 +20,6 @@ CONFIG_ARCH_CHIP_STM32=y
CONFIG_ARCH_CHIP_STM32F407VG=y
CONFIG_ARCH_INTERRUPTSTACK=2048
CONFIG_ARCH_STACKDUMP=y
-CONFIG_ARMV7M_LAZYFPU=y
Review Comment:
@anchao 's patch is merged, let's continue this PR.
--
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
[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#issuecomment-1104640815
@xiaoxiang781216
I noticed that fpu_test with spresense:smp failed.
```
user_main: FPU test
Starting task FPU#1
FPU#1: pass 1
fpu_test: Started task FPU#1 at PID=27
Starting task FPU#2
FPU#2: pass 1
fpu_test: Started task FPU#2 at PID=28
ERROR FPU#1: save1 and save2 do not match
Values before waiting (save1) (0x2d027204):
0000: 2d02c8d0 000000e0 2d027204 2d0272d8 42d11c6e 400601fb 85a238dc 3ffc03f6
0008: 00000001 00000001 ffffffe9 40300fda 00000000 f01b866e 400921f9 d3458cd2
0010: 401309d9 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0018: 00000000 00000000 00000000 00000001 2d027204 80dc3370 101243f2 00000000
0020: 0d00c6f1 0d00823c 21000000 00000000 00000000 00000000 00000000 00000000
0028: 00000000 00000000 00000000 00000000 00000000 00000000 ERROR FPU#2: save1 and save2 do not match
00000000 402df84d
Values before waiting (save1) (0x2d0273cc):
0030: 3fe01fb4 40984ecf 40490fd0 00000010 0d00c6bb
0000: 2d02d410 000000e0 2d0273cc 2d0274a0 2336023d 400f40e9
2336023d 400740e9
Values after waiting (save2) (0x2d0272d8):
0008: 00000002 00000001 ffffffe9 407a0749 00000000 f01b866e 0000: 2d02c8d0 000000e0 2d027204 2d0272d8 42d11c6e 400601fb 401921f9 cb535009
85a238dc 3ffc03f6
0010: 401f9ad6 00000000 00000000 00000000 00000000 00000000 0008: 00000001 00000001 ffffffe9 40300fda 00000000 f01b866e 00000000 00000000
400921f9 d3458cd2
0018: 00000000 00000000 00000000 00000001 2d0273cc 80dc3370 0010: 401309d9 00000000 00000000 00000000 00000000 00000000 103243f2 00000000
00000000 00000000
0020: 0d00c6f1 0d00823c 21000000 00000000 00000000 00000000 0018: 00000000 00000000 00000000 00000001 2d0272d8 00000001 00000000 00000000
00000000 98968000
0028: 00000000 00000000 00000000 00000000 00000000 00000000 0020: 0d00c733 0d00823c 61000000 00000000 00000000 00000000 00000000 402df84d
00000000 00000000
0030: 403a0749 40fcd6b7 40c90fd0 00000010 0d00c6bb
0028: 00000000 00000000 00000000 00000000 00000000 00000000
00000000 402df84d
Values after waiting (save2) (0x2d0274a0):
0030: 3fe01fb4 40984ecf 40490fd0 00000010 0d00c72d
0000: 2d02d410 000000e0 2d0273cc 2d0274a0 2336023d 400f40e9
2336023d 400740e9
0008: 00000002 00000001 ffffffe9 407a0749 00000000 f01b866e 401921f9 cb535009
0010: 401f9ad6 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0018: 00000000 00000000 00000000 00000001 2d0274a0 00000001 00000000 98968000
0020: 0d00c733 0d00823c 61000000 00000000 00000000 00000000 00000000 00000000
0028: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 402df84d
0030: 403a0749 40fcd6b7 40c90fd0 00000010 0d00c72d
```
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#discussion_r852435289
##########
arch/arm/src/common/arm_internal.h:
##########
@@ -89,67 +89,10 @@
#define INTSTACK_SIZE (CONFIG_ARCH_INTERRUPTSTACK & ~STACK_ALIGN_MASK)
-/* Macros to handle saving and restoring interrupt state. In the current ARM
- * model, the state is always copied to and from the stack and TCB. In the
- * Cortex-M0/3 model, the state is copied from the stack to the TCB, but only
- * a referenced is passed to get the state from the TCB. Cortex-M4 is the
- * same, but may have additional complexity for floating point support in
- * some configurations.
- */
-
-#if defined(CONFIG_ARCH_ARMV6M) || defined(CONFIG_ARCH_ARMV7M) || \
- defined(CONFIG_ARCH_ARMV8M)
-
- /* If the floating point unit is present and enabled, then save the
- * floating point registers as well as normal ARM registers. This only
- * applies if "lazy" floating point register save/restore is used
- */
-
-# if defined(CONFIG_ARCH_FPU) && (defined(CONFIG_ARMV7M_LAZYFPU) || \
- defined(CONFIG_ARMV8M_LAZYFPU))
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS, arm_savefpu(regs))
-# else
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS)
-# endif
-# define arm_restorestate(regs) (CURRENT_REGS = regs)
-
-/* The Cortex-A and Cortex-R support the same mechanism, but only lazy
- * floating point register save/restore.
- */
-
-#elif defined(CONFIG_ARCH_ARMV7A) || defined(CONFIG_ARCH_ARMV7R)
-
- /* If the floating point unit is present and enabled, then save the
- * floating point registers as well as normal ARM registers.
- */
+/* Macros to handle saving and restoring interrupt state. */
-# if defined(CONFIG_ARCH_FPU)
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS, arm_savefpu(regs))
-# else
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS)
-# endif
-# define arm_restorestate(regs) (CURRENT_REGS = regs)
-
-/* Otherwise, for the ARM7 and ARM9. The state is copied in full from stack
- * to stack. This is not very efficient and should be fixed to match
- * Cortex-A5.
- */
-
-#else
-
- /* If the floating point unit is present and enabled, then save the
- * floating point registers as well as normal ARM registers. Only "lazy"
- * floating point save/restore is supported.
- */
-
-# if defined(CONFIG_ARCH_FPU)
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS, arm_savefpu(regs))
-# else
-# define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS)
-# endif
-# define arm_restorestate(regs) (CURRENT_REGS = regs)
-
-#endif
+#define arm_savestate(regs) (regs = (FAR uint32_t *)CURRENT_REGS)
Review Comment:
```suggestion
#define arm_savestate(regs) (regs = (uint32_t *)CURRENT_REGS)
```
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#issuecomment-1099321745
@pkarashchenko the new update just change:
arch/arm/src/armv7-m/exc_return.h
arch/arm/src/armv8-m/exc_return.h
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6073: arch/arm: Remove all lazy fpu related code
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6073:
URL: https://github.com/apache/incubator-nuttx/pull/6073#discussion_r850777193
##########
boards/arm/stm32/stm32f4discovery/configs/wifi/defconfig:
##########
@@ -20,7 +20,6 @@ CONFIG_ARCH_CHIP_STM32=y
CONFIG_ARCH_CHIP_STM32F407VG=y
CONFIG_ARCH_INTERRUPTSTACK=2048
CONFIG_ARCH_STACKDUMP=y
-CONFIG_ARMV7M_LAZYFPU=y
Review Comment:
Probably need to remove `CONFIG_TESTING_OSTEST_FPUSIZE=y` here and in some other configs where `CONFIG_ARMV7M_LAZYFPU=y` is removed.
--
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