Skip to content
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

- Add initial support for ARMv8A, ARMV7R and ARMV8R #45

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Masmiseim36
Copy link
Contributor

  • Added core_cxx.h files for ARMv8A, ARMV7R and ARMV8R based profiles
  • Moved gic peripheral to separate file
  • Moved __FPU_Enable function to the architecture specific file

@github-actions
Copy link

github-actions bot commented Aug 12, 2023

Test Results

   240 files   -   132     240 suites   - 132   0s ⏱️ - 8m 45s
    56 tests +    7      54 ✅ +   10      2 💤  -     3  0 ❌ ±0 
12 948 runs   - 5 280  10 860 ✅  - 1 104  2 088 💤  - 4 176  0 ❌ ±0 

Results for commit a85823e. ± Comparison against base commit b0bbb04.

This pull request removes 49 and adds 56 tests. Note that renamed tests count towards both.
CMSIS-Core.src ‑ apsr.c
CMSIS-Core.src ‑ basepri.c
CMSIS-Core.src ‑ bkpt.c
CMSIS-Core.src ‑ clrex.c
CMSIS-Core.src ‑ clz.c
CMSIS-Core.src ‑ control.c
CMSIS-Core.src ‑ cp15.c
CMSIS-Core.src ‑ cpsr.c
CMSIS-Core.src ‑ dmb.c
CMSIS-Core.src ‑ dsb.c
…
TC_CML1Cache_CleanDCacheByAddrWhileDisabled
TC_CML1Cache_EnDisableDCache
TC_CML1Cache_EnDisableICache
TC_CoreFunc_APSR
TC_CoreFunc_BASEPRI
TC_CoreFunc_Control
TC_CoreFunc_EnDisIRQ
TC_CoreFunc_EncDecIRQPrio
TC_CoreFunc_FAULTMASK
TC_CoreFunc_FPSCR
…
This pull request removes 5 skipped tests and adds 2 skipped tests. Note that renamed tests count towards both.
CMSIS-Core.src ‑ lda.c
CMSIS-Core.src ‑ ldaex.c
CMSIS-Core.src ‑ stl.c
CMSIS-Core.src ‑ stlex.c
CMSIS-Core.src ‑ systick.c
TC_CoreInstr_WFE
TC_CoreInstr_WFI

♻️ This comment has been updated with latest results.

@JonatanAntoni
Copy link
Member

@Masmiseim36, could you please update this one with latest cleanups @GuentherMartin is working on, see PR #56.

Copy link
Contributor

@GuentherMartin GuentherMartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the CMSIS compiler include files missing in r-profile?

@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from 396c99a to 924de88 Compare October 22, 2023 16:04
@Masmiseim36
Copy link
Contributor Author

are the CMSIS compiler include files missing in r-profile?

They come with this pull Request #44. That PR should be merged first
It makes it a bit difficult to break-up the changes in small chunks. I did the testing with the complete change than break it up again to have smaller Pull-Requests

@JonatanAntoni
Copy link
Member

See PR #58 fixing the tests.

@JonatanAntoni JonatanAntoni force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from 924de88 to 4961d52 Compare October 23, 2023 10:00
@JonatanAntoni JonatanAntoni reopened this Oct 24, 2023
@JonatanAntoni JonatanAntoni force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch 2 times, most recently from bc0287b to ce515d6 Compare October 25, 2023 16:43
@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from ce515d6 to f66607e Compare October 27, 2023 20:58
@JonatanAntoni JonatanAntoni added the enhancement New feature or request label Jan 19, 2024
@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from f66607e to dd2c27b Compare January 28, 2024 16:47
@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch 3 times, most recently from 00ed1eb to 7f7aae9 Compare February 26, 2024 19:26
@Masmiseim36
Copy link
Contributor Author

This PR introduces a backward compatibility issue by removing core_ca.h in favour of separate headers per core. Unfortunately, existing Cortex-A DFPs like Cortex DFP refer to core_ca.h explicitly. We need to somehow keep this one for compatibility reasons.

We could use the define __CORTEX_A to bridge into the new headers.

Thanks for the hint. I have reintroduced the file, with the original functionality. Bridging to the new headers would lead to a second definition of __CORTEX_A which would create a compiler warning.

Just fyi, I got aware of another Cortex-R enhancement we didn't pick up, yet. https://github.com/stephanosio/CMSIS_5/blob/core_r/CMSIS/Core_R/

Looks interesting

Another set is in CoreValidation where we check runtime behaviour based on target. This requires execution in fast models (not yet accessible with Arm community license). Or, it could work with Qemu, but I never tried. It should be straight forward to add a build/test configuration for Cortex-R4. But it requires compiler and model support, which won't be part of Arm community license. It should work for GCC and Clang but in absence of a fast model we can't get the tests executed on GitHub.

Thanks for the feedback. I'm trying to read up on the subject. Simulator-based tests are also new to me.
My idea was quite simple. I wanted to compile the functions for GIC, Cache, Timer, MMU, FPU with every compiler and target without errors.

