Skip to content
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

C++17 Support #84

Open
codecircuit opened this issue Nov 18, 2020 · 7 comments
Open

C++17 Support #84

codecircuit opened this issue Nov 18, 2020 · 7 comments
Assignees
Labels
bug Something isn't working fixed on development A fix for this issue has been committed to the development branch

Comments

@codecircuit
Copy link

I tried to use cuda-kat with c++17, which fails to compile. The reproduction should be easy, because already the include of the header causes compilation issues.

@eyalroz
Copy link
Owner

eyalroz commented Nov 18, 2020

And does it compile ok with CUDA 11 but C++14? C++11?

@codecircuit
Copy link
Author

With C++14 it works fine.

@eyalroz
Copy link
Owner

eyalroz commented Nov 23, 2020

Which version of CUDA are you using?

When using CUDA 11.0.3, I get compilation errors with C++11 (or 14) as well, see:

https://stackoverflow.com/questions/64968200/constexpr-note-invalid-arithmetic-on-non-array-pointer

Can you describe what you're seeing more specifically, with relevant version numbers and error messages?

@eyalroz
Copy link
Owner

eyalroz commented Nov 23, 2020

... well, I've started working on C++17 compatibility with CUDA 11.1. Definitely a bunch of issues - but all minor so far.

@codecircuit
Copy link
Author

codecircuit commented Nov 23, 2020

CMakeLists.txt:

cmake_minimum_required(VERSION 3.18)

project(foobarbaz LANGUAGES CXX CUDA)

find_package(cuda-kat REQUIRED)

add_executable(foobarbaz "foobarbaz.cu")

target_link_libraries(
    foobarbaz
    PRIVATE
    cuda-kat::cuda-kat
)

target_compile_features(foobarbaz PUBLIC cuda_std_17)

My source code file foobarbaz.cu:

#include <kat/containers/array.hpp>

__global__ void kernel(kat::array<kat::array<int*, 5>, 4> arr) {
    *arr[threadIdx.x][threadIdx.x] = threadIdx.x;
}

int main() {}

I use CMake 3.18.4, CUDA 11.1.105, and GCC 8.32 and built the project with the Release build. The compile error I get:

/home/wolf/repositories/cuda-kat/src/kat/common.hpp(49): error: constant "B" is not a type name

/home/wolf/repositories/cuda-kat/src/kat/detail/range_access.hpp(283): error: "empty" has already been declared in the current scope

/home/wolf/repositories/cuda-kat/src/kat/detail/range_access.hpp(283): error: identifier "initializer_list" is undefined

/home/wolf/repositories/cuda-kat/src/kat/detail/range_access.hpp(283): error: type name is not allowed

/home/wolf/repositories/cuda-kat/src/kat/detail/range_access.hpp(283): error: identifier "init_list" is undefined

/home/wolf/repositories/cuda-kat/src/kat/detail/range_access.hpp(283): error: expected a ";"

/usr/include/wchar.h(284): error: identifier "wint_t" is undefined

/usr/include/wchar.h(288): error: identifier "wint_t" is undefined

/usr/include/wchar.h(316): error: identifier "wint_t" is undefined

/usr/include/wchar.h(318): error: expected a ";"

/usr/include/wchar.h(324): error: declaration is incompatible with "int kat::wctob(<error-type>) throw()"
(288): here

...

To compile it again I applied this hack:

diff --git a/src/kat/common.hpp b/src/kat/common.hpp
index 0c6c6e2..0d6c770 100644
--- a/src/kat/common.hpp
+++ b/src/kat/common.hpp
@@ -25,8 +25,8 @@ namespace kat {
  */
 using size_t = std::size_t;
 
-#if __cplusplus < 201703L
-
+//#if __cplusplus < 201703L
+#if 1
 // Some C++17 type traits definable in C++11
 
 template<typename ...               > struct conjunction : std::true_type {};
diff --git a/src/kat/detail/range_access.hpp b/src/kat/detail/range_access.hpp
index b2b9aa9..54a0d6f 100644
--- a/src/kat/detail/range_access.hpp
+++ b/src/kat/detail/range_access.hpp
@@ -278,10 +278,10 @@ namespace kat {
    *  @brief  Return whether an initializer_list is empty.
    *  @param  init_list  Initializer list.
    */
-  template <typename T>
-    [[nodiscard]] constexpr bool
-    empty(initializer_list<T> init_list) noexcept
-    { return init_list.size() == 0;}
+  // template <typename T>
+  //   [[nodiscard]] constexpr bool
+  //   empty(initializer_list<T> init_list) noexcept
+  //   { return init_list.size() == 0;}
 
   /**
    *  @brief  Return the data pointer of a container.
@@ -316,10 +316,10 @@ namespace kat {
    *  @brief  Return the data pointer of an initializer list.
    *  @param  init_list  Initializer list.
    */
-  template <typename T>
-    constexpr const T*
-    data(initializer_list<T> init_list) noexcept
-    { return init_list.begin(); }
+  // template <typename T>
+  //   constexpr const T*
+  //   data(initializer_list<T> init_list) noexcept
+  //   { return init_list.begin(); }
 
 #endif // C++17
 

eyalroz pushed a commit that referenced this issue Nov 23, 2020
…ed using C++17):

* Corrected `using` syntax for `conjunction`, `disjunction`, `bool_constant` and `negation`
* Now using `kat::swap` in the swapping loop of `kat::array::swap`
* Now using iterators in the swapping loop of `kat::array::swap`
* Not trying to issue a trap instruction with C++17 in constexpr methods; with C++20 - issuing the instruction when guaranteed evaluation at run-time (and on the device)
* Added: Missing template argument deducation guide for `kat::array`
* Made sure `std::forward` and `std::move` are made accessible with `using` where necessary
* Dropped a copy of an internal libstdc++ definition we were making by mistake
* Avoiding warnings in `span.hpp` about a useless comparison of a 0-valued template argument (which only show up with C++17 enabled)
* Removed inappropriately-placed `KAT_HD` markers within `if constexpr` statements
* Properly marking `std::byte` with its namespace (rather than using just `byte`)
* Properly marking `std::initializer_list` with its namespace (rather than using just `initializer_list`)
* Added: A note about a missing implementation of kat::apply (see issue #86)
* Avoiding a warning about a narrowing cast `int` -> `std::size_t` which the compiler should not be issuing (for i between 0 and 4)
* Moved a type definition out of a doctest sub-testcase - another issue similar to what we had with `tuple_get` (this may already be a problem with C++14)
* Some commented-out code and `#if FALSE`'es related to issue #87 (C++17 + swap troubles)
* Some code formatting tweaks
@eyalroz
Copy link
Owner

eyalroz commented Nov 23, 2020

Try it now.

@codecircuit
Copy link
Author

codecircuit commented Nov 24, 2020

It works now. Thanks!

@eyalroz eyalroz self-assigned this Nov 24, 2020
@eyalroz eyalroz added the bug Something isn't working label Nov 24, 2020
eyalroz pushed a commit that referenced this issue May 28, 2021
…ng C++17 by default):

* Corrected `using` syntax for `conjunction`, `disjunction`, `bool_constant` and `negation`
* Now using `kat::swap` in the swapping loop of `kat::array::swap`
* Now using iterators in the swapping loop of `kat::array::swap`
* Not trying to issue a trap instruction with C++17 in constexpr methods; with C++20 - issuing the instruction when guaranteed evaluation at run-time (and on the device)
* Added: Missing template argument deducation guide for `kat::array`
* Made sure `std::forward` and `std::move` are made accessible with `using` where necessary
* Dropped a copy of an internal libstdc++ definition we were making by mistake
* Avoiding warnings in `span.hpp` about a useless comparison of a 0-valued template argument (which only show up with C++17 enabled)
* Removed inappropriately-placed `KAT_HD` markers within `if constexpr` statements
* Properly marking `std::byte` with its namespace (rather than using just `byte`)
* Properly marking `std::initializer_list` with its namespace (rather than using just `initializer_list`)
* Added: A note about a missing implementation of kat::apply (see issue #86)
* Avoiding a warning about a narrowing cast `int` -> `std::size_t` which the compiler should not be issuing (for i between 0 and 4)
* Moved a type definition out of a doctest sub-testcase - another issue similar to what we had with `tuple_get` (this may already be a problem with C++14)
* Some commented-out code and `#if FALSE`'es related to issue #87 (C++17 + swap troubles)
* Some code formatting tweaks
@eyalroz eyalroz added the fixed on development A fix for this issue has been committed to the development branch label May 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed on development A fix for this issue has been committed to the development branch
Projects
None yet
Development

No branches or pull requests

2 participants