Build BOUT++ with SYCL

[bendudson]

Basic CMake scripts for using the SYCL compiler. Tested with Intel DPC++, but only building the library due to hitting issue #2273.

Requires switching to C++17.

Based on a minimal MPI-SYCL example https://github.com/bendudson/mpi-sycl/

[ZedThree]

This is failing because it now uses C++17, which causes googletest to use std::variant, which in turn clashes with mpark::variant. I thought that mpark::variant nicely fell-back to using std::variant, but that doesn't seem to be the case.

I would think that something like the below (in our variant.hxx) would work, but it's failing and I'm not sure why:

#ifdef __has_include
#if __has_include(<variant>) && __cplusplus >= 201703L
#ifndef BOUT_HAS_STD_VARIANT
#define BOUT_HAS_STD_VARIANT 1
#endif // BOUT_HAS_STD_VARIANT
#include <variant>
#else
#ifndef BOUT_HAS_STD_VARIANT
#define BOUT_HAS_STD_VARIANT 0
#endif // BOUT_HAS_STD_VARIANT
#include "mpark/variant.hpp"
#endif  // __has_include(<variant>) && __cplusplus >= 201703L
#endif  // __has_include

#include "utils.hxx"

namespace bout {
namespace utils {

/// Import variant, visit into bout::utils namespace
#if BOUT_HAS_STD_VARIANT > 0
using std::variant;
using std::visit;
using std::holds_alternative;
using std::get;
#else
using mpark::variant;
using mpark::visit;
using mpark::holds_alternative;
using mpark::get;
#endif // BOUT_HAS_STD_VARIANT

This gives us lovely errors like:

In file included from ../include/bout/sys/variant.hxx:26,
                 from ../include/options.hxx:46,
                 from ../include/bout/deriv_store.hxx:42,
                 from ../include/bout/mesh.hxx:49,
                 from ../src/fileio/dataformat.cxx:2:
/usr/include/c++/11/variant: In instantiation of ‘struct std::variant_size<const Options::AttributeType>’:
/usr/include/c++/11/variant:1754:13:   required from ‘constexpr decltype(auto) std::visit(_Visitor&&, _Variants&& ...) [with _Visitor = bout::utils::details::IsEqual<std::__cxx11::basic_string<char> >; _Variants = {const Options::AttributeType&}]’
../include/bout/sys/variant.hxx:93:15:   required from ‘bool bout::utils::variantEqualTo(const Variant&, const T&) [with Variant = Options::AttributeType; T = std::__cxx11::basic_string<char>]’
../include/options.hxx:462:78:   required from here
/usr/include/c++/11/variant:84:12: error: invalid use of incomplete type ‘struct std::variant_size<Options::AttributeType>’
   84 |     struct variant_size<const _Variant> : variant_size<_Variant> {};
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/11/variant:81:12: note: declaration of ‘struct std::variant_size<Options::AttributeType>’
   81 |     struct variant_size;
      |            ^~~~~~~~~~~~
/usr/include/c++/11/variant: In instantiation of ‘constexpr decltype(auto) std::visit(_Visitor&&, _Variants&& ...) [with _Visitor = bout::utils::details::IsEqual<std::__cxx11::basic_string<char> >; _Variants = {const Options::AttributeType&}]’:
../include/bout/sys/variant.hxx:93:15:   required from ‘bool bout::utils::variantEqualTo(const Variant&, const T&) [with Variant = Options::AttributeType; T = std::__cxx11::basic_string<char>]’
../include/options.hxx:462:78:   required from here
/usr/include/c++/11/variant:1754:20: error: ‘value’ is not a member of ‘std::variant_size<const Options::AttributeType>’
 1754 |               std::make_index_sequence<
      |                    ^~~~~~~~~~~~~~~~~~~~
 1755 |                 std::variant_size<remove_reference_t<_Variants>...>::value>());
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/11/variant:1758:29: error: non-constant condition for static assertion
 1758 |               static_assert(__visit_rettypes_match,
      |                             ^~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/11/variant: In instantiation of ‘constexpr const size_t std::variant_size_v<const Options::AttributeType>’:
/usr/include/c++/11/variant:1049:10:   required from ‘struct std::__detail::__variant::__gen_vtable<std::__detail::__variant::__deduce_visit_result<bool>, bout::utils::details::IsEqual<std::__cxx11::basic_string<char> >&&, const Options::AttributeType&>’
/usr/include/c++/11/variant:1711:45:   required from ‘constexpr decltype(auto) std::__do_visit(_Visitor&&, _Variants&& ...) [with _Result_type = std::__detail::__variant::__deduce_visit_result<bool>; _Visitor = bout::utils::details::IsEqual<std::__cxx11::basic_string<char> >; _Variants = {const Options::AttributeType&}]’
/usr/include/c++/11/variant:1764:34:   required from ‘constexpr decltype(auto) std::visit(_Visitor&&, _Variants&& ...) [with _Visitor = bout::utils::details::IsEqual<std::__cxx11::basic_string<char> >; _Variants = {const Options::AttributeType&}]’
../include/bout/sys/variant.hxx:93:15:   required from ‘bool bout::utils::variantEqualTo(const Variant&, const T&) [with Variant = Options::AttributeType; T = std::__cxx11::basic_string<char>]’
../include/options.hxx:462:78:   required from here
/usr/include/c++/11/variant:97:70: error: ‘value’ is not a member of ‘std::variant_size<const Options::AttributeType>’
   97 |     inline constexpr size_t variant_size_v = variant_size<_Variant>::value;
      |                                                                      ^~~~~
/usr/include/c++/11/variant: In instantiation of ‘constexpr decltype(auto) std::__do_visit(_Visitor&&, _Variants&& ...) [with _Result_type = std::__detail::__variant::__deduce_visit_result<bool>; _Visitor = bout::utils::details::IsEqual<std::__cxx11::basic_string<char> >; _Variants = {const Options::AttributeType&}]’:
/usr/include/c++/11/variant:1764:34:   required from ‘constexpr decltype(auto) std::visit(_Visitor&&, _Variants&& ...) [with _Visitor = bout::utils::details::IsEqual<std::__cxx11::basic_string<char> >; _Variants = {const Options::AttributeType&}]’
../include/bout/sys/variant.hxx:93:15:   required from ‘bool bout::utils::variantEqualTo(const Variant&, const T&) [with Variant = Options::AttributeType; T = std::__cxx11::basic_string<char>]’
../include/options.hxx:462:78:   required from here
/usr/include/c++/11/variant:1711:52: error: ‘_S_vtable’ is not a member of ‘std::__detail::__variant::__gen_vtable<std::__detail::__variant::__deduce_visit_result<bool>, bout::utils::details::IsEqual<std::__cxx11::basic_string<char> >&&, const Options::AttributeType&>’
 1711 |         _Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
      |                                                    ^~~~~~~~~

[ZedThree]

This is either a compiler or a language bug, depending on your point of view: https://stackoverflow.com/questions/51309467/using-stdvisit-on-a-class-inheriting-from-stdvariant-libstdc-vs-libc

:disappointed:

One workaround seems to be to specialise std::variant_size for Options::AttributeType. I'll see if there are any other workarounds

[ZedThree]

I cannot get std::variant to work -- basically it's not currently possible to inherit from std::variant with gcc. Might be fixed in C++23!

Ok, I think this is really a case of ADL being dumb or something, and the easy fix is to just explicitly use bout::utils::visit in our variant.hxx, rather than trying to switch between mpark::variant and std::variant.

The less pleasant way involves a lot of changes to Options::AttributeType, which is where we inherit from std::variant.

modified   include/bout/sys/variant.hxx
@@ -81,7 +81,7 @@ struct IsEqual {
 /// which \p v can hold.
 template <typename Variant, typename T>
 bool variantEqualTo(const Variant& v, const T& t) {
-  return visit(details::IsEqual<T>(t), v);
+  return bout::utils::visit(details::IsEqual<T>(t), v);
 }
 
 ////////////////////////////////////////////////////////////
@@ -119,7 +119,7 @@ struct StaticCastOrThrow {
 /// in which case std::bad_cast will be thrown at runtime
 template <typename Variant, typename T>
 T variantStaticCastOrThrow(const Variant &v) {
-  return visit( details::StaticCastOrThrow<T>(), v );
+  return bout::utils::visit( details::StaticCastOrThrow<T>(), v );
 }
 
 namespace details {
@@ -134,7 +134,7 @@ struct ToString {
 } // namespace details
 
 template <typename Variant>
-std::string variantToString(const Variant& v) { return visit(details::ToString(), v); }
+std::string variantToString(const Variant& v) { return bout::utils::visit(details::ToString(), v); }
 
 } // namespace utils
 } // namespace bout

@bendudson Do you want me to push these changes?

Also, we'll need to do something about our use of std::uncaught_exception. Looking at that now

[bendudson]

Thanks for looking into this @ZedThree ! It sounds like always using mpark::variant is the best option. That's a pain. Yes, happy for those changes to be pushed.

[github-actions[bot]]

clang-tidy review says "All clean, LGTM! :+1:"

[dschwoerer]

There are already many systems out there where packages are compiled with an older compiler, so switching to a newer compiler can cause issues and requires rebuilding all dependencies. I am fine with that, but thought maybe others should also chime in ...

Would it be worth to only require C++17 if SYCL is enabled, and otherwise use C++14? One of the CI jobs could be converted to run on C++17, to ensure that keeps working ...

[bendudson]

@dschwoerer yes I think that's a sensible plan. SYCL support is experimental, and it's not clear yet that it will even be useful. It would be nice to use C++17 soon, but probably not for 5.0 at least.