-
Notifications
You must be signed in to change notification settings - Fork 3k
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
CMAKE : correct "Cortex-M7F" link parameter #14159
Conversation
@jeromecoutant, thank you for your changes. |
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.
What error does this fix ? Did we port the cpu flag incorrectly or there is an issue somewhere else?
Here is error during link before patch:
|
tools/cmake/cores/Cortex-M7F.cmake
Outdated
@@ -16,10 +16,10 @@ elseif(${MBED_TOOLCHAIN} STREQUAL "ARM") | |||
"-mfloat-abi=hard" | |||
) | |||
list(APPEND asm_compile_options | |||
"-mcpu=Cortex-M7.fp.sp" | |||
"-mcpu=Cortex-M7" |
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.
I think the problem here is that you're using armclang
to invoke the assembler (why?), but passing it armasm-type CPU options.
I believe your asm_compile_options
should be the same as the c_cxx_compile_options
, including the -mfpu
and -mfloat-abi
, and armclang
should translate.
(Not actually sure how this change effects that translation though - don't know how armclang interprets "Cortex-M7.fp.sp". Do you get a warning about it?)
Not sure about the link, but potentially the same thing applies if you're invoking it via armclang
. In which case you're in the same situation as GCC, and they should all be common_options
. (Edit: ah, I see that's using --cpu
not -mcpu
which suggests you're not?)
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.
I think the problem here is that you're using armclang to invoke the assembler (why?), but passing it armasm-type CPU options.
I've missed this earlier. We were suggested due to another bug to use armclang and --auto mode for Mbed OS asm files.
The flags should be fixed m thanks for noticing.
tools/cmake/cores/Cortex-M7F.cmake
Outdated
@@ -16,10 +16,10 @@ elseif(${MBED_TOOLCHAIN} STREQUAL "ARM") | |||
"-mfloat-abi=hard" | |||
) | |||
list(APPEND asm_compile_options | |||
"-mcpu=Cortex-M7.fp.sp" | |||
"-mcpu=Cortex-M7" |
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.
I can confirm that asm_compile_option
should share the same options as c_cxx_compile_options
because they are processed by armclang
.
A similar issue was encountered for Cortex-M33.
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.
@jeromecoutant Can you do the same in this PR as we did for m33 ?
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.
Done
16b79b1
to
541c681
Compare
CI started |
To clarify on the linker thing - it was using Presumably this change needs to be made to all chips? |
Jenkins CI Test : ❌ FAILEDBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Ci restarted |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Summary of changes
Needed for STM32F7
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers