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

Remove integer_sequence backward compatibility implementation #3533

Merged
merged 4 commits into from
Jan 22, 2021

Conversation

dhollman
Copy link

@dhollman dhollman commented Oct 27, 2020

They're in namespace Impl and they're only used in the tests that test if they work.

Removing

  • integer_sequence
  • make_integer_sequence_concat
  • index_sequence
  • make_index_sequence
  • integer_sequence_at
  • at
  • reverse_integer_sequence
  • exclusive_scan_integer_sequence
  • inclusive_scan_integer_sequence

dalg24
dalg24 previously approved these changes Oct 27, 2020
Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me if it passes CI

@dhollman dhollman changed the title Remove integer_sequence algorithms. Remove integer_sequence backward compatibility implementation Oct 27, 2020
@dhollman
Copy link
Author

@dalg24 we can actually just pull all of our integer_sequence implementation out.

@dalg24
Copy link
Member

dalg24 commented Oct 28, 2020

Didn't find any match in Kokkos-Kernels nor Trilinos

@dalg24 dalg24 dismissed their stale review October 28, 2020 05:58

One of the CUDA builds failed on Jenkins

@dhollman
Copy link
Author

This is an issue with how EDG handles alias templates with respect to function template matching. I've run in to this before and I'll fix it as soon as I'm in front of my machine again.

Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@dalg24 dalg24 force-pushed the remove-int-seq-scans branch 2 times, most recently from c9919c2 to 3947ee2 Compare November 6, 2020 13:34
@dalg24
Copy link
Member

dalg24 commented Jan 19, 2021

Retest this please

@masterleinad
Copy link
Contributor

/var/jenkins/workspace/Kokkos/core/src/impl/KokkosExp_IterateTileGPU.hpp(84): error: integer_sequence is not a template

should be easy to fix after rebasing.

@masterleinad
Copy link
Contributor

diff --git a/core/src/impl/KokkosExp_IterateTileGPU.hpp b/core/src/impl/KokkosExp_IterateTileGPU.hpp
index 5ca1f245e..688afcc10 100644
--- a/core/src/impl/KokkosExp_IterateTileGPU.hpp
+++ b/core/src/impl/KokkosExp_IterateTileGPU.hpp
@@ -81,7 +81,7 @@ _tag_invoke(Functor const& f, Args&&... args) {
 template <class Tag, class Functor, class T, size_t N, size_t... Idxs,
           class... Args>
 KOKKOS_IMPL_FORCEINLINE_FUNCTION void _tag_invoke_array_helper(
-    Functor const& f, T (&vals)[N], integer_sequence<size_t, Idxs...>,
+    Functor const& f, T (&vals)[N], std::integer_sequence<size_t, Idxs...>,
     Args&&... args) {
   _tag_invoke<Tag>(f, vals[Idxs]..., (Args &&) args...);
 }
@@ -90,7 +90,7 @@ template <class Tag, class Functor, class T, size_t N, class... Args>
 KOKKOS_IMPL_FORCEINLINE_FUNCTION void _tag_invoke_array(Functor const& f,
                                                         T (&vals)[N],
                                                         Args&&... args) {
-  _tag_invoke_array_helper<Tag>(f, vals, make_index_sequence<N>{},
+  _tag_invoke_array_helper<Tag>(f, vals, std::make_index_sequence<N>{},
                                 (Args &&) args...);
 }

fixes this.

@dalg24
Copy link
Member

dalg24 commented Jan 22, 2021

Retest this please

@masterleinad
Copy link
Contributor

The failing SYCL CI is fixed by #3741.

@dalg24 dalg24 merged commit 98cde78 into kokkos:develop Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants