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.

Sign up for GitHub

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

Jump to bottom

Split up the compiler files to a generic part and a profile specific part #44

Merged
merged 1 commit into from
Feb 26, 2024
Merged

Split up the compiler files to a generic part and a profile specific part #44

merged 1 commit into from
Feb 26, 2024

Conversation

Masmiseim36
Copy link
Contributor

  • Top-level compiler headers will exist in the Core folder (no separate Compiler folder is needed).
  • The cmsis_compiler.h header will continue to figure out which compiler toolchain is being used. ** Compiler headers specific to each architecture profiles can reside within the new architecture profile folders.

@github-actions
Copy link

github-actions bot commented Aug 9, 2023 •
edited
Loading

Test Results

0 files   -    348  0 suites   - 348   0s ⏱️ - 7m 54s
0 tests  -     49  0 ✅  -     44  0 💤  -     5  0 ❌ ±0 
0 runs   - 17 052  0 ✅  - 11 076  0 💤  - 5 976  0 ❌ ±0 

Results for commit 2f82b0d. ± Comparison against base commit 688b703.

♻️ This comment has been updated with latest results.

github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 9, 2023
View reviewed changes
CMSIS/Core/Include/cmsis_gcc.h Fixed Show fixed Hide fixed
CMSIS/Core/Include/cmsis_gcc.h Fixed Show resolved Hide resolved
CMSIS/Core/Include/cmsis_gcc.h Dismissed Show dismissed Hide dismissed
CMSIS/Core/Include/cmsis_gcc.h Dismissed Show dismissed Hide dismissed
CMSIS/Core/Include/m-profile/cmsis_gcc_m.h Fixed Show fixed Hide fixed
@Masmiseim36 Masmiseim36 mentioned this pull request Sep 29, 2023
Closed
@JonatanAntoni
Copy link
Member

@Masmiseim36, I am adding some additional validation tests to assure feature parity between all compilers, see PR #52. I think we should update this PR once the tests are there. What do you think?

@Masmiseim36 Masmiseim36 mentioned this pull request Oct 22, 2023
Open
@Masmiseim36
Copy link
Contributor Author

@Masmiseim36, I am adding some additional validation tests to assure feature parity between all compilers, see PR #52. I think we should update this PR once the tests are there. What do you think?

Sure, the more tests the better. I have already rebased the PR. I think you have to start the tests again manually?

@JonatanAntoni
Copy link
Member

Sure, the more tests the better.

May I ask you to comment on PR #52? What do you think?

I think you have to start the tests again manually?

No, CoreValidation tests got triggered automatically and failed.
The problem is we decided to rename CMSIS_DFP into Cortex_DFP to be more specific about its content and to properly distinguish to other CMSIS components (like CMSIS-RTX, CMSIS-DSP, and CMSIS-NN).

I need to fix this on master and ask you to rebase once again. Sorry for this inconvenience.

-Jonatan

@JonatanAntoni
Copy link
Member

See PR #58 fixing the tests.

@JonatanAntoni
Copy link
Member

@Masmiseim36, sorry, I think I messed your PR by adding a preliminary cmsis_clang_a.h. Could you try to rebase this PR? Might it make sense to revisit the test suites, i.e. core tests and core validation for completeness in terms of Cortex-A? We might be limited in what we can actually test by the available compiler support.

@Masmiseim36
Copy link
Contributor Author

@Masmiseim36, sorry, I think I messed your PR by adding a preliminary cmsis_clang_a.h. Could you try to rebase this PR? Might it make sense to revisit the test suites, i.e. core tests and core validation for completeness in terms of Cortex-A? We might be limited in what we can actually test by the available compiler support.

Hello @JonatanAntoni
I have rebased the branch. I’m in the moment on vacation, so I’m quite limited in time and testing-possibilities. I may have a look at it after my holiday

@JonatanAntoni
Copy link
Member

Hi @Masmiseim36,

Are you still working on this contribution?

@JonatanAntoni JonatanAntoni added the enhancement New feature or request label Jan 19, 2024
@Masmiseim36
Copy link
Contributor Author

Hi @Masmiseim36,

Are you still working on this contribution?

Hello @JonatanAntoni

Sorry for the long period of inactivity. It's extremely stressful at work at the moment and time sometimes seems to be running away.
In the meantime, the two implementations for the a- and m-profile have diverged again. I have therefore first created a pull request that brings the two variants back together. After that I can continue here.
--> #128

@Masmiseim36 Masmiseim36 force-pushed the feature/RemoveDuplicatedCode branch 2 times, most recently from f00fb31 to f06b915 Compare February 14, 2024 23:12
@Masmiseim36
* Top-level compiler headers will exist in the core folder (no separa…
2f82b0d 
…te compiler folder is needed).

** The cmsis_compiler.h header will continue to figure out which compiler toolchain is being used.
** Compiler headers specific to each architecture profiles can reside within the new architecture profile folders.
* Introduce compiler abstraction for cortex-R
@Masmiseim36
Copy link
Contributor Author

Hello @JonatanAntoni
Is there anything else I can contribute to this pull request?
Regards

@JonatanAntoni JonatanAntoni merged commit 4d18b8d into ARM-software:main Feb 26, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonatanAntoni JonatanAntoni JonatanAntoni approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Masmiseim36 @JonatanAntoni