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

cmsis: remove arm math #12055

Merged
merged 3 commits into from
Jan 8, 2020
Merged

cmsis: remove arm math #12055

merged 3 commits into from
Jan 8, 2020

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Dec 9, 2019 •
edited
Loading

Summary of changes

Not needed file, should be part of DSP. The functionality is part of the library provided
by CMSIS.

I would like to test this, for all devices. This header file should be removed in 6.0.0 as we do not provide DSP library. Thus I set this as Major update.

Fixes #12054

Impact of changes

Applications should use CMSIS DSP package. As the order of includes is not defined in our tools, this might cause an error - hard to tell which cmsis math header was actually included. The removed math header we used in this code base was much older version.

Migration actions required

Use CMSIS DSP release (not part of Mbed OS at the moment) - the header file cmsis math is included there.

Documentation

Not needed

Pull request type 

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[X] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results 

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@ciarmcom ciarmcom requested a review from a team December 9, 2019 12:00
@ciarmcom
Copy link
Member

ciarmcom commented Dec 9, 2019

@0xc0170, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@0xc0170 0xc0170 added the release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 label Dec 9, 2019
@0xc0170 0xc0170 requested a review from a team December 11, 2019 13:31
@bulislaw
Copy link
Member

I remember this being needed as there was some math compiler intrinsics being missing from IAR.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Dec 11, 2019

I remember this being needed as there was some math compiler intrinsics being missing from IAR.

Me as well not certain which one. Newer IAR hopefully has it in or we find anothe way rather than keeping this outdated file causing conflicts for users.

We'll run CI once 5.15rc2 is completed

@0xc0170
Copy link
Contributor Author

0xc0170 commented Dec 16, 2019

Needs first CI run

@kjbracey
Copy link
Contributor

CI started

@mbed-ci
Copy link

mbed-ci commented Dec 17, 2019

Test run: FAILED

Summary: 4 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_unittests
  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-ARM
  • jenkins-ci/mbed-os-ci_build-IAR

@kjbracey
Copy link
Contributor

[Fatal Error] fsl_powerquad_cmsis.c@10,10: 'arm_math.h' file not found

for LPC55S69_NS

@0xc0170
cmsis: remove arm math
5a985a7 
Fixes ARMmbed#12054

Not needed file, should be part of DSP. The functionality is part of the library provided
by CMSIS.
@0xc0170
LPC55S6: remove cmsis powerquad
88ae99f 
This file requires CMSIS library (DSP).
@0xc0170
Copy link
Contributor Author

0xc0170 commented Jan 3, 2020

Rebased on top of lastest master. And also updated LPC code - removed cmsis implementation as DSP is not part of Mbed OS

@mmahadevan108 Please review

@0xc0170 0xc0170 requested a review from a team January 3, 2020 12:32
@0xc0170
Copy link
Contributor Author

0xc0170 commented Jan 3, 2020

CI restarted

@mbed-ci
Copy link

mbed-ci commented Jan 3, 2020

Test run: FAILED

Summary: 1 of 4 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-IAR

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jan 3, 2020

This failure with IAR uncovers this one:

#if defined(__ICCARM__) && ( defined(__CORTEX_M0) || defined(__CORTEX_M0P) || defined(__CORTEX_M23))
#include <arm_math.h>
#endif

I believe updated IAR should have this fixed, will test.

@0xc0170 0xc0170 requested a review from a team January 3, 2020 15:26
@mbed-ci
Copy link

mbed-ci commented Jan 3, 2020

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

@0xc0170 0xc0170 requested a review from bulislaw January 7, 2020 08:29
@0xc0170
Copy link
Contributor Author

0xc0170 commented Jan 7, 2020

I remember this being needed as there was some math compiler intrinsics being missing from IAR.

@bulislaw all green, shall we get this in today?

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jan 8, 2020

I believe this is ready to get in. Let's check nightly tomorrow morning if there is any other issue with this header being removed.

I set this to breaking change as this header might cause breaking compilation - to be part of the release notes and notify users about this one. I'll add release notes now.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Jan 8, 2020

Release notes added

@0xc0170 0xc0170 merged commit b77f6b4 into ARMmbed:master Jan 8, 2020
biagiom added a commit to biagiom/tensorflow that referenced this pull request Apr 19, 2020
@biagiom
Fix build issues in micro_speech project
d99bfe1 
* Remove non-existant `arm_cmplx_mag_squared_q10p6.c/h` from
  "tensorflow/lite/micro/examples/micro_speech/CMSIS/Makefile.inc",
  which cause the build of micro_speech project to fail.
  See also: tensorflow#36444

* Remove unnecessary `third_party/CMSIS_ext/README.md` from `CMSIS_PREPROCESSOR_HDRS`
  in "tensorflow/lite/micro/examples/micro_speech/CMSIS/Makefile.inc",
  which cause the build of micro_speech project to fail.

* Update MBED version to mbed-os-6.0.0-alpha-3: this fixes the issues related
  to `arm_mult_q15.c` in the ARM math library when building the micro_speech
  project.
  See also:
  1. tensorflow#37930
  2. ARMmbed/mbed-os#12055
@0xc0170 0xc0170 deleted the fix_12054 branch October 28, 2020 08:42
@MarceloSalazar MarceloSalazar mentioned this pull request Oct 28, 2020
Closed
0xc0170 added a commit to 0xc0170/mbed-os that referenced this pull request Nov 2, 2020
@0xc0170
cmsis: remove math header file
0bb4ff0 
It was removed in 6.0 (see reference below), and it was reintroduced when we updated
cmsis from the upstream. We missed to remove the commit adding the file in the cmsis
importer. This fixes it and the file should not be introduced again.

Fixes ARMmbed#13823

Already removed in 6.0: ARMmbed#12055
@0xc0170 0xc0170 mentioned this pull request Nov 2, 2020
Merged
facchinm pushed a commit to arduino/mbed-os that referenced this pull request Nov 9, 2020
@0xc0170 @facchinm
cmsis: remove math header file
2398c20 
It was removed in 6.0 (see reference below), and it was reintroduced when we updated
cmsis from the upstream. We missed to remove the commit adding the file in the cmsis
importer. This fixes it and the file should not be introduced again.

Fixes ARMmbed#13823

Already removed in 6.0: ARMmbed#12055
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmahadevan108 mmahadevan108 mmahadevan108 approved these changes

@ccli8 ccli8 ccli8 approved these changes

@kjbracey kjbracey kjbracey approved these changes

@bulislaw bulislaw bulislaw approved these changes

Assignees
No one assigned
Labels
BREAKING-CHANGE release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

arm math: DSP header file should be part of CMSIS DSP, not alone in cmsis
7 participants
@0xc0170 @ciarmcom @bulislaw @kjbracey @mbed-ci @ccli8 @mmahadevan108