There was an RFC thread on the LLVM forum, and it seems to have gained acceptance. There is an outstanding patch in LLVM that will update the minimum compiler requirements for the top-of-tree LLVM codebase.
I think we need to consider something similar, given that we may no longer be able to compile LLVM code after this change.
I’d like to hear everybody’s thoughts about this.
I agree that follow LLVM seems to be a natural option here
Yeah I’d love to clean up std::tie(...) nonsense with the structured binding.
Seems reasonable to me as well given that Ubuntu 18.04 (which is what we use in CI and is the oldest supported version) comes with gcc 7.5 which supports -std=c++17
Would this require just bumping the minimum requirements to compile TVM or are things being deprecated that we rely on?
Just bumping the minimum compiler requirements.
It’s possible that compiling as C++17 will expose some issues (code valid in C++14 but not in C++17), but I think those should be minimal and nothing fundamental.
+1 from me, would be good to move forward
Overrally I agree it’s reasonable to migrate to C++17, but have we considered to maintain a C++11 compactible packed runtime? I have seen many legacy projects (including many smash-hit PC/mobile games) lack of support for C++14, thus hard to integrated with exisiting TVM runtime.
@zhuwenxi Thanks for bringing up an interesting point.
TVM have clear isolation of runtime and compiler part so it is indeed possible to build things for runtime part only. In the meantime, it does brings a bit of overhead and thought experiments on when writing code for runtime only – we can not use c++17 features for runtime only features like object family.
Personally I don’t see an issue doing such c++11 compatible integration for the runtime part only if there are use-cases. One possible idea is we add a CI flow that builds the runtime part optionally with c++11
Alternatively, these applications can also leverage the C runtime, which should remain compatible without c++17 requirements
Would love to see what others think.
The runtime no longer compiles with C++11. Examples of errors:
In file included from include/tvm/runtime/container/map.h:36:
include/tvm/runtime/container/./optional.h:112:3: error: 'auto' return without trailing return type; deduced return types are a C++14 extension
auto operator==(const Optional<T>& other) const {
^
In file included from include/tvm/runtime/registry.h:46:
include/tvm/runtime/packed_func.h:148:29: error: no template named 'enable_if_t' in namespace 'std'; did you mean 'enable_if'?
typename = std::enable_if_t<
~~~~~^~~~~~~~~~~
enable_if
/local/mnt/w/c/clang+llvm-13.0.0-x86_64-linux-gnu-ubuntu-20.04/bin/../include/c++/v1/type_traits:546:63: note: 'enable_if' declared here
template <bool, class _Tp = void> struct _LIBCPP_TEMPLATE_VIS enable_if {};
^
In file included from include/tvm/runtime/registry.h:46:
include/tvm/runtime/packed_func.h:1318:34: error: no member named 'index_sequence_for' in namespace 'std'
using T = typename Zipper<std::index_sequence_for<Args...>>::T;
~~~~~^
include/tvm/runtime/packed_func.h:1318:53: error: 'Args' does not refer to a value
using T = typename Zipper<std::index_sequence_for<Args...>>::T;
^
include/tvm/runtime/packed_func.h:1300:23: note: declared here
template <typename... Args>
^
include/tvm/runtime/packed_func.h:1318:22: error: expected a qualified name after 'typename'
using T = typename Zipper<std::index_sequence_for<Args...>>::T;
^
I’d prefer not to convert it all back to C++11, but if there is a strong need to have a C++11 runtime, that may be something to do…
It seems like we could add a CI step that compiles libtvm_runtime.so under c++11 with USE_LLVM OFF?
The runtime doesn’t use LLVM, there shouldn’t be any dependencies on LLVM headers in the runtime.
I think we still have LLVMModule which is technically runtime even if it shouldn’t be used when loading a library at runtime, no?
Ah, yes. You’re right.
That is why LLVMModule is not part of the runtime folder, it is only used for JIT(with compiler support).
By runtime i guess we mean those that get linked into libtvm_runtime
The actual switch in LLVM will take place right after branch release/15.x is created, which I’m guessing will happen at the beginning of August. We have some time to decide whether we want a C++11-only runtime.
if we want our runtime to deploy various and more OS and hardwares, C++11 or C++14 will be a safer choice. C api is a good choice too, but we don’t provide the same functionality as C++ api. For example, vm runtime doesn’t have C api(if I remember wrongly, please correct me). if we want users to use C api, maybe we still have some things to do.
I was also pondering whether at some point, AotExecutor is really just simple enough that we should just have a C impl…
LLVM 15 release schedule has now been posted.
Context: LLVM plans to switch to C++17 in the code base starting with LLVM 16. The switch will happen in the development branch (main) some time after release branch for LLVM 15 has been created.
It’s done: [LLVM] Update C++ standard to 17.
I’ll have a patch to update our cmake files on Monday, if nobody beats be to it.