Skip to content

[wip] Switch to C++14 #27864

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.

Sign up for GitHub

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

Jump to bottom
Merged
smessmer merged 16 commits into from
Nov 25, 2019
Merged

[wip] Switch to C++14 #27864

smessmer merged 16 commits into from
Nov 25, 2019

Conversation

@smessmer
Copy link
Contributor

@smessmer smessmer commented Oct 14, 2019 •
edited
Loading

Stack from ghstack:

Differential Revision: D17908478

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Oct 14, 2019
@smessmer smessmer mentioned this pull request Oct 14, 2019
Closed
smessmer added a commit that referenced this pull request Oct 14, 2019
@smessmer
[wip] Switch to C++14
c50c8c5 
Pull Request resolved: #27864


ghstack-source-id: 91872594

Differential Revision: [D17908478](https://our.internmc.facebook.com/intern/diff/D17908478/)
soumith
soumith approved these changes Oct 14, 2019
View reviewed changes
Copy link
Contributor

@soumith soumith left a comment

Choose a reason for hiding this comment

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

conditional on CI passing.

@peterjc123 anything to know about C++14 and Windows incompatibility, now is the time to speak :)

smessmer added a commit that referenced this pull request Oct 14, 2019
@smessmer
[wip] Switch to C++14
d64a4f4 
Pull Request resolved: #27864


ghstack-source-id: 91896396

Differential Revision: [D17908478](https://our.internmc.facebook.com/intern/diff/D17908478/)
@peterjc123
Copy link
Collaborator

peterjc123 commented Oct 15, 2019

@soumith According to this table, all the C++ 14 features are available in VS 2017. /std:c++14 is automatically enabled in MSVC 2017, so there is actually nothing we should do to add C++ 14 support. More details could be found here.

@smessmer smessmer mentioned this pull request Oct 16, 2019
Closed
bddppq
bddppq reviewed Oct 16, 2019
View reviewed changes
cmake/Dependencies.cmake Outdated
list(APPEND HIP_CXX_FLAGS -Wno-duplicate-decl-specifier)
list(APPEND HIP_CXX_FLAGS -DCAFFE2_USE_MIOPEN)
list(APPEND HIP_CXX_FLAGS -DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_HIP)
list(APPEND HIP_CXX_FLAGS -std=gnu++14)
Copy link
Contributor

@bddppq bddppq Oct 16, 2019

Choose a reason for hiding this comment

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

-std=gnu++14 -> -std=c++14, hcc doesn't support gnu extensions.

smessmer added a commit that referenced this pull request Oct 17, 2019
@smessmer
[wip] Switch to C++14
4d34067 
Pull Request resolved: #27864


ghstack-source-id: 92081119

Differential Revision: [D17908478](https://our.internmc.facebook.com/intern/diff/D17908478/)
@smessmer smessmer mentioned this pull request Oct 17, 2019
Closed
smessmer added a commit that referenced this pull request Oct 17, 2019
@smessmer
[wip] Switch to C++14
c359852 
Pull Request resolved: #27864


ghstack-source-id: 92086808

Differential Revision: [D17908478](https://our.internmc.facebook.com/intern/diff/D17908478/)
smessmer added a commit that referenced this pull request Oct 17, 2019
@smessmer
[wip] Switch to C++14
34a62d2 
Pull Request resolved: #27864


ghstack-source-id: 92099795

Differential Revision: [D17908478](https://our.internmc.facebook.com/intern/diff/D17908478/)
This was referenced Oct 22, 2019
Closed
Closed
Closed
Closed
@smessmer smessmer mentioned this pull request Oct 22, 2019
Closed
This was referenced Oct 25, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@mrshenli mrshenli mentioned this pull request Nov 7, 2019
Closed
@smessmer smessmer mentioned this pull request Nov 25, 2019
Closed
@smessmer smessmer merged commit 5fed937 into gh/smessmer/72/base Nov 25, 2019
smessmer added a commit that referenced this pull request Nov 25, 2019
@smessmer
[wip] Switch to C++14
63e8869 
Pull Request resolved: #27864


ghstack-source-id: 94519851

Differential Revision: [D17908478](https://our.internmc.facebook.com/intern/diff/D17908478/)
@smessmer
Copy link
Contributor Author

smessmer commented Nov 25, 2019

This is not merged yet, there was an error in the export. Re-created this PR in #30406.

@smessmer smessmer mentioned this pull request Nov 25, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apaszke apaszke Awaiting requested review from apaszke

@ezyang ezyang Awaiting requested review from ezyang

@fmassa fmassa Awaiting requested review from fmassa

@goldsborough goldsborough Awaiting requested review from goldsborough

@ebetica ebetica Awaiting requested review from ebetica

@mrshenli mrshenli Awaiting requested review from mrshenli

@pietern pietern Awaiting requested review from pietern

@yf225 yf225 Awaiting requested review from yf225

@zhaojuanmao zhaojuanmao Awaiting requested review from zhaojuanmao

2 more reviewers

@bddppq bddppq bddppq left review comments

@soumith soumith soumith approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@smessmer @peterjc123 @soumith @bddppq @facebook-github-bot