Skip to content

cmake: optimization levels - #2054

Merged
jajanusz merged 2 commits into
thesofproject:masterfrom
jajanusz:cmake-optimization-levels
Nov 8, 2019
Merged

cmake: optimization levels#2054
jajanusz merged 2 commits into
thesofproject:masterfrom
jajanusz:cmake-optimization-levels

Conversation

@jajanusz

@jajanuszjajanusz commented Nov 5, 2019

Copy link
Copy Markdown
Contributor

Introduces configs for optimization levels.
F.e. Baytrail needs -Os to fit in IRAM when using XCC (probably tried to inline too much with -O2), as shown in #2024

@jajanusz

jajanusz commented Nov 5, 2019

Copy link
Copy Markdown
ContributorAuthor

@lbetlej@lgirdwood
This PR doesn't change default settings (still optimized for performance), but if you'd like to save space on some memory-constrained platforms f.e. on APL with defconfig I got .text smaller by 14432 bytes

@paulstelian97paulstelian97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor nit: What happens if someone pulls this and tries to build without updating the config? (having none of the choices selected). Would the build succeed (with no optimization) or would a make oldconfig or something be required?

Otherwise I understand the motivation behind this. Approved.

@jajanusz

Copy link
Copy Markdown
ContributorAuthor

@paulstelian97 It would have no -O flag at all in this case (I'm not sure if it's -O0 or -O2 then, depends on compiler i guess), It may be better to add catch in CMake file for that, thanks

@jajanuszjajanusz changed the title cmake: optimization levels[WIP] cmake: optimization levelsNov 5, 2019
@jajanusz

Copy link
Copy Markdown
ContributorAuthor

I need to add some smart checks for Kconfig and CMake dependencies to prevent undefined behaviour when new configs are introduced.

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will be useful for debug too.

Comment threadsrc/arch/xtensa/CMakeLists.txt
@xiulipan

Copy link
Copy Markdown
Contributor

@jajanusz So for the build, do we need to make the level to default OPTIMIZE_FOR_SIZE to BYT/CHT XCC build in make_XXXdefconfig. Or we need user to select the option?

@jajanusz

Copy link
Copy Markdown
ContributorAuthor

Can be merged after #2061

@lgirdwood

Copy link
Copy Markdown
Member

@jajanusz So for the build, do we need to make the level to default OPTIMIZE_FOR_SIZE to BYT/CHT XCC build in make_XXXdefconfig. Or we need user to select the option?

Default is speed for all platforms unless size is selected.

@xiulipan

Copy link
Copy Markdown
Contributor

@lgirdwood@jajanusz The problem now is for BYT/CHT XCC build will fail with -O2
So could we make them as default -Os?

Add configs for optimization levels.
These configs should be used by build system to properly
tune compiler flags.
Signed-off-by: Janusz Jankowski <janusz.jankowski@linux.intel.com>
Use optimization configs that come from Kconfig in CMake
to set appropriate compiler flags.
Signed-off-by: Janusz Jankowski <janusz.jankowski@linux.intel.com>
@jajanusz
jajanuszforce-pushed the cmake-optimization-levels branch from 9d21ac6 to a222f96CompareNovember 7, 2019 08:14
@jajanuszjajanusz changed the title [WIP] cmake: optimization levels cmake: optimization levelsNov 7, 2019
@jajanusz

Copy link
Copy Markdown
ContributorAuthor

@lgirdwood@jajanusz The problem now is for BYT/CHT XCC build will fail with -O2
So could we make them as default -Os?

Yes, please

@jajanusz

Copy link
Copy Markdown
ContributorAuthor

SOFCI TEST

@jajanusz

jajanusz commented Nov 7, 2019

Copy link
Copy Markdown
ContributorAuthor

@xiulipan please let me know when you update and if I need to rebase in order to make it work

@xiulipan

Copy link
Copy Markdown
Contributor

@jajanusz I think I will need this PR as base in #2024

I will create some defconfig change for byt/cht xcc build.

@jajanusz
jajanusz merged commit a366d37 into thesofproject:masterNov 8, 2019
@jajanusz

Copy link
Copy Markdown
ContributorAuthor

@xiulipan You can use it now. I guess I'll see in other PRs if BSW works

@xiulipan

Copy link
Copy Markdown
Contributor

@jajanusz Thank!
BSW is a known issue, I will try to send an email to announce that.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jajanusz@xiulipan@lgirdwood@paulstelian97