WIP: Updated mdspan #801
WIP: Updated mdspan #801
Conversation
| #pragma clang diagnostic ignored "-Wunused-parameter" | ||
| #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" | ||
| #pragma clang diagnostic ignored "-Wreserved-id-macro" | ||
| #endif |
There was a problem hiding this comment.
We added all of the above suppressions. Probably should keep these. You may need to add a new one for gcc.
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Werror=undef"
#endif
[original content of mdspan goes here]
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic pop
#endifThere was a problem hiding this comment.
Optionally, create a new file mdspan_wrapper.hh that contains all of these pragmas and has one line of code: #include <experimental/mdspan. This might might updating mdspan easier.
There was a problem hiding this comment.
Since a separate wrapper header file will I think require one of (1) switching #include <mdspan> (which is nice because it mimics future STL behavior) with #include <mdspan_wrapper> (2) changing the kokkos/mdspan mdspanto e.g.mdspan_wrappedand including this in a newmdspan` file, I'm in favor of just leaving things the way they are now. Also I now know how to re-add the compiler guards (which may anyways change with future versions of mdspan...)
|
|
||
| #ifdef _MSC_FULL_VER | ||
| #pragma warning(pop) | ||
| #endif |
There was a problem hiding this comment.
We need to keep these pop pragmas (plus add the gnu one that I mentioned above).
| @@ -45,20 +45,22 @@ | |||
|
|
|||
| #include "macros.hpp" | |||
|
|
|||
| #include <cstddef> | |||
| #include <cstddef> // ptrdiff_t | |||
There was a problem hiding this comment.
Did you process these with clang-format? If so, that is fine. I'm also okay with adding a pragma to tell clang-format to ignore the file. Ref: https://philwrightdev.wordpress.com/2014/10/01/making-clang-format-ignore-sections/
There was a problem hiding this comment.
Preserving our discussion on mattermost here: clang-format'ing this version of kokkos/mdspan appears to cause compiler errors. So format checking is now disabled for experimental with a .clang-format
| @@ -56,9 +56,13 @@ | |||
| #pragma clang diagnostic ignored "-Wreserved-id-macro" | |||
| #endif | |||
|
|
|||
| #if defined(__GNUC__) && !defined(__clang__) | |||
| #pragma GCC diagnostic push | |||
| #pragma GCC diagnostic ignored "-Werror=undef" | |||
There was a problem hiding this comment.
This should be -Wundef.
The error message is confusing because we also have -Werror that converts warning to errors for the CI to capture.
|
There are fatal bugs in the new mdspan wrt compiling with gcc-8.1.0. See kokkos/mdspan#21 and associated issue kokkos/mdspan#20. You can merge my diff into draco or wait for kokkos/mdspan to fix this issue. |
|
It looks like Visual Studio also has major issues with the new version of mdspan. I'm digging into it. The errors are significant and beyond my ability to debug. We may have to pass on this version. We may want to find the new |
|
My thinking is it would be best to wait for these fixes to go into kokkos/mdspan so we can reference a specific commit of theirs. If we need intel 17 + CPT in the near future and the VS errors are challenging, maybe the thing to do will be to patch in manually unrolled dimensional indexing to the Draco tabulated DEDX reader for now. |
I agree. Let's put this PR on hold. |
|
Replaced by #814 |
Background
Purpose of Pull Request
Description of changes
Status
masterbranch of mdspan is broken until these issues are fixed: