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 warning about INTERPROCEDURAL_OPTIMIZATION from vcpkg.cmake #2355

Closed
patrikhuber opened this issue Aug 1, 2020 · 9 comments · Fixed by #2370
Closed

CMake warning about INTERPROCEDURAL_OPTIMIZATION from vcpkg.cmake #2355

patrikhuber opened this issue Aug 1, 2020 · 9 comments · Fixed by #2370
Assignees

Comments

@patrikhuber
Copy link
Contributor

patrikhuber commented Aug 1, 2020

Issue description

Hi,

I'm using the latest pybind11 master commit (as of today) - as a submodule, not from vcpkg. I'm including that pybind11 commit in one of my projects. That project happens to have a few dependencies, and I'm using vcpkg for those. When clicking "Generate" in the CMake GUI, I get the following warning:

pybind11 v2.6.0 dev
Configuring done
CMake Warning (dev) at C:/Users/User/vcpkg/scripts/buildsystems/vcpkg.cmake:362 (_add_library):
  Policy CMP0069 is not set: INTERPROCEDURAL_OPTIMIZATION is enforced when
  enabled.  Run "cmake --help-policy CMP0069" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.

  INTERPROCEDURAL_OPTIMIZATION property will be ignored for target
  'python-bindings'.
Call Stack (most recent call first):
  3rdparty/pybind11/tools/pybind11Tools.cmake:150 (add_library)
  python/CMakeLists.txt:1 (pybind11_add_module)
This warning is for project developers.  Use -Wno-dev to suppress it.

Generating done

It's probably safe to ignore it but a red message during the "build" stage is always unsatisfactory. When I enable PYBIND11_CLASSIC_LTO, the warning disappears.
I'm using cmake-3.18.0.

I don't fully understand the warning - is this something vcpkg have to fix in their vcpkg/scripts/buildsystems/vcpkg.cmake file? If so, I could open an issue there, but I wanted to check here first.

@henryiii
Copy link
Collaborator

henryiii commented Aug 1, 2020

What is your CMake policy setting? Can you try setting something like this in your CMake file:

cmake_minimum_required(VERSION 3.7...3.18)

(Second number just needs to be equal to or larger than 3.9). If that's the case, I can detect the policy setting before changing to the new LTO setting - I should be doing that anyway. Even if you are running CMake 3.18, if you have a policy setting less than 3.9, this won't enable LTO. CMake's gymnastics to be backward compatible are really irritating sometimes.

There is a another possibility, where a target is created by a function in a file that has an old policy setting - if that's the case, the file needs to be updated. I didn't see a policy setting immediately in that file, but I could have missed it. I'm working on a similar problem with CMake's own modules (!) here: https://gitlab.kitware.com/cmake/cmake/-/issues/21042 (The new FindPython sets a policy level of 3.7, which makes no sense, as it was introduced in 3.12)

I might even be able to detect it from the target (which would be ideal), but I'm not sure that detail is exposed in the API, I only have found it so far in the C++ CMake source.

henryiii added a commit that referenced this issue Aug 2, 2020
@henryiii
Copy link
Collaborator

henryiii commented Aug 2, 2020

@patrikhuber, can you try the branch in #2347 ? I'm not totally sure why this is happening - by the same logic causing the FindPython issue, this should be fine, since we are setting the policies in the tool file where the function is defined (not sure if that's ideal, actually, I did it because the old file did it without thinking a great deal about it, but FindPython does it too (and that has caused a bug)). I have a few other ideas to test if this doesn't work (but this is the one with the least manual intervention, and I haven't used vcpkg before).

@patrikhuber
Copy link
Contributor Author

What is your CMake policy setting?