@JonatanAntoni
Copy link
Member

Thanks for the hint. I have reintroduced the file, with the original functionality. Bridging to the new headers would lead to a second definition of __CORTEX_A which would create a compiler warning.

True, sorry, I was mislead by the different approach we used for core_ca.h. Instead of defining __CORTEX_A in the core header we moved the define to the device header. So yes, just lets keep the core_ca.h for compatibility reasons.

Thanks for the feedback. I'm trying to read up on the subject. Simulator-based tests are also new to me. My idea was quite simple. I wanted to compile the functions for GIC, Cache, Timer, MMU, FPU with every compiler and target without errors.

The core tests using LLVM's lit utility run static checks on the compiler output. While this is feasible for simple stuff it might become arbitrarily complex for (extensive) C code. The compiler has a high degree of freedom how to translate certain C statements into assembly language. Of course, lit can be used at any level and not only on disassembly. But I doubt we can find any representation of compiler output that allows consistent/stable checks across different toolchains and versions.

Hence, this kind of tests should rather go to CoreValidation. Even without execution you could enhance the build configurations and check at least error (and warning) free compilation. On the long run we should check if we can add models for all relevant A and R class processors so that we can execute the dynamic checks. The devil is often in the details and even if the C code passe compilation it can easily cause faults at runtime.

@Masmiseim36
Copy link
Contributor Author

Hello @JonatanAntoni ,

I have tried to get the CoreValidation Suite to work. Unfortunately I get an error message. Is another environment variable missing?

  CMake Error at CMakeLists.txt:59 (project):
  The CMAKE_C_COMPILER:

    armclang

  is not a full path and was not found in the PATH.

  Tell CMake where to find the compiler by setting either the environment
  variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
  the compiler, or to the compiler name if it is in the PATH.

