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

Change default optimisation level to O1 when exporting to uVision #11212

Merged
merged 1 commit into from Aug 20, 2019

Conversation

@chrissnow
Copy link
Contributor

commented Aug 13, 2019

Description

When exporting to uVision the default level is O0 which is wasteful as dead code and variables are included. O1 removes this without causing debugging issues.

From
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0472k/chr1359124221739.html

Although the debug view produced by -O0 corresponds most closely to the source code, users might prefer the debug view produced by -O1 because this improves the quality of the code without changing the fundamental structure.

Fixes #11068

Pull request type

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

Reviewers

Release Notes

Default uVision optimisation level changed to O1. It matches the debug profile.

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Aug 13, 2019
@ciarmcom

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

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

Copy link
Member

left a comment

Before I approve, I would like to make sure the other exporters are aligned or why they shouldn't !

@chrissnow

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2019

I agree that it would be nice that they are all aligned,I think Os is used in some exports, O1 in others.
uVision has limited options,
image

Ideally the export should match debug profile which it now does?

"common": ["-c", "--target=arm-arm-none-eabi", "-mthumb", "-g", "-O1",

It would be possible to output uVision projects that include all 3 profiles as different project targets
http://www.keil.com/support/man/docs/uv4/uv4_ca_projtargfilegr.htm

Or alternatively pass the profiles optimisation level to the exporter.

Sadly both are a bit beyond the time I can commit to this.

Copy link
Member

left a comment

Matching debug profile

I've checked IAR it's set to High balanced, but that shall be fixed separately

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

CI started

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Aug 19, 2019
@kjbracey-arm

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

How do exporters and profiles interact? Are the exporters supposed to export according to a specified profile, or create an export with all profiles?

I think it's got to be one of the above, and either way, it would be preferable to pick the optimisation level up from the profile, just like the language selection. I was messing with that mapping in #11225; I'm not familiar with how all the plumbing hangs together, but presumably that concept could easily be copied over for the optimisation and debug options.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

My initial expectation were the same but I checked some and could not find optimization being exported.

They contain "non valid flag" checks:

IAR implementation:

        # Flag invalid if set in template
        # Optimizations are also set in template
        invalid_flag = lambda x: x in template or re.match("-O(\d|time|n|hz?)", x)

uvision


        def valid_flag(x):
            return (
                x not in in_template and
                not x.startswith("-O") and
                not x.startswith("-std") and
                not x.startswith("-D")
            )

This means exporters are using predefined optimization level in templates and even ignores some other flags as we can see.

I think it's got to be one of the above, and either way, it would be preferable to pick the optimisation level up from the profile, just like the language selection. I was messing with that mapping in #11225; I'm not familiar with how all the plumbing hangs together, but presumably that concept could easily be copied over for the optimisation and debug options.

Ideally yes (exporting profiles is not that simple but if we can use the profile then it would be as simple as export debug or export release. Having multiple profiles is supported by many tools, but might require bit more work to refactor templates). It might be worth creating a new ticket for this (to find out why there are these non valid options in each exporter).

cc @ARMmbed/mbed-os-tools

@mbed-ci

This comment has been minimized.

Copy link

commented Aug 19, 2019

Test run: SUCCESS

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

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Aug 20, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 20, 2019

I'll merge this as it aligns with the default profile and create a new issue for profiles not being exported

@0xc0170 0xc0170 merged commit b2b1ac1 into ARMmbed:master Aug 20, 2019
25 checks passed
25 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR 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-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8647 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8464B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
@0xc0170

This comment has been minimized.

Copy link
Member

commented Aug 20, 2019

created #11263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.