Migrate PyTorch to C++17 #85969

malfet · 2022-09-30T14:27:19Z

With CUDA-10.2 gone we can finally do it!

This PR mostly contains build system related changes, invasive functional ones are to be followed.
Among many expected tweaks to the build system, here are few unexpected ones:

Force onnx_proto project to be updated to C++17 to avoid duplicate symbols error when compiled by gcc-7.5.0, as storage rule for constexpr changed in C++17, but gcc does not seem to follow it
Do not use std::apply on CUDA but rely on the built-in variant, as it results in test failures when CUDA runtime picks host rather than device function when std::apply is invoked from CUDA code.
std::decay_t -> ::std::decay_t and std::move->::std::move as VC++ for some reason claims that std symbol is ambigious
Disable use of std::aligned_alloc on Android, as its libc++ does not implement it.

Some prerequisites:

Fixes #56055

cc @VitalyFedyunin @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @EikanWang @mlazos @soumith @voznesenskym @yanboliang @penguinwu @anijain2305 @Guobing-Chen @chunyuan-w @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @desertfire

pytorch-bot · 2022-09-30T14:27:22Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/85969

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Failures

As of commit eaf499f:

The following jobs have failed:

cuda11.6-py3.10-gcc7-sm86 / test (inductor_torchbench, 1, 1, linux.g5.4xlarge.nvidia.gpu)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

atalman

LGTM. Can you please include testing plan ? Do we need any followup PR's in builder for this ?

malfet · 2022-09-30T15:02:20Z

@atalman no builder PRs should be necessary...

ezyang

IT'S HAPPENING

as a follow up, should get rid of at::optional for std::optional

r-barnes · 2022-09-30T16:10:49Z

Yay!

kulinseth

mps side changes look fine

facebook-github-bot · 2022-10-03T23:03:10Z

/easycla

As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details.

This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign.

mantaionut · 2022-10-19T13:12:28Z

I made some fixes for the windows build. Feel free to cherry-pick those changes #87200.

pytorchmergebot · 2022-12-08T00:34:46Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2022-12-08T01:55:46Z

Merge failed

Reason: 1 additional jobs have failed, first few of them are: inductor

Details for Dev Infra team

Raised by workflow job

malfet · 2022-12-08T02:00:35Z

@pytorchbot merge

pytorchmergebot · 2022-12-08T02:02:43Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

malfet · 2022-12-08T02:25:48Z

@pytorchbot merge -f "All tests but inductor_torchbench are passing and latter has been broken for a while"

pytorchmergebot · 2022-12-08T02:26:08Z

The merge job was canceled. If you believe this is a mistake,then you can re trigger it through pytorch-bot.

pytorchmergebot · 2022-12-08T02:27:43Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

Splitting out various smaller pieces from pytorch#85969 Pull Request resolved: pytorch#89297 Approved by: https://github.com/huydhn

Prep change for moving the codebase to C++17 standard Was part of pytorch#85969 Pull Request resolved: pytorch#89605 Approved by: https://github.com/weiwangmeta, https://github.com/kit1980

`register` keyword is removed in C++17, but keeping it there under ifdef as I have not measured the perf implication on older compiler, though there shouldn't be any: all modern compilers supposed to downright ignore it. This code originates from facebookresearch/xformers#375 will propose similar PR to remove register keyword usage to that repo. Yet another thing discovered while working on pytorch#85969 Pull Request resolved: pytorch#90389 Approved by: https://github.com/drisspg

Get rid of std::iterator inheritance/references for `c10::DictIterator`, `c10::IListRefIterator` and `c10::ListIterator` Followup after pytorch#90174 Fixes deprecation warning and extension compilation failures using VC++ that raises following errors: ``` C:\actions-runner\_work\pytorch\pytorch\build\win_tmp\build\torch\include\ATen/core/IListRef.h(517): error C4996: 'std::iterator<std::bidirectional_iterator_tag,T,ptrdiff_t,T *,T &>::value_type': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning. C:\actions-runner\_work\pytorch\pytorch\build\win_tmp\build\torch\include\ATen/core/List.h(169): error C4996: 'std::iterator<std::random_access_iterator_tag,T,ptrdiff_t,T *,T &>::difference_type': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning. ``` Discovered while working on pytorch#85969 Pull Request resolved: pytorch#90379 Approved by: https://github.com/ezyang, https://github.com/dagitses

With CUDA-10.2 gone we can finally do it! This PR mostly contains build system related changes, invasive functional ones are to be followed. Among many expected tweaks to the build system, here are few unexpected ones: - Force onnx_proto project to be updated to C++17 to avoid `duplicate symbols` error when compiled by gcc-7.5.0, as storage rule for `constexpr` changed in C++17, but gcc does not seem to follow it - Do not use `std::apply` on CUDA but rely on the built-in variant, as it results in test failures when CUDA runtime picks host rather than device function when `std::apply` is invoked from CUDA code. - `std::decay_t` -> `::std::decay_t` and `std::move`->`::std::move` as VC++ for some reason claims that `std` symbol is ambigious - Disable use of `std::aligned_alloc` on Android, as its `libc++` does not implement it. Some prerequisites: - pytorch#89297 - pytorch#89605 - pytorch#90228 - pytorch#90389 - pytorch#90379 - pytorch#89570 - facebookincubator/gloo#336 - facebookincubator/gloo#343 - pytorch/builder@919676f Fixes pytorch#56055 Pull Request resolved: pytorch#85969 Approved by: https://github.com/ezyang, https://github.com/kulinseth

With C++17 these tests are not failing Fixes #25161 Depends on #85969 Pull Request resolved: #87284 Approved by: https://github.com/soulitzer

Prep change for moving the codebase to C++17 standard Was part of pytorch#85969 Pull Request resolved: pytorch#89605 Approved by: https://github.com/weiwangmeta, https://github.com/kit1980

`register` keyword is removed in C++17, but keeping it there under ifdef as I have not measured the perf implication on older compiler, though there shouldn't be any: all modern compilers supposed to downright ignore it. This code originates from facebookresearch/xformers#375 will propose similar PR to remove register keyword usage to that repo. Yet another thing discovered while working on pytorch#85969 Pull Request resolved: pytorch#90389 Approved by: https://github.com/drisspg

Get rid of std::iterator inheritance/references for `c10::DictIterator`, `c10::IListRefIterator` and `c10::ListIterator` Followup after pytorch#90174 Fixes deprecation warning and extension compilation failures using VC++ that raises following errors: ``` C:\actions-runner\_work\pytorch\pytorch\build\win_tmp\build\torch\include\ATen/core/IListRef.h(517): error C4996: 'std::iterator<std::bidirectional_iterator_tag,T,ptrdiff_t,T *,T &>::value_type': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning. C:\actions-runner\_work\pytorch\pytorch\build\win_tmp\build\torch\include\ATen/core/List.h(169): error C4996: 'std::iterator<std::random_access_iterator_tag,T,ptrdiff_t,T *,T &>::difference_type': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning. ``` Discovered while working on pytorch#85969 Pull Request resolved: pytorch#90379 Approved by: https://github.com/ezyang, https://github.com/dagitses

With CUDA-10.2 gone we can finally do it! This PR mostly contains build system related changes, invasive functional ones are to be followed. Among many expected tweaks to the build system, here are few unexpected ones: - Force onnx_proto project to be updated to C++17 to avoid `duplicate symbols` error when compiled by gcc-7.5.0, as storage rule for `constexpr` changed in C++17, but gcc does not seem to follow it - Do not use `std::apply` on CUDA but rely on the built-in variant, as it results in test failures when CUDA runtime picks host rather than device function when `std::apply` is invoked from CUDA code. - `std::decay_t` -> `::std::decay_t` and `std::move`->`::std::move` as VC++ for some reason claims that `std` symbol is ambigious - Disable use of `std::aligned_alloc` on Android, as its `libc++` does not implement it. Some prerequisites: - pytorch#89297 - pytorch#89605 - pytorch#90228 - pytorch#90389 - pytorch#90379 - pytorch#89570 - facebookincubator/gloo#336 - facebookincubator/gloo#343 - pytorch/builder@919676f Fixes pytorch#56055 Pull Request resolved: pytorch#85969 Approved by: https://github.com/ezyang, https://github.com/kulinseth

Prep change for moving the codebase to C++17 standard Was part of pytorch#85969 Pull Request resolved: pytorch#89605 Approved by: https://github.com/weiwangmeta, https://github.com/kit1980

