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

Correct some CPU selections in tools #10390

Merged
merged 3 commits into from Apr 26, 2019
Merged

Conversation

kjbracey
Copy link
Contributor

@kjbracey kjbracey commented Apr 12, 2019

Description

  • For ARMC6, core types Cortex-M4 and Cortex-M7 did not explicitly add --fpu=none, so it defaulted to assuming FPU present. This would cause a compilation error if the target's cmsis.h had __FPU_PRESENT defined to 0.

  • For GCC, Cortex-M33FE did not include +dsp in the architecture selection.

  • For ARMC5 and ARMC6, Cortex-M0+ did not pass M0plus to the non-Clang tools.

  • Add missing Cortex-M33E DSP Extension without FP possibility.

  • Switch KW24D to use ARMC6 - it was using ARMC5 because it has non-FP Cortex-M4 and hit this tool problem.

Pull request type

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

@kjbracey
Copy link
Contributor Author

kjbracey commented Apr 12, 2019

Target KW24D is currently marked to not work with ARMC6 - it's possible that this FPU selection problem is the only reason - it's the only real target with Cortex-M4 that doesn't have an FPU. All others actually do, and should have selected Cortex-M4F - other PRs are in to adjust those.

@ciarmcom ciarmcom requested review from a team April 12, 2019 13:00
@ciarmcom
Copy link
Member

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

Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

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

Nice catch! Looks good to me, probably should run this past @deepikabhavnani too.

@deepikabhavnani
Copy link

No issue in explicitly setting fpu and dsp options, but it will be good to verify them and make sure all combinations are tested in CI, for some weird reasons I have seen fp is accepted and no_fp not with different compiler versions, don;t remember the exact combinations on top of my head but saw this weird behavior recently.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 15, 2019

Lets start first CI

@kjbracey
Copy link
Contributor Author

We would have to add a bunch of dummy targets for the as-yet-unused CPU selections. I'm not sure if it's worth it - we would find any problems immediately when someone tries adding a new target.

(This one did pop up with someone adding a non-FP Cortex-M7 in a private repo).

@mbed-ci
Copy link

mbed-ci commented Apr 15, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_dynamic-memory-usage
  • jenkins-ci/mbed-os-ci_greentea-test

@alekla01
Copy link
Contributor

restarted jenkins-ci/mbed-os-ci_dynamic-memory-usage

@alekla01
Copy link
Contributor

restarted jenkins-ci/greentea-test

@SenRamakri
Copy link
Contributor

@kjbracey-arm - I think "build-ARM" is failing with KW24D cause its still compiling with ARMC5. Please make a change to targets.json for KW24D to replace "ARMC5" entry with "ARM" in supported_toolchains.

@bridadan
Copy link
Contributor

KW24D failed to build with the following error: Fatal error: C3903U: Argument 'Cortex-M4.no_fp' not permitted for option 'cpu'. I think we probably just need to update the cpu flag with the correct combination of Cortex-M4 and "no fpu" option for ARMC5.

@SenRamakri we probably don't want to shift the KW24D to ARMC6 until the target has been verified to work with that right? Is this work already done?

@SenRamakri
Copy link
Contributor

@bridadan - This is the change we need to have KW24D(Coretx-M4 targets) working with ARMC6. @studavekar has already verified this.

@mbed-ci
Copy link

mbed-ci commented Apr 16, 2019

Test run: FAILED

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

Failed test jobs:

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

@bridadan
Copy link
Contributor

Ah good to know! Thanks @SenRamakri

@cmonr
Copy link
Contributor

cmonr commented Apr 16, 2019

[DEBUG] Output: Fatal error: C3903U: Argument 'Cortex-M4.no_fp' not permitted for option 'cpu'.
Traceback (most recent call last):
  File "/builds/ws/mbed-os-ci_build-ARM@2/mbed-os/tools/build.py", line 208, in main
    notify = notifier,
  File "/builds/ws/mbed-os-ci_build-ARM@2/mbed-os/tools/build_api.py", line 771, in build_library
    res, res.get_file_paths(FileType.INC_DIR))
  File "/builds/ws/mbed-os-ci_build-ARM@2/mbed-os/tools/toolchains/mbed_toolchain.py", line 457, in compile_sources
    return self.compile_queue(queue, objects)
  File "/builds/ws/mbed-os-ci_build-ARM@2/mbed-os/tools/toolchains/mbed_toolchain.py", line 530, in compile_queue
    raise ToolException(err)
ToolException: Fatal error: C3903U: Argument 'Cortex-M4.no_fp' not permitted for option 'cpu'.

@kjbracey
Copy link
Contributor Author

Adjusted ARMC5 selections - I want to check that makes the KW24D work with ARMC5, then I'll switch it to use ARMC6.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 17, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Apr 17, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@alekla01
Copy link
Contributor

restarted jenkins-ci/greentea-test

* For ARMC6, core types `Cortex-M4` and `Cortex-M7` did not explicitly
  add `--fpu=none`, so it defaulted to assuming FPU present. This would
  cause a compilation error if the target's cmsis.h had `__FPU_PRESENT`
  defined to 0.

* For GCC, `Cortex-M33FE` did not include `+dsp` in the architecture
  selection.

* For ARMC5 and ARMC6, `Cortex-M0+` did not pass `M0plus` to the
  non-Clang tools.
There was a gap in our pattern - we didn't offer M33 with DSP Extension
but no floating-point.
KW24D was set to ARMC5 because the ARMC6 tooling didn't correctly handle
Cortex-M4 without floating-point. Now fixed.
@kjbracey
Copy link
Contributor Author

kjbracey commented Apr 23, 2019

Cortex-M7F versus M7FD flags to ARMC6 linker+assembler had got muddled - hopefully fixed now.

@SenRamakri
Copy link
Contributor

I started CI on this as it has been pending for more than a day.

@mbed-ci
Copy link

mbed-ci commented Apr 25, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants