-
Notifications
You must be signed in to change notification settings - Fork 3k
CMake: Use armclang for ASM files #13534
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
Fixes the issue of armasm, passing -DMACRO={0,0} - fails to parse. We use armclang frontend but passing -masm=armasm.
@0xc0170, thank you for your changes. |
Oh, the bot was fast, due to the wrong base branch, all reviewers :-) I fixed that. |
cmake/toolchains/ARM.cmake
Outdated
target_compile_options(${target} | ||
PUBLIC | ||
$<$<COMPILE_LANGUAGE:ASM>:--target=arm-arm-none-eabi -masm=armasm> |
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.
-masm=auto
works for me. If confirmed, I'll change this
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.
-masm=auto
also worked on my Mac OS system with cmake version 3.17.3
.
However, would it not be prudent to explicitly set it?
In rare circumstances, the auto-detection might select the wrong assembler for the input file.
For example, if the input file is a .S file that requires preprocessing, auto-detection might select
the wrong assembler. For the files where auto-detection selects the wrong assembler, you must
select -masm=gnu or -masm=armasm explicitly.
Source: https://www.keil.com/support/man/docs/armclang_ref/armclang_ref_yml1538053827173.htm
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.
For now, armasm would be better but what about the future? New MCU startup files will soon have GNU syntax for ARMClang .S files, therefore this armasm won't work. Auto mode is neat and exactly what we need. if we find bugs, we shall report it to the compiler (it is state as "rare").
I would rather take care of these rare cases (overwrite armasm to use gnu manually - the component would request to be compiled with GNU syntax for ARMClang6, for instance target component for specific board). Is this feasible ?
cmake/toolchains/ARM.cmake
Outdated
target_compile_options(${target} | ||
PUBLIC | ||
$<$<COMPILE_LANGUAGE:ASM>:--target=arm-arm-none-eabi -masm=armasm> |
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.
-masm=auto
also worked on my Mac OS system with cmake version 3.17.3
.
However, would it not be prudent to explicitly set it?
In rare circumstances, the auto-detection might select the wrong assembler for the input file.
For example, if the input file is a .S file that requires preprocessing, auto-detection might select
the wrong assembler. For the files where auto-detection selects the wrong assembler, you must
select -masm=gnu or -masm=armasm explicitly.
Source: https://www.keil.com/support/man/docs/armclang_ref/armclang_ref_yml1538053827173.htm
cmake/cores/Cortex-M0+.cmake
Outdated
@@ -27,7 +27,7 @@ function(mbed_set_cpu_core_options target mbed_toolchain) | |||
PUBLIC | |||
$<$<COMPILE_LANGUAGE:C>:${compile_options}> | |||
$<$<COMPILE_LANGUAGE:CXX>:${compile_options}> | |||
$<$<COMPILE_LANGUAGE:ASM>:--cpu=Cortex-M0plus> | |||
$<$<$<$<COMPILE_LANGUAGE:ASM>:-mcpu=Cortex-M0plus> |
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'm not a CMake expert but the extra $<$<
looks imbalanced?
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.
Good catch, fixing now
5b71715
to
090e686
Compare
Pull request has been modified.
Auto mode for ASM files. There might be rare cases where this fails, we will fix them by requesting the component to use gnu syntax for assembly files.
8bf9e3f
to
ab48be2
Compare
I fixed the typos in cores |
Summary of changes
The ARM Assembler
armasm
has a bug processing macro with array values. It omits the,
delimiter which results in syntax errors.As it is being deprecated, Arm Compiler
armclang
is the recommended tool to compile ARM Assembly Language.However, given that the various Mbed ARM TOOLCHAIN assembly code are written using the legacy
armasm
syntax, the command line option-masm
needs to be passed toarmclang
so it can select the correct assembler for the input assembly source files. This has been successfully tested withmbed-os-example-blinky
.The option was set to
auto
as there are circumstances where the auto-detection might select the wrong assembler for the input file. One of thos circumstances is if the input file is a .S file that requires preprocessing, this is the case for a number of Mbed Assembly source file.Note: The current release of CMake (3.18.2) has a bug for
armclang
where it does not append target definitions to the ASM compiler command.This was reported, subsequentenly fixed and scheduled to be part of the next CMake release.
Thanks @hugueskamba for fixing it.
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers