Skip to content

Correct typo no_dsp - nodsp #9606

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
Merged
merged 1 commit into from
Feb 5, 2019
Merged

Correct typo no_dsp - nodsp #9606

merged 1 commit into from
Feb 5, 2019

Conversation

deepikabhavnani
Copy link

Description

Correct typo - no dsp option for armclang is nodsp (without underscore)

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Correct typo no_dsp - nodsp
b49a6b2 
armclang - nodsp
armlink - no_dsp

diff --git a/tools/t
@ciarmcom ciarmcom requested review from a team February 4, 2019 22:00
@ciarmcom
Copy link
Member

ciarmcom commented Feb 4, 2019

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

NirSonnenschein
NirSonnenschein reviewed Feb 5, 2019
View reviewed changes
tools/toolchains/arm.py
@@ -404,8 +404,8 @@ def __init__(self, target, *args, **kwargs):
"Cortex-M4F": "cortex-m4",
"Cortex-M7F": "cortex-m7",
"Cortex-M7FD": "cortex-m7",
"Cortex-M33": "cortex-m33+no_dsp+no_fp",
"Cortex-M33F": "cortex-m33+no_dsp",
"Cortex-M33": "cortex-m33+nodsp",
Copy link
Contributor

Choose a reason for hiding this comment

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

no_fp was removed as well as the fix to no_dsp, is this intentional?

Copy link
Author

Choose a reason for hiding this comment

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

Yes nofp as well gave error when used with clang. No_fp is valid with armlink

Copy link
Contributor

Choose a reason for hiding this comment

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

Just wanted to make sure as this wasn't mentioned in the commit comment or PR description.

@NirSonnenschein
Copy link
Contributor

CI started

@jeromecoutant
Copy link
Collaborator

Hi
Could you check in GCC as well ?

[DEBUG] Output: arm-none-eabi-gcc: error: unrecognized argument in option '-mcpu=cortex-m33+no_dsp'

@mbed-ci
Copy link

mbed-ci commented Feb 5, 2019

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 1
Build artifacts

@deepikabhavnani
Copy link
Author

deepikabhavnani commented Feb 5, 2019 •
edited
Loading

~~Isn’t dsp option nodsp - c472005#diff-f8a1211bf0ff7089cff6fc47859a1734R78~~~

@jeromecoutant - My bad, same mistake was done while creation of lookup tables. febbeff#diff-f8a1211bf0ff7089cff6fc47859a1734. Since this PR has passed CI will add it to another PR.

@mikisch81
Copy link
Contributor

@ARMmbed/mbed-os-maintainers ready for merge?

@0xc0170 0xc0170 merged commit 4ce6ed0 into ARMmbed:master Feb 5, 2019
@kjbracey kjbracey mentioned this pull request Feb 6, 2019
Merged
@deepikabhavnani deepikabhavnani deleted the core_arch_v8m branch March 1, 2019 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bridadan bridadan bridadan approved these changes

@NirSonnenschein NirSonnenschein NirSonnenschein approved these changes

Assignees
No one assigned
Labels
release-version: 5.11.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@deepikabhavnani @ciarmcom @NirSonnenschein @jeromecoutant @mbed-ci @mikisch81 @bridadan @0xc0170