-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[libcpu][cortex-m4]Added HardFault_Handler to save floating point registers #10618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the HardFault_Handler for ARM Cortex-M4 to properly save floating point registers (s16-s31) when a hard fault occurs. This improvement provides better coredump support by preserving the complete floating point context.
- Adds conditional saving of FPU registers d8-d15 (corresponding to s16-s31) when FPU is active
- Replaces dummy flag with proper FPU context flag based on EXC_RETURN[4] bit
- Maintains consistency across three assembly implementations (RVDS, IAR, GCC)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
libcpu/arm/cortex-m4/context_rvds.S | Adds FPU register saving and proper flag handling for RVDS assembler |
libcpu/arm/cortex-m4/context_iar.S | Adds FPU register saving and proper flag handling for IAR assembler |
libcpu/arm/cortex-m4/context_gcc.S | Adds FPU register saving and proper flag handling for GCC assembler |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@@ -236,10 +236,21 @@ HardFault_Handler PROC | |||
MRSEQ r0, msp ; [2]=0 ==> Z=1, get fault context from handler. | |||
MRSNE r0, psp ; [2]=1 ==> Z=0, get fault context from thread. | |||
|
|||
IF {FPU} != "SoftVFP" | |||
TST lr, #0x10 ; if(!EXC_RETURN[4]) | |||
VSTMFDEQ r0!, {d8 - d15} ; push FPU register s16~s31 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment incorrectly states 's16~s31' but the instruction saves double precision registers d8-d15. The comment should be corrected to 'd8-d15' or explain that these correspond to s16-s31 in single precision mode.
VSTMFDEQ r0!, {d8 - d15} ; push FPU register s16~s31 | |
VSTMFDEQ r0!, {d8 - d15} ; push FPU register d8~d15 (corresponds to s16~s31 in single precision mode) |
Copilot uses AI. Check for mistakes.
#if defined ( __ARMVFP__ ) | ||
TST lr, #0x10 ; if(!EXC_RETURN[4]) | ||
BNE skip_push_fpu | ||
VSTMDB r0!, {d8 - d15} ; push FPU register s16~s31 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment incorrectly states 's16~s31' but the instruction saves double precision registers d8-d15. The comment should be corrected to 'd8-d15' or explain that these correspond to s16-s31 in single precision mode.
VSTMDB r0!, {d8 - d15} ; push FPU register s16~s31 | |
VSTMDB r0!, {d8 - d15} ; push FPU registers d8-d15 (corresponds to s16~s31 in single precision mode) |
Copilot uses AI. Check for mistakes.
#if defined (__VFP_FP__) && !defined(__SOFTFP__) | ||
TST lr, #0x10 /* if(!EXC_RETURN[4]) */ | ||
IT EQ | ||
VSTMDBEQ r0!, {d8 - d15} /* push FPU register s16~s31 */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment incorrectly states 's16~s31' but the instruction saves double precision registers d8-d15. The comment should be corrected to 'd8-d15' or explain that these correspond to s16-s31 in single precision mode.
VSTMDBEQ r0!, {d8 - d15} /* push FPU register s16~s31 */ | |
VSTMDBEQ r0!, {d8 - d15} /* push FPU registers d8-d15 (corresponds to s16~s31 in single precision mode) */ |
Copilot uses AI. Check for mistakes.
@@ -236,10 +236,21 @@ HardFault_Handler PROC | |||
MRSEQ r0, msp ; [2]=0 ==> Z=1, get fault context from handler. | |||
MRSNE r0, psp ; [2]=1 ==> Z=0, get fault context from thread. | |||
|
|||
IF {FPU} != "SoftVFP" | |||
TST lr, #0x10 ; if(!EXC_RETURN[4]) | |||
VSTMFDEQ r0!, {d8 - d15} ; push FPU register s16~s31 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VSTMFDEQ instruction uses full descending stack mode, but this is inconsistent with the STMFD instructions used elsewhere. Consider using VSTMDBEQ for consistency with decrement before operation mode.
VSTMFDEQ r0!, {d8 - d15} ; push FPU register s16~s31 | |
VSTMDBEQ r0!, {d8 - d15} ; push FPU register s16~s31 |
Copilot uses AI. Check for mistakes.
添加HardFault_Handler中针对浮点寄存器的保存,可以为后续coredump提供浮点寄存器
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
你的解决方案是什么 (what is your solution)
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality:
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0
代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up