Make ARMC6 as the default ARM toolchain for MTB targets #9860

SenRamakri · 2019-02-26T15:54:39Z

Description

This PR is to make ARMC6 as the default ARM toolchain for MTB targets below:
MTB_ADV_WISE_1530
MTB_MXCHIP_EMW3166
MTB_USI_WM_BN_BM_22

Pull request type

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

Reviewers

@cmonr @0xc0170 @SeppoTakalo

ciarmcom · 2019-02-26T16:00:37Z

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

0xc0170 · 2019-02-26T17:32:04Z

Just to make sure, a reason for this is?

cmonr · 2019-02-26T17:36:06Z

@SenRamakri I only see two targets updated in targets.json?

SenRamakri · 2019-02-26T17:51:59Z

@cmonr - Actually USI_WM_BN_BM_22 is inherited by both MTB_ADV_WISE_1530 and MTB_USI_WM_BN_BM_22, so that makes it 3.

SenRamakri · 2019-02-26T18:11:55Z

@ARMmbed/mbed-os-maintainers - Looks like continuous-integration/travis-ci/pr job failed due to some infrastructure issues. We may have to restart test on this.

theotherjimmy

Looks fine to me.

cmonr · 2019-02-26T22:09:18Z

@SenRamakri The results will posted shortly, but it looks like build-ARMC6 did have some build issues.

I suspect CI continued because of an earlier config request to have the pipeline continue even on a build failure.

SenRamakri · 2019-02-26T22:18:28Z

@cmonr - Yes, there are some failures. I looked into the failure logs and I guess those targets(which failed the builds) were tried to compile on machines without ARMC6 installed and thus failed as below:

[OS ERROR] Command: armclang -c --target=arm-arm-none-eabi -mthumb -Os -Wno-armcc-pragma-push-pop -Wno-armcc-pragma-anon-unions -DMULADDC_ ....

I have seen in the past that the [OS ERROR] usually happens when the executables are not available.
Adding @OPpuolitaival @timurh01 to take a look to confirm. I'm also aware that CI team is working on enabling ARMc5 and ARMc6 builds in CI based on targets.json.

mbed-ci · 2019-02-26T23:18:59Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

SenRamakri · 2019-02-27T02:54:49Z

@ARMmbed/mbed-os-maintainers - As a sanity check, I tried compiling each of the failing targets with ARMC6 locally and they are all passing successfully.

cmonr · 2019-02-27T04:23:31Z

@SenRamakri has more information, and needs to sync up with @ARMmbed/mbed-os-test-team

OPpuolitaival · 2019-02-27T13:16:36Z

@SenRamakri there was some old exceptions in CI scripts. Fixed and retriggered the test

mbed-ci · 2019-02-27T14:12:22Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test
jenkins-ci/mbed-os-ci_greentea-test

theotherjimmy · 2019-02-27T14:59:21Z

tools/build_api.py

@@ -283,7 +291,7 @@ def get_mbed_official_release(version):
                [
                    TARGET_MAP[target].name,
                    tuple(transform_release_toolchains(
-                        TARGET_MAP[target].supported_toolchains, version))
+                        TARGET_MAP[target].supported_toolchains, version, target))


I'm pretty sure that this is passing in the target string, and not the target object. Since transform_release_toolchains is only ever called here, how about changing the function to take target, version instead of supported_toolchain_list, version, target? That way we don't have to do lookup 3(!) times.

Is there a reason for not using TARGET_MAP[target].supported_toolchains, as in #9878?
Does targets.json only define mbed2 supported toolchains?

@alekla01 - Please see my changes in this file to take care of previous tools version as well.
But you asked great question on why we are not using
supported toolchains. I would like to investigate why we are not just return
supported toolchain. The current attempt is to keep the behavior same as before as it might impact online compiler.

tools/build_api.py

…in matrix output

mbed-ci · 2019-02-27T21:17:07Z

Test run: FAILED

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

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

mbed-ci · 2019-02-28T03:19:21Z

Test run: SUCCESS

Summary: 4 of 4 test jobs passed
Build number : 5
Build artifacts

SenRamakri · 2019-02-28T04:18:39Z

@ARMmbed/mbed-os-maintainers - Looks like the tests passed. Can we please go ahead merge this? I'm waiting on this to create the merge PR to master.

cmonr

I'm fine with this for now, but will want to do a deeper scrub when the PR to master is opened.

cmonr · 2019-02-28T05:48:15Z

Have let the other maintainers know this is ready.

Too sleepy to trust myself with the merge button.

Make ARMC6 as the default ARM toolchain for MTB targets

a28c6e7

ciarmcom requested review from 0xc0170, cmonr and SeppoTakalo February 26, 2019 16:00

ciarmcom requested a review from a team February 26, 2019 16:00

ciarmcom added the needs: review label Feb 26, 2019

Add right toolchains for version 2 toolchain support check

c6a33af

SenRamakri requested a review from theotherjimmy February 26, 2019 19:22

theotherjimmy approved these changes Feb 26, 2019

View reviewed changes

cmonr added needs: work and removed needs: review labels Feb 27, 2019

SeppoTakalo approved these changes Feb 27, 2019

View reviewed changes

Modify tools to return ARMC5 or ARM for supported toolchain matrix

2b43dc2

theotherjimmy suggested changes Feb 27, 2019

View reviewed changes

deepikabhavnani reviewed Feb 27, 2019

View reviewed changes

tools/build_api.py Show resolved Hide resolved

Fix for review comments and ARMC5 listing added for supported toolcha…

89934d4

…in matrix output

SenRamakri mentioned this pull request Feb 27, 2019

Documentation updates for Toolchain changes ARMmbed/mbed-os-5-docs#945

Merged

theotherjimmy approved these changes Feb 27, 2019

View reviewed changes

alekla01 mentioned this pull request Feb 27, 2019

get_mbed_official_release() mbed5 fix #9878

Closed

SenRamakri added 3 commits February 27, 2019 19:28

Changes to use response files(pass includes in a file)

814eeaf

Fix compiler flags for ARMC6 in sys stats test

880b45c

Use ARMC5 for FVP targets

7265ed7

cmonr approved these changes Feb 28, 2019

View reviewed changes

cmonr added ready for merge risk: A release-version: feature branch and removed needs: work labels Feb 28, 2019

NirSonnenschein merged commit 1751212 into ARMmbed:feature-armc6 Feb 28, 2019

NirSonnenschein removed the ready for merge label Feb 28, 2019

cmonr removed the risk: A label Feb 28, 2019

Make ARMC6 as the default ARM toolchain for MTB targets #9860

Make ARMC6 as the default ARM toolchain for MTB targets #9860

Uh oh!

Conversation

SenRamakri commented Feb 26, 2019

Description

Pull request type

Reviewers

Uh oh!

ciarmcom commented Feb 26, 2019

Uh oh!

0xc0170 commented Feb 26, 2019

Uh oh!

cmonr commented Feb 26, 2019

Uh oh!

SenRamakri commented Feb 26, 2019

Uh oh!

SenRamakri commented Feb 26, 2019

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

cmonr commented Feb 26, 2019

Uh oh!

SenRamakri commented Feb 26, 2019

[OS ERROR] Command: armclang -c --target=arm-arm-none-eabi -mthumb -Os -Wno-armcc-pragma-push-pop -Wno-armcc-pragma-anon-unions -DMULADDC_ ....

Uh oh!

mbed-ci commented Feb 26, 2019

Test run: FAILED

Uh oh!

SenRamakri commented Feb 27, 2019

Uh oh!

cmonr commented Feb 27, 2019

Uh oh!

OPpuolitaival commented Feb 27, 2019

Uh oh!

mbed-ci commented Feb 27, 2019

Test run: FAILED

Uh oh!

theotherjimmy Feb 27, 2019

Choose a reason for hiding this comment

Uh oh!

alekla01 Feb 27, 2019

Choose a reason for hiding this comment

Uh oh!

SenRamakri Feb 28, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mbed-ci commented Feb 27, 2019

Test run: FAILED

Uh oh!

mbed-ci commented Feb 28, 2019

Test run: SUCCESS

Uh oh!

SenRamakri commented Feb 28, 2019

Uh oh!

cmonr left a comment

Choose a reason for hiding this comment

Uh oh!

cmonr commented Feb 28, 2019

Uh oh!

Uh oh!