I am not setting any policies anywhere, so these should all be at their default values. (vcpkg.cmake might set some, I haven't checked that.)

Your comment about cmake 3.9 got me thinking. At the very top of my CMakeLists, I have the following construct:

cmake_minimum_required(VERSION 3.8.2)
if(MSVC)
  cmake_minimum_required(VERSION 3.10.0) # needed for CMAKE_CXX_STANDARD 17 on >=VS2017.3
endif()

Maybe that's not a correct way of using those directives? If I comment out cmake_minimum_required(VERSION 3.8.2) and just unconditionally use cmake_minimum_required(VERSION 3.10.0), then I don't get any of the INTERPROCEDURAL_OPTIMIZATION warnings. Perhaps (assuming MSVC), the first call sets the policy level to 3.8.2 and then the second call to cmake_minimum_required does not overwrite this with the values for 3.10.0?

Shall I still try your branch (happy to!) or did I uncover the reason and I shouldn't be using the construct I'm using above?

@henryiii
Copy link
Collaborator

henryiii commented Aug 2, 2020

I am not setting any policies anywhere

Yes, you are. The first call to cmake_minimum_required sets the policies to that level. So you are setting the policies to 3.8. You never raise those policies, so you don't get the benefits of a newer CMake, even if you use it. It's generally a good idea to raise the polices to the latest version you test on (3.12 added a way to do that inline, cmake_minimum_required(VERSION 3.8.2...3.18)). See this section for a more general version and caveats.

Without making any changes, can you try my branch? I'm curious to see if that makes a difference.


I would at least change this line to:

cmake_minimum_required(VERSION 3.8.2)

# Optional:
if(MSVC AND CMAKE_VERSION VERSION_LESS 3.10)
  message(FATAL_ERROR "Must have CMake 3.10+ on Windows")
endif()

And then add something like:

if(${CMAKE_VERSION} VERSION_LESS 3.18)
    cmake_policy(VERSION ${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION})
else()
    cmake_policy(VERSION 3.18)
endif()

This will also allow 3.10+ users to set CXX 17. As long as add every other policy improvement and fix in CMake's after 3.8.

Or, ignoring a bug in some built-in MSVC CMake's:

cmake_minimum_required(VERSION 3.8.2...3.18)
if(MSVC AND CMAKE_VERSION VERSION_LESS 3.10)
  message(FATAL_ERROR "Must have CMake 3.10+ on Windows")
endif()

@patrikhuber
Copy link
Contributor Author

patrikhuber commented Aug 4, 2020

Yes, you are. The first call to cmake_minimum_required sets the policies to that level.

I see! That's what I suspected I guess. What I originally meant was "I am not setting any policies explicitly, like with cmake_policy(...)". But yes, completely understand that cmake_minimum_required(...) does set policies.

You never raise those policies, so you don't get the benefits of a newer CMake, even if you use it.

Thanks a lot for this comment and also your suggestions. I was not aware of this!

Without making any changes, can you try my branch? I'm curious to see if that makes a difference.

I just tried that branch, without any changes, i.e. with the original if(MSVC).... It results in the same warning as in the original post.

@henryiii
Copy link
Collaborator

henryiii commented Aug 4, 2020

I was afraid of that. I may have to make the new LTO behavior opt-in due to this terrible policy. It's also breaking the new FindPython work, due to CMake itself locking the policies to 3.7 accidentally in FindPython. I could base it on CMAKE_INTERPROCEDURAL_OPTIMIZATION being defined (even to OFF), which would provide a very reasonable way to opt in/out.

@patrikhuber
Copy link
Contributor Author

patrikhuber commented Aug 4, 2020

Ouch! I hope you find a good solution for it. I would say you can just ignore my use case, as I know now that what I'm doing is not 100% ideal anyway. Unfortunately I can see you're fighting with a similar problem with FindPython. Well hopefully at least for future cmake versions, things can be better :)

Feel free to close this at your own discretion, and thank you very much again for all your advice!

Also I look forward to reading more about the new LTO stuff once it's officially released.

@wxmerkt
Copy link

wxmerkt commented Aug 11, 2020

Hi,
We have run into a similar issue since upgrading to the latest master. In particular, we have to set cmake_minimum_required to the lowest version based on CMake features we require due to supporting multiple distros (e.g. back to Ubuntu 16.04 which ships with 3.5)---as thus it's set to 3.0.2. Code that compiled fine before now throws on 18.04 (CMake 3.10.2) with:

CMake Error at /usr/share/cmake-3.10/Modules/CheckIPOSupported.cmake:138 (message):
  Policy CMP0069 is not set
Call Stack (most recent call first):
  [pybind11-share]/cmake/pybind11Tools.cmake:189 (check_ipo_supported)

Setting the policy to NEW then results in linking errors for finding Python (Python 2.7 in this context). I did not check the latest release notes but perhaps 2.7 has been deprecated since v2.5.0.

We downgraded to v2.5.0 to circumvent these two issues. Very interested as well in learning more about a possible work-around or the new features.

Thanks,
Wolfgang

@henryiii
Copy link
Collaborator

henryiii commented Aug 11, 2020

This should be fixed in #2370 - the IPO selection was made completely optional rather than trying to activate based on CMake version (which was a bad idea). What are your linking errors?

My general recommendation is to set a range of CMake versions, rather than just a minimum. Also, 3.1 is a much better minimum than 3.0. The minimum version of CMake require by PyBind11 has been increased to 3.7 - though config mode may handle earlier versions.

Python 2.7 is still supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants