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

Enable C++11 in cmake #5325

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Enable C++11 in cmake #5325

wants to merge 1 commit into from

Conversation

ssudholt
Copy link

This PR enables C++11 for compilation of both C++ and CUDA code (see #5262).

As C++11 has been supported by CUDA since 7.0, the minimally required version is bumped as well in the cmake file. In order to not raise warnings, host compiler flag propagation to nvcc is disabled (see https://cmake.org/Bug/view.php?id=15240).

@hgaiser
Copy link
Contributor

hgaiser commented Feb 24, 2017

I agree that c++11 should be enabled, however it is also included in #5294 because it was necessary for move semantics and other c++11 features. Be aware that this PR also includes enabling c++11 but in a slightly different manner. It bumps required cmake version to 3.1, since this works on Ubuntu 14.04 I don't see an issue there.

@ssudholt
Copy link
Author

@hgaiser cmake < 3.1 would allow compilation on Debian 8 with cmake from stable sources. That was pretty much the only reason for me to include it. Wouldn't be opposed to changing it though if this feature is not required. Wasn't aware that #5294 enables C++11 as well.

@de-vri-es
Copy link

Note that #5294 doesn't bump the required version for CUDA to 7.0 and doesn't enable C++11 for nvcc. #5294 can be rebased on this PR if it is merged, but before that C++11 should also be enabled in the old non-cmake build system. Otherwise anything using C++11 features will still fail the CI tests.

It does seem that just dropping the old build system would save everyone a lot of trouble. But I suppose that is a different discussion.

@pkdogcom
Copy link

This commit resolves my issue of adding C++11 dependent layers. Hopefully it will be merged into master soon so that people won't return into the same issue.

@cooperpellaton
Copy link

@de-vri-es Is there any particular benefit to not, by default, enabling "C++11 for nvcc"? I don't see any reason why this would negatively affect the build system or procedure.

@de-vri-es
Copy link

@de-vri-es Is there any particular benefit to not, by default, enabling "C++11 for nvcc"? I don't see any reason why this would negatively affect the build system or procedure.

The main reason would be to support CUDA < 7.0. There doesn't seem to be a pressing need to enable C++11 support in CUDA code right now. However, I'm not against bumping the minimum CUDA version and enabling C++11.

Though the cmake 3.1 path should also have this then:

set(CMAKE_CXX_STANDARD_REQUIRED on)

@PENGUINLIONG
Copy link

constexpr is really helpful making the codes clearer. Although I would love C++11 to be enabled, I still couldn't assert it's a necessity since too much has to be changed, and boost is already there.

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

Successfully merging this pull request may close these issues.

7 participants