-
Notifications
You must be signed in to change notification settings - Fork 57
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
Core(M): Add support for LLVM/Clang #19
Conversation
JonatanAntoni
commented
Jun 27, 2023
- Add compiler header for clang.
- Enhance CoreValidation for clang.
4f8d602
to
552460e
Compare
83c1ce5
to
552460e
Compare
3cb4c3c
to
d315ea5
Compare
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.
Looks fine to me
CMSIS/Core/Include/cmsis_clang.h
Outdated
{ | ||
uint32_t result; | ||
|
||
#if __ARM_ISA_THUMB >= 2 |
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.
replace __ARM_ISA_THUMB with __ARM_ARCH_ISA_THUMB
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
#endif | ||
|
||
#ifndef __INITIAL_SP | ||
#define __INITIAL_SP __stack |
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.
where is __stack defined?
why not use __StackTop as in GCC?
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.
This is a linker defined symbol, defined in the linker script. We can use same names as in GCC if we adopt the linker script. I kept the symbols as defined in the toolchain provided linker script template.
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.
if we provide the linker script than I would suggest to align the symbol names.
#endif | ||
|
||
#ifndef __STACK_LIMIT | ||
#define __STACK_LIMIT __stack_limit |
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.
where is __stack_limit defined?
why not use __StackLimit as in GCC?
|
||
#if defined (__ARM_FEATURE_CMSE) && (__ARM_FEATURE_CMSE == 3U) | ||
#ifndef __STACK_SEAL | ||
#define __STACK_SEAL __stack_seal |
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.
where is _stack_seal defined?
why not use __StackSeal as in GCC
CMSIS/Core/Include/cmsis_clang.h
Outdated
} | ||
#endif /* __ARM_ARCH_ISA_THUMB >= 2 */ | ||
|
||
#ifdef __ARM_FEATURE_LDREX |
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.
Change
#ifdef __ARM_FEATURE_LDREX
to
#if __ARM_FEATURE_LDREX >= 7
see https://developer.arm.com/documentation/101028/0012/5--Feature-test-macros
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
} | ||
|
||
/** | ||
\brief LDRT Unprivileged (8 bit) |
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.
are the LDR*/STR* Unprivileged instructions also covered by __ARM_FEATURE_LDREX?
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 point.
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.
LDRT
is Thumb-2 instruction, so nothing to do with exclusive access ones.
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