I also noticed that the readme for the CoreValidation Suite does not mention that the Cortex_DFP package (https://github.com/ARM-software/Cortex_DFP) must be installed.

If I understand correctly, this package must first be extended for the new core types in order to be able to test them here. But first I want to be able to run the current version locally.

@JonatanAntoni
Copy link
Member

I have tried to get the CoreValidation Suite to work. Unfortunately I get an error message. Is another environment variable missing?

I am not sure, but it might be missing the AC6_TOOLCHAIN_6_21_0=/path/to/armclang-6.21/bin variable which is used to find the install location of different toolchains and versions. If you are installing Arm Compiler via vcpkg this environment variable should be set automatically. Check the CMSIS-Toolbox user guide..

I also noticed that the readme for the CoreValidation Suite does not mention that the Cortex_DFP package (https://github.com/ARM-software/Cortex_DFP) must be installed.

Yes, that's true. The build command as executed via build.py doesn't add --packs argument to cbuild. Hence, required packs are not installed automatically.

If I understand correctly, this package must first be extended for the new core types in order to be able to test them here. But first I want to be able to run the current version locally.

Yes, we need some sort of device family pack for the new devices/cores. Easiest is probably to enhance Cortex DFP. You can do this locally, by using the "local packs flow":

  • Clone the Cortex DFP repo.
  • Register the clone as a "local pack", e.g. cpackget add /path/to/ARM.Cortex_DFP.pdsc.
  • Uninstall any released version of Cortex DFP or pin the pack version to 0.0.0.
    This can either be done in the CSolution .clayer.yml where - pack: ARM::Cortex_DFP@0.0.0 is requested. Or in the generated CMSIS/CoreValidation/Project/Validation.cbuild-pack.yml by modifying the - resolved-pack: ARM::Cortex_DFP@0.0.0 line.

@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from ed4a77e to 33afe88 Compare April 13, 2024 11:10
@Masmiseim36
Copy link
Contributor Author

Hello @JonatanAntoni and @GuentherMartin

Have you changed anything about the core checks?
None of the Cortex-A projects build anymore, not even on the current unchanged head version. It seems that tests that are unsupported are counted as errors.

@JonatanAntoni
Copy link
Member

JonatanAntoni commented Apr 29, 2024

Have you changed anything about the core checks? None of the Cortex-A projects build anymore, not even on the current unchanged head version. It seems that tests that are unsupported are counted as errors.

Not that I am aware of. PR workflow shall prevent that.
I just check main branch on my machine for all Cortex-A devices, all compilers and optimization modes without issues.

Please double check your environment. Mine was:

arm:compilers/arm/armclang 6.21.0  installed            Arm Compiler for Embedded
arm:compilers/arm/arm-none-eabi-gcc 13.2.1  installed            GCC compiler for ARM CPUs
arm:compilers/arm/llvm-embedded 17.0.1  installed            LLVM Embedded Toolchain for Arm CPUs

And after update:

arm:compilers/arm/armclang 6.22.0  installed            Arm Compiler for Embedded
arm:compilers/arm/arm-none-eabi-gcc 13.2.1  installed            GCC compiler for ARM CPUs
arm:compilers/arm/llvm-embedded 17.0.1  installed            LLVM Embedded Toolchain for Arm CPUs

@Masmiseim36
Copy link
Contributor Author

Hello @JonatanAntoni

the whole thing is a bit strange.

I have the same behavior locally as in this pull request, whether on this commit, the current head or on older versions: With the AC6 compiler all Cortex-A tests fail.

What makes the whole thing even stranger is that the errors only occur when I start all tests via ' build.py lit'. However, if I specify the compiler and target architecture, it works (build.py -c AC6 -d CA53 lit).

I had a similar problem before. Due to the lack of a license for AC6, every test that was not for Cortex-M0 failed. Since I have been using the MDK6 Community license, however, everything has been OK. Could this be a license problem again?

@JonatanAntoni
Copy link
Member

Hi @Masmiseim36,

It probably is an licensing issue. The workflow for your PR runs in "isolation", i.e., it doesn't have access to the professional Arm license. Hence, all tests for Cortex-A and -R are supposed to fail.

Let me check your PR on my local machine. Then, I can recreate the PR with my maintainer privileges which gives it access to the license.

CMSIS/Core/Test/lit.cfg.py Outdated Show resolved Hide resolved
CMSIS/Core/Test/lit.cfg.py Outdated Show resolved Hide resolved
CMSIS/Core/Test/lit.cfg.py Outdated Show resolved Hide resolved
CMSIS/Core/Test/lit.cfg.py Outdated Show resolved Hide resolved
@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch 2 times, most recently from 8d2bcd8 to 4f98c7f Compare May 7, 2024 17:05
@Masmiseim36
Copy link
Contributor Author

The PR Open-CMSIS-Pack/Open-CMSIS-Pack-Spec#312 should solve the issues during pack generation

@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from 4f98c7f to 75f40ab Compare May 7, 2024 19:25
@JonatanAntoni
Copy link
Member

The PR Open-CMSIS-Pack/Open-CMSIS-Pack-Spec#312 should solve the issues during pack generation

We need to update the pack schema reference URL in the pdsc file once this PR is merged.

@JonatanAntoni JonatanAntoni force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from 75f40ab to 432826e Compare May 13, 2024 13:03
ARM.CMSIS.pdsc Outdated Show resolved Hide resolved
ARM.CMSIS.pdsc Outdated Show resolved Hide resolved
@JonatanAntoni
Copy link
Member

The PR Open-CMSIS-Pack/Open-CMSIS-Pack-Spec#312 should solve the issues during pack generation

We need to update the pack schema reference URL in the pdsc file once this PR is merged.

The updated schema is now available at

https://raw.githubusercontent.com/Open-CMSIS-Pack/Open-CMSIS-Pack-Spec/v1.7.37/schema/PACK.xsd

@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from 432826e to 9a7d696 Compare May 13, 2024 18:46
@Masmiseim36
Copy link
Contributor Author

The PR Open-CMSIS-Pack/Open-CMSIS-Pack-Spec#312 should solve the issues during pack generation

We need to update the pack schema reference URL in the pdsc file once this PR is merged.

Any plans for 0.11.2 release?

@JonatanAntoni
Copy link
Member

The PR Open-CMSIS-Pack/Open-CMSIS-Pack-Spec#312 should solve the issues during pack generation

We need to update the pack schema reference URL in the pdsc file once this PR is merged.

Any plans for 0.11.2 release?

What 0.11.2 release? The above PR to the pack schema has been merged and published via the schema version 1.3.37 tag.

Please update the schema reference in pdsc file at
https://github.com/ARM-software/CMSIS_6/pull/45/files#diff-2f515e4f53b33d558f9d7ef1cea041f96465055739878ea684a6f19dae2f84efR3

@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from 9a7d696 to 780e513 Compare May 14, 2024 20:15
@Masmiseim36
Copy link
Contributor Author

The PR Open-CMSIS-Pack/Open-CMSIS-Pack-Spec#312 should solve the issues during pack generation

We need to update the pack schema reference URL in the pdsc file once this PR is merged.

Any plans for 0.11.2 release?

What 0.11.2 release? The above PR to the pack schema has been merged and published via the schema version 1.3.37 tag.

Please update the schema reference in pdsc file at https://github.com/ARM-software/CMSIS_6/pull/45/files#diff-2f515e4f53b33d558f9d7ef1cea041f96465055739878ea684a6f19dae2f84efR3

I have mixed the gen-pack version with the pack-spec version. Sorry for that

- Added core_cxx.h files for ARMv8A, ARMV7R and ARMV8R based profiles
- Moved gic peripheral to separate file
- Moved __FPU_Enable function to the architecture specific file
- Extend core-test routines for new supported core-types
@Masmiseim36 Masmiseim36 force-pushed the feature/AddInitialARMv8AandARMv7Rsupport branch from 780e513 to a85823e Compare May 20, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants