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

cmake: do not enable LTO on MinGW builds automatically #145

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

andrea-iob
Copy link
Member

LTO doens't work on MinGW.

if (${LTO_SUPPORTED} AND "${CMAKE_BUILD_TYPE}" STREQUAL "Release")
set(ENABLE_LTO TRUE)
else()
set(ENABLE_LTO FALSE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Disabling LTO in mingw is quite hidden to the final User here, LTO_STRATEGY remains untouched, leaving the impression it is effectively used. In my opinion, it is better to force LTO_STRATEGY to disabled in MINGW case e raise a -- message(WARNING "LTO is forced to Disabled in MSYS2/MinGW ") -- just to make clear the concept and to make it simpler to re-enable LTO in the future (once the issue will be faced).

Copy link
Member Author

Choose a reason for hiding this comment

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

The default strategy for LTO is auto and, after these change, it will disable LTO on MinGW. I can update the description of the LTO_STRATEGY in something like "Choose the Link Time Optimization (LTO) strategy, options are: Auto (optimiziation is enabled only in release build and only for configurations that have been tested) Enabled Disabled.".

The fact the LTO doesn't work on MinGW doesn't seem related to bitpit. In my understanding it's a problem/misconfiguration of MinGW. So, if someone wants to enable LTO on MinGW, is free to do so. We can add a warning, stating that MinGW LTO builds will probably don't work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't matter how, what's important for me is that the User is notified of the situation. A warning raise should be enough. I agree to pinpoint an issue to investigate the problem/misconfiguration in MinGW.

@andrea-iob
Copy link
Member Author

I've updated the patch. Since it seems that making LTO work on MinGW requires changes to bitpit, a cmake error is now issued when trying to enable LTO on MinGW.

@andrea-iob andrea-iob merged commit fb955a1 into master Feb 23, 2021
@andrea-iob andrea-iob deleted the cmake.disable.lto.on.mingw branch February 23, 2021 08:28
Sign up for free to 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.

2 participants