Update C++ standard to C++17 #12337
Update C++ standard to C++17 #12337
Conversation
|
LGTM! @kparzysz-quic do you mind also post an announcement thread? So people are aware |
Sure, I'll just need to resolve the build failures. I'll post the announcement once the PR is ready to merge. |
.github/workflows/main.yml
Outdated
| @@ -36,14 +36,16 @@ concurrency: | |||
|
|
|||
| jobs: | |||
| MacOS: | |||
| if: ${{ github.repository == 'apache/tvm' }} | |||
| if: ${{ github.repository == 'kparzysz-quic/tvm' }} | |||
There was a problem hiding this comment.
Should we keep this change local? Otherwise LGTM!
There was a problem hiding this comment.
Yes, I'll remove it. I'm trying to figure out the macOS build failure. I'll clean this PR up before merging.
LLVM has switched to C++17 in its development branch. Follow suit to be able to compile LLVM headers.
|
Great news! Just need to carefully check the installation documents as well. For example, update "C++ 14" to "C++ 17" in https://github.com/apache/tvm/blob/fbb7b5d1a0d82acb1f581dd2ec362b4dcad2638e/docs/install/from_source.rst In the meantime, it might be better to specify the required minimal compiler versions as the coverage to C++17 standard varies a lot in different compiler versions. For example, if we just want to pass the "-std=c++17" flag, then the required minimals can be:
However, those initial version might poorly support some useful library features like Since the motivation is to compiler newer versions of LLVM, maybe we can just follow LLVM's compiler constraints.
|
Something seems to be wrong with the llvmdev 10.0.0 packages, since the LLVM unit test fails on Windows. It works fine when LLVM 10 is compiled from sources.
… to 10.0" This reverts commit 5c05627. This is no longer needed.
|
I would love to second @ganler that we might want to provide clearer instruction on TVM official website to make TVM more approachable to new contributors. @kparzysz-quic @tqchen would either of you mind making such a change accordingly? I'm happy to submit a PR too if you guys prefer |
|
I created a #12405 with docs update. Let me know if you want to have more information in it. |
With C++17 enabled in apache#12337, using structured bindings to replace cases where `std::tie` is used to define local variables.
* [Refactor] Replace std::tie with structured bindings With C++17 enabled in #12337, using structured bindings to replace cases where `std::tie` is used to define local variables. * Added missing header for <optional> * Silenced unused variable warnings after structured bindings This is a bug in gcc version 7, resolved in gcc 8. While gcc version 7 is used for CI, we'll need to silence unused variable warnings resulting from using only part of a structured binding. More information: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81767
As a follow-up to apache#12337, updating the EMCC flags from `-std=c++14` to `-std=c++17`.
As a follow-up to #12337, updating the EMCC flags from `-std=c++14` to `-std=c++17`.
Follow-up from apache#12337 and apache#12693, a few additional locations that specify C++14.
* Update C++ standard to C++17 LLVM has switched to C++17 in its development branch. Follow suit to be able to compile LLVM headers. * Clang 8.0+ also supports -faligned-new * Make make verbose * Use CMAKE_OSX_DEPLOYMENT_TARGET to set minimum macOS version (10.12) * Update llvmdev version in conda to >= 11 Something seems to be wrong with the llvmdev 10.0.0 packages, since the LLVM unit test fails on Windows. It works fine when LLVM 10 is compiled from sources.
* [Refactor] Replace std::tie with structured bindings With C++17 enabled in apache#12337, using structured bindings to replace cases where `std::tie` is used to define local variables. * Added missing header for <optional> * Silenced unused variable warnings after structured bindings This is a bug in gcc version 7, resolved in gcc 8. While gcc version 7 is used for CI, we'll need to silence unused variable warnings resulting from using only part of a structured binding. More information: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81767
As a follow-up to apache#12337, updating the EMCC flags from `-std=c++14` to `-std=c++17`.
Follow-up from apache#12337 and apache#12693, updating a few additional locations that specified C++14.
* Update C++ standard to C++17 LLVM has switched to C++17 in its development branch. Follow suit to be able to compile LLVM headers. * Clang 8.0+ also supports -faligned-new * Make make verbose * Use CMAKE_OSX_DEPLOYMENT_TARGET to set minimum macOS version (10.12) * Update llvmdev version in conda to >= 11 Something seems to be wrong with the llvmdev 10.0.0 packages, since the LLVM unit test fails on Windows. It works fine when LLVM 10 is compiled from sources.
LLVM has switched to C++17 in its development branch. Follow suit to be able to compile LLVM headers.