forked from pytorch/pytorch
-
Notifications
You must be signed in to change notification settings - Fork 62
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
Migration to C++17 #1152
Merged
pruthvistony
merged 7 commits into
rocm5.5_internal_testing
from
rocm5.5_internal_testing_c17
Dec 20, 2022
Merged
Migration to C++17 #1152
pruthvistony
merged 7 commits into
rocm5.5_internal_testing
from
rocm5.5_internal_testing_c17
Dec 20, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Not sure why, but top-level `using namespace` directive causes VC++ fail with (if C++17 standard is used, but everything is fine with C++14): ``` C:\actions-runner\_work\pytorch\pytorch\third_party\pybind11\include\pybind11\detail\../pytypes.h(1520): error C2872: 'attr': ambiguous symbol C:\actions-runner\_work\pytorch\pytorch\aten\src\ATen/core/interned_strings.h(349): note: could be 'c10::attr' C:\actions-runner\_work\pytorch\pytorch\torch/csrc/jit/ir/ir.h(75): note: or 'torch::jit::attr' C:\actions-runner\_work\pytorch\pytorch\cmake\..\third_party\pybind11\include\pybind11/pybind11.h(1094): note: see reference to function template instantiation 'pybind11::str pybind11::str::format<_Ty1&>(_Ty1 &) const' being compiled with [ _Ty1=pybind11::handle ] ``` Solve this by replacing global `using namespace torch::jit;` with specific usages of objects/methods from namespaces Another prep change for https://github.com/pytorch/pytorch/70188 Pull Request resolved: pytorch#90228 Approved by: https://github.com/kit1980, https://github.com/albanD
`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
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/pytorch/pytorch/pull/90174). * __->__ pytorch#90174 fix c10::detail::integer_iterator for C++17 Summary: std::iterator is deprecated. Test Plan: Rely on CI. Pull Request resolved: pytorch#90174 Approved by: https://github.com/clee2000, https://github.com/malfet
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
Set `cmake.dir` to `/usr/local` in `.circleci/scripts/build_android_gradle.sh ` Prep change for raising compiler standard to C++17: cmake-3.18 is the first one to support CUDA17 language Pull Request resolved: pytorch#89570 Approved by: https://github.com/atalman
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
pytorch Build is verified - http://rocm-ci.amd.com/job/mainline-framework-pytorch-ci/555/ |
jeffdaily
approved these changes
Dec 20, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cherry-pick of - pytorch#85969
and dependent patches.
Issue - https://ontrack-internal.amd.com/browse/SWDEV-374101