-
Notifications
You must be signed in to change notification settings - Fork 129
Run Core tests and CoreValidation with IAR toolchain #229
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
base: main
Are you sure you want to change the base?
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 adds support for running Core tests with the IAR toolchain by updating several test check patterns and configuration files. Key changes include:
- Updated CHECK patterns in test source files to distinguish between IAR and non-IAR output.
- Added a new Toolchain_IAR class and modified configuration settings in lit.cfg.py.
- Updated build scripts and workflow configurations to support IAR.
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| CMSIS/Core/Test/src/simd.c | Updated CHECK patterns to separate IAR and non-IAR cases. |
| CMSIS/Core/Test/src/rrx.c | Modified CHECK patterns to include both rrx and rrxs instructions. |
| CMSIS/Core/Test/src/ror.c | Revised CHECK patterns to update regex for THUMB and ARM variants. |
| CMSIS/Core/Test/src/noreturn.c | Adjusted regex checks to support both b and bl branch instructions. |
| CMSIS/Core/Test/src/cpsr.c | Minor regex update to allow optional 's' in the AND operation check. |
| CMSIS/Core/Test/lit.cfg.py | Added Toolchain_IAR class with new configuration settings; updated mfpu. |
| CMSIS/Core/Test/build.py | Removed obsolete IAR matrix filter support. |
| CMSIS/Core/Include/m-profile/cmsis_iccarm_m.h | Simplified preprocessor conditions regarding architecture features. |
| .github/workflows/core.yml | Added IAR token environment variable for CI integration. |
Files not reviewed (1)
- CMSIS/Core/Test/vcpkg-configuration.json: Language not supported
Comments suppressed due to low confidence (2)
CMSIS/Core/Test/lit.cfg.py:891
- The variable 'device' is used instead of 'self.device' when accessing the DEVICES configuration. Update to 'DEVICES[self.device]["header"]' for consistency.
f'--cpu={self.cpu()}', f'--fpu={self.fpu()}', '-I', os.path.abspath('../Include'), '-c', '-D', f'CORE_HEADER="{DEVICES[device]["header"]}"'
CMSIS/Core/Test/lit.cfg.py:892
- The variable 'device' is undefined in this context; use 'self.device' to ensure the correct configuration is referenced.
if device.endswith('S') and not device.endswith('NS'):
|
@RobinKastberg, I enhanced CI tests with IAR toolchain. Could you please cross check at least the modification to IAR compiler header? Looks like many test cases do not pass with IAR compiler. Could you have a look as well? We might need to enhance the test expectations as IAR compiler generates another assembly variant. |
Great work!
I will take a look, but I am quite busy with other things currently. |
820212c to
2f3846e
Compare
|
@RobinKastberg, I am now about to run CoreValidation tests. But I am seeing a weird issue on GitHub. Despite having set This is surprising, because in within the Core test job the toolchain has no issues. |
|
@JonatanAntoni where did you get toolchain from? ARM artifactory? |
Yes, its installed to the build machine via To be exact, its downloaded from https://github.com/iarsystems/arm/releases/download/9.60.4/cxarm-9.60.4-linux-x86_64-base.tar.bz2 by the vcpkg manifest distributed via Arm Artifactory. It is working for the Core test job, without issues. In there, the compiler is launched directly via |
|
@JonatanAntoni Issue is currently being looked into. We don't know details yet. Can you try rerunning the job a few times. Does it happen every time? |
Yes, the behaviour is reproducible each time. |
d3d0f84 to
8a420dc
Compare
f64544b to
34df51b
Compare
ed512cb to
20b1e31
Compare
|
@RobinKastberg, now the toolchain seems to run correctly. We need to focus on fixing the tests, now. Can you help with that? |
20b1e31 to
76338d5
Compare
Nice! I will get started on it next week! |
|
@JonatanAntoni I took a look at build errors for CoreValidation to start with, couldn't get FVP running this time due to licensing issues (I mailed you regarding this)
I will take a look at the lit tests next. |
|
Thanks for spotting such issues. Yes, I'd expect most of the issues to be some toolchain integration issues, such as wrong/misaligned compiler flags. We need to sort them out gradually. |
774691b to
2362d73
Compare
|
Do not forget to update the README.md file with instructions on how to build with IAR. Currently, it is missing. |
|
This looks good, however I am currently busy with other things, so my hunting down the last lit tests will take a while. |
I am a bit hesitant about merging changes that cause test failures. |
e224a92 to
d075cc3
Compare
No description provided.