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

Fallback to ARMC5 when ARMC6 is not configured #10193

Merged
merged 11 commits into from Mar 24, 2019

Conversation

Projects
None yet
10 participants
@bridadan
Copy link
Contributor

commented Mar 22, 2019

Description

This PR makes it so:

  • if -t ARM is being supplied as the toolchain
  • and if ARMC6 is not configured in the user's environment
  • and ARMC5 is configured
  • and the the target supports building with both ARMC5 and ARMC6
    Then the tools will select ARMC5 as the toolchain for building. Previously, this was always set to ARMC6 (with a few targets being the exception).

If the tools fallback to ARMC5, the following warning will be printed at the bottom of the log:

------------------------------------------------------------
Warning: We noticed that you are using Arm Compiler 5. We are deprecating the use of Arm Compiler 5 soon. Please upgrade your environment to Arm Compiler 6 which is free to use with Mbed OS. For more information, please visit https://os.mbed.com/docs/mbed-os/latest/tools/index.html
------------------------------------------------------------

This feature has been documented here (thanks @SenRamakri): ARMmbed/mbed-os-5-docs#1024

Normally I'd write out all the implications of the changes in this PR, however the implementation details are fairly extensive. Reading the docs above is a good place to start, probably followed by the commit history.

FYI @screamerbg

TODO

  • The ARMC5 warning needs to be updated with a link to the proper doc.
  • I would really prefer to bring this in with a unit test. The implementation is quite complex and the details can easily be lost if verifying manually. Most likely this will have to come in in a later release due to time constraints.

Pull request type

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

Reviewers

@SenRamakri @theotherjimmy @bulislaw

Release Notes

The -t ARM argument defaults to using the Arm Compiler 6 when it is configured in a user's enviroment. However, In cases where a target supports building with both Arm Compiler 5 and Arm Compiler 6 and Arm Compiler 6 is not properly configured, the tools will fallback to compiling with Arm Compiler 5.

@ciarmcom ciarmcom requested review from bulislaw, SenRamakri, theotherjimmy and ARMmbed/mbed-os-maintainers Mar 22, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Started CI to get early test run

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Mar 22, 2019

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 22, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-IAR8
  • jenkins-ci/mbed-os-ci_build-GCC_ARM
@mbed-ci

This comment has been minimized.

Copy link

commented Mar 22, 2019

Test run: FAILED

Summary: 2 of 9 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-IAR8
@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Looks like this has affect on one target and its unsupported toolchains (build failures)

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Setting to rc4

cc @adbridge @bulislaw

@bulislaw
Copy link
Member

left a comment

Looks good. Please remember to add a link to docs to the warning.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

@0xc0170 The PR is failing because the CI is attempting to build the examples for the NUMAKER_PFM_M2351 target using IAR and GCC_ARM, both of which the target does not support (and that is the error we get in CI: "Could not compile for NUMAKER_PFM_M2351: Target NUMAKER_PFM_M2351 is not supported by toolchain IAR").

In my opinion there are two fixes for this:

  1. Change CI so it doesn't attempt the build in the first place
  2. Change the example builder script its invoking so it double checks supported toolchains and does not raise an error

I'd prefer to go with choice 1. Any thoughts @OPpuolitaival?

@bridadan bridadan marked this pull request as ready for review Mar 22, 2019

@0xc0170 0xc0170 added needs: review and removed needs: work labels Mar 22, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

Thanks to @OPpuolitaival, I believe the last commit will fix the CI failure.

@adbridge

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2019

ci started

@adbridge adbridge added needs: CI and removed needs: review labels Mar 22, 2019

@bulislaw
Copy link
Member

left a comment

Looks good (here's a 🍩 token that can be redeemed next time I'm around. Don't loose it though there's no replacement policy!)

@bridadan bridadan force-pushed the bridadan:arm_tc_with_fallback branch from 6f96d01 to bbcc25c Mar 22, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

I'm looking into the exporter issue. Since it failed on #10131 as well I doubt my changes caused this issue.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

The exporter issue is unrelated to this change, however I've pushed a fix (hopefully) for it here: #10199

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 22, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter
@mbed-ci

This comment has been minimized.

Copy link

commented Mar 22, 2019

Test run: FAILED

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

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter
@SenRamakri
Copy link
Collaborator

left a comment

Changes looks good to me, although uvision6 exporter seems to be failing.

@adbridge

This comment has been minimized.

Copy link
Contributor

commented Mar 23, 2019

@bridadan could you resolve the conflict please, this is the last PR needing to get through ci now ?

bridadan added some commits Mar 22, 2019

Only enable ARMC6 for a few targets
The affected targets are Renesas targets, USI_WM_BN_BM_22 based
targets, and the MTB_MXCHIP_EMW3166.
Add utility functions and Exceptions
These functions will be used to handle some of the error state and
warning messages produced when the scripts attempt to select a valid
toolchain.
Add functions to enable ARM fallback to ARMC5.
There are two new functions: get_valid_toolchain_names and
find_valid_toolchain. These functions are used to figure out if a
fallback is possible and necessary. find_valid_toolchain is expected to
be used by the front-end scripts.

get_toolchain_name was updated with some different logic and comments.
Front-end scripts now use the ARM toolchain fallback.
Some unused imports were removed as well as some general clean up.
Make args_erroor less verbose.
This removes the arugment help from the output, making the error much
easier to find. This solves #10090.

@bridadan bridadan force-pushed the bridadan:arm_tc_with_fallback branch from bbcc25c to b95732a Mar 23, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 23, 2019

Rebased!

@adbridge

This comment has been minimized.

Copy link
Contributor

commented Mar 23, 2019

ci started

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 24, 2019

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter
@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 24, 2019

exporters restarted, license server error

@NirSonnenschein NirSonnenschein merged commit b29b55a into ARMmbed:master Mar 24, 2019

28 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARMC5 Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR8 Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARMC5 Success
Details
jenkins-ci/mbed2-build-ARMC6 Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR8 Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9150 cycles (-795 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.