Get rid of std::iterator inheritance/references for `c10::DictIterator`, `c10::IListRefIterator` and `c10::ListIterator` Followup after pytorch#90174 Fixes deprecation warning and extension compilation failures using VC++ that raises following errors: ``` C:\actions-runner\_work\pytorch\pytorch\build\win_tmp\build\torch\include\ATen/core/IListRef.h(517): error C4996: 'std::iterator<std::bidirectional_iterator_tag,T,ptrdiff_t,T *,T &>::value_type': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning. C:\actions-runner\_work\pytorch\pytorch\build\win_tmp\build\torch\include\ATen/core/List.h(169): error C4996: 'std::iterator<std::random_access_iterator_tag,T,ptrdiff_t,T *,T &>::difference_type': warning STL4015: The std::iterator class template (used as a base class to provide typedefs) is deprecated in C++17. (The <iterator> header is NOT deprecated.) The C++ Standard has never required user-defined iterators to derive from std::iterator. To fix this warning, stop deriving from std::iterator and start providing publicly accessible typedefs named iterator_category, value_type, difference_type, pointer, and reference. Note that value_type is required to be non-const, even for constant iterators. You can define _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning. ``` Discovered while working on pytorch#85969 Pull Request resolved: pytorch#90379 Approved by: https://github.com/ezyang, https://github.com/dagitses

With CUDA-10.2 gone we can finally do it! This PR mostly contains build system related changes, invasive functional ones are to be followed. Among many expected tweaks to the build system, here are few unexpected ones: - Force onnx_proto project to be updated to C++17 to avoid `duplicate symbols` error when compiled by gcc-7.5.0, as storage rule for `constexpr` changed in C++17, but gcc does not seem to follow it - Do not use `std::apply` on CUDA but rely on the built-in variant, as it results in test failures when CUDA runtime picks host rather than device function when `std::apply` is invoked from CUDA code. - `std::decay_t` -> `::std::decay_t` and `std::move`->`::std::move` as VC++ for some reason claims that `std` symbol is ambigious - Disable use of `std::aligned_alloc` on Android, as its `libc++` does not implement it. Some prerequisites: - pytorch#89297 - pytorch#89605 - pytorch#90228 - pytorch#90389 - pytorch#90379 - pytorch#89570 - facebookincubator/gloo#336 - facebookincubator/gloo#343 - pytorch/builder@919676f Fixes pytorch#56055 Pull Request resolved: pytorch#85969 Approved by: https://github.com/ezyang, https://github.com/kulinseth

升级 c++ 标准到 17，PyTorch 已经升级了 pytorch/pytorch#85969 同时废弃对 cuda 10 的支持 ----- 改动： * 升级 json 库修复对 `<filesystem>` 头文件的检测错误 * op_schema_emitter.cpp 156 行 sourceIncludeFilename 改为 sourceIncludeFilename.getValue()，修复 c++17 引入 filesystem 后 sourceIncludeFilename 类型为 `cl::opt<std::string>` 而引起的 ambiguous 调用 * 删掉 cplusplus_17.cpp/.h 文件 * 在 CI 中改为用 gcc9 而不是 gcc7 编译 oneflow，停止支持和 c++17 不兼容的 cuda 10.2 ----- C++17 主要特性： * 结构化绑定用于返回多值，相比传入指针或引用更清晰简洁： ```c++ std::pair<int, int> f() { return {1, 2}; } auto [a, b] = f(); // a=1, b=2 ``` * 使用 if constexpr 可以去掉很多 sfinae * 提供 `std::***_v` 代替 `std::***::value` * 自带 std::variant 和 std::optional --------- Signed-off-by: daquexian <daquexian566@gmail.com> Co-authored-by: oneflow-ci-bot <ci-bot@oneflow.org> Co-authored-by: Shenghang Tsai <jackalcooper@gmail.com>

Summary: [This PR](pytorch#85969) migrated PyTorch to C++17. This removes some old C++14 stuff. - If you approve of this diff, please use the "Accept & Ship" button :-) (1 files modified.) Test Plan: Sandcastle Differential Revision: D43006625 fbshipit-source-id: 15174474de78ae36adae2dfd0c1a7ab5c8ce465b

facebook-github-bot added the cla signed label Sep 30, 2022

malfet requested review from albanD, ezyang and a team September 30, 2022 14:27

atalman reviewed Sep 30, 2022

View reviewed changes

malfet force-pushed the malfet/make-pytorch-c++17-project branch from 23cb59f to 917bd45 Compare September 30, 2022 14:57

malfet requested a review from kulinseth as a code owner September 30, 2022 14:57

ezyang approved these changes Sep 30, 2022

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 30, 2022

malfet force-pushed the malfet/make-pytorch-c++17-project branch 4 times, most recently from 11dea83 to 6a78b1f Compare September 30, 2022 23:53

kulinseth reviewed Oct 1, 2022

View reviewed changes

kulinseth approved these changes Oct 1, 2022

View reviewed changes

pytorch-bot bot added ciflow/mps Run MPS tests (subset of trunk) release notes: mps Release notes category labels Oct 1, 2022

malfet force-pushed the malfet/make-pytorch-c++17-project branch from 6a78b1f to 5ae16e2 Compare October 1, 2022 15:21

malfet requested a review from jeffdaily as a code owner October 1, 2022 15:21

malfet requested review from fmassa and soumith as code owners October 1, 2022 19:00

malfet force-pushed the malfet/make-pytorch-c++17-project branch from c516937 to 14014df Compare October 12, 2022 16:07

swolchok mentioned this pull request Oct 17, 2022

[PyTorch] Make c10::irange(x) generate the same assembly as for loop #86841

Closed

mantaionut mentioned this pull request Oct 19, 2022

Re-enabled 2 Metaprogramming tests on Windows #87284

Closed

malfet force-pushed the malfet/make-pytorch-c++17-project branch from 14014df to 0d75095 Compare October 26, 2022 18:03

pytorchmergebot added the Merged label Dec 8, 2022

pytorchmergebot closed this in 36ac095 Dec 8, 2022

kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022

Move bazel to c++17 (pytorch#89297)

40d2c05

Splitting out various smaller pieces from pytorch#85969 Pull Request resolved: pytorch#89297 Approved by: https://github.com/huydhn

pytorchmergebot pushed a commit that referenced this pull request Dec 13, 2022

Re-enabled 2 Metaprogramming tests on Windows (#87284)

7498e23

With C++17 these tests are not failing Fixes #25161 Depends on #85969 Pull Request resolved: #87284 Approved by: https://github.com/soulitzer

daquexian mentioned this pull request Dec 20, 2022

bump to cpp17 Oneflow-Inc/oneflow#9632

Merged

pruthvistony mentioned this pull request Dec 20, 2022

Migration to C++17 ROCm/pytorch#1152

Merged

r-barnes mentioned this pull request Feb 24, 2023

PyTorch: Switch C++14 to C++17 in caffe2/c10/util/string_view.h #95440

Closed

justinchuby mentioned this pull request Jan 1, 2024

Build fails with USE_SYSTEM_ONNX=OFF: C++17 features are used while PyTorch is building with the C++14 option #116422

Closed

malfet deleted the malfet/make-pytorch-c++17-project branch January 20, 2024 03:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate PyTorch to C++17 #85969

Migrate PyTorch to C++17 #85969

malfet commented Sep 30, 2022 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Sep 30, 2022 •

edited

Loading

atalman left a comment

malfet commented Sep 30, 2022

ezyang left a comment

r-barnes commented Sep 30, 2022

kulinseth left a comment

facebook-github-bot commented Oct 3, 2022

mantaionut commented Oct 19, 2022

pytorchmergebot commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

malfet commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

malfet commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

Migrate PyTorch to C++17 #85969

Migrate PyTorch to C++17 #85969

Conversation

malfet commented Sep 30, 2022 • edited by pytorch-bot bot Loading

pytorch-bot bot commented Sep 30, 2022 • edited Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/85969

❌ 1 Failures

atalman left a comment

Choose a reason for hiding this comment

malfet commented Sep 30, 2022

ezyang left a comment

Choose a reason for hiding this comment

r-barnes commented Sep 30, 2022

kulinseth left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Oct 3, 2022

mantaionut commented Oct 19, 2022

pytorchmergebot commented Dec 8, 2022

Merge started

pytorchmergebot commented Dec 8, 2022

Merge failed

malfet commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

Merge started

malfet commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

Merge started

malfet commented Sep 30, 2022 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Sep 30, 2022 •

edited

Loading