Skip to content
This repository was archived by the owner on Mar 21, 2024. It is now read-only.

Fix structured binding support #317

Merged
merged 2 commits into from
Nov 10, 2022
Merged

Conversation

miscco
Copy link
Collaborator

@miscco miscco commented Sep 21, 2022

Currently structured bindings for cuda::std::tuple and cuda::std::array were broken.

The reason for that is that the standard requires, that the specializations of tuple_size and tuple_element reside in namespace std. whereas our specializations resided in namespace cuda::std

Work around that by pulling those specializations into namespace std too.

Fixes CUDA Tuple Structured Binding Declaration Broken #316

@miscco miscco requested review from wmaxey and jrhemstad September 21, 2022 07:30
@miscco miscco linked an issue Sep 21, 2022 that may be closed by this pull request
@jrhemstad jrhemstad requested a review from griwes September 21, 2022 12:56
@wmaxey wmaxey added the testing: internal ci in progress Currently testing on internal NVIDIA CI (DVS). label Sep 22, 2022
@wmaxey wmaxey added this to the 1.9.0 milestone Sep 22, 2022
template <class _Tp, size_t _Size>
struct tuple_size<_CUDA_VSTD::array<_Tp, _Size>>
: _CUDA_VSTD::tuple_size<_CUDA_VSTD::array<_Tp, _Size>>
{};
Copy link
Member

Choose a reason for hiding this comment

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

libcxx is failing to build due to this:

2022-09-22 06:23:19+00:00| In file included from /sw/gpgpu/libcudacxx/libcxx/src/filesystem/filesystem_common.h:14:
2022-09-22 06:23:19+00:00| /sw/gpgpu/libcudacxx/libcxx/include/array:497:12: error: explicit specialization of non-template struct 'tuple_size'
2022-09-22 06:23:19+00:00| struct tuple_size<_CUDA_VSTD::array<_Tp, _Size>>
2022-09-22 06:23:19+00:00| ^         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2022-09-22 06:23:19+00:00| /sw/gpgpu/libcudacxx/libcxx/include/array:502:12: error: explicit specialization of non-template struct 'tuple_element'
2022-09-22 06:23:19+00:00| struct tuple_element<_Ip, _CUDA_VSTD::array<_Tp, _Size>>
2022-09-22 06:23:19+00:00| ^            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2022-09-22 06:23:19+00:00| In file included from /sw/gpgpu/libcudacxx/libcxx/src/filesystem/operations.cpp:10:
2022-09-22 06:23:19+00:00| /sw/gpgpu/libcudacxx/libcxx/include/array:497:12: error: explicit specialization of non-template struct 'tuple_size'
2022-09-22 06:23:19+00:00| struct tuple_size<_CUDA_VSTD::array<_Tp, _Size>>
2022-09-22 06:23:19+00:00| ^         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2022-09-22 06:23:19+00:00| /sw/gpgpu/libcudacxx/libcxx/include/array:502:12: error: explicit specialization of non-template struct 'tuple_element'
2022-09-22 06:23:19+00:00| struct tuple_element<_Ip, _CUDA_VSTD::array<_Tp, _Size>>
2022-09-22 06:23:19+00:00| ^            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2022-09-22 06:23:19+00:00| 1 warning and 2 errors generated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But the forward declaration is like directly above. How can that fail?

template<size_t _Ip, class... _Tp>
struct tuple_element<_Ip, _CUDA_VSTD::tuple<_Tp...>>
: _CUDA_VSTD::tuple_element<_Ip, _CUDA_VSTD::tuple<_Tp...>>
{};
Copy link
Member

Choose a reason for hiding this comment

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

Same as other comment:

2022-09-22 06:23:19+00:00| In file included from /sw/gpgpu/libcudacxx/libcxx/include/utility:200:
2022-09-22 06:23:19+00:00| /sw/gpgpu/libcudacxx/libcxx/include/__tuple:574:12: error: explicit specialization of non-template struct 'tuple_size'
2022-09-22 06:23:19+00:00| struct tuple_size<_CUDA_VSTD::tuple<_Tp...>>
2022-09-22 06:23:19+00:00| ^         ~~~~~~~~~~~~~~~~~~~~~~~~~~~
2022-09-22 06:23:19+00:00| /sw/gpgpu/libcudacxx/libcxx/include/__tuple:579:12: error: explicit specialization of non-template struct 'tuple_element'
2022-09-22 06:23:19+00:00| struct tuple_element<_Ip, _CUDA_VSTD::tuple<_Tp...>>
2022-09-22 06:23:19+00:00| ^            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@miscco
Copy link
Collaborator Author

miscco commented Sep 22, 2022

There are even worse ones

2022-09-22 00:10:56-07:00| /sw/gpgpu/libcudacxx/libcxx/include/__tuple:574:12: error: reference to 'tuple_size' is ambiguous
2022-09-22 00:10:56-07:00| struct tuple_size<_CUDA_VSTD::tuple<_Tp...>>
2022-09-22 00:10:56-07:00| ^
2022-09-22 00:10:56-07:00| /sw/gpgpu/libcudacxx/libcxx/include/__tuple:568:12: note: candidate found by name lookup is 'std::tuple_size'
2022-09-22 00:10:56-07:00| struct tuple_size;
2022-09-22 00:10:56-07:00| ^
2022-09-22 00:10:56-07:00| /sw/gpgpu/libcudacxx/libcxx/include/__tuple:26:54: note: candidate found by name lookup is 'std::__1::tuple_size'
2022-09-22 00:10:56-07:00| template <class _Tp> struct _LIBCUDACXX_TEMPLATE_VIS tuple_size;
2022-09-22 00:10:56-07:00| ^
2022-09-22 00:10:56-07:00| /sw/gpgpu/libcudacxx/libcxx/include/__tuple:579:12: error: reference to 'tuple_element' is ambiguous
2022-09-22 00:10:56-07:00| struct tuple_element<_Ip, _CUDA_VSTD::tuple<_Tp...>>
2022-09-22 00:10:56-07:00| ^
2022-09-22 00:10:56-07:00| /sw/gpgpu/libcudacxx/libcxx/include/__tuple:571:12: note: candidate found by name lookup is 'std::tuple_element'
2022-09-22 00:10:56-07:00| struct tuple_element;
2022-09-22 00:10:56-07:00| ^
2022-09-22 00:10:56-07:00| /sw/gpgpu/libcudacxx/libcxx/include/__tuple:58:66: note: candidate found by name lookup is 'std::__1::tuple_element'
2022-09-22 00:10:56-07:00| template <size_t _Ip, class _Tp> struct _LIBCUDACXX_TEMPLATE_VIS tuple_element;

That would require us to specialize depending on host library

@miscco miscco force-pushed the tuple_structured_bindings branch 2 times, most recently from 5b1799e to 9493cf5 Compare September 22, 2022 07:51
Copy link
Collaborator

@griwes griwes left a comment

Choose a reason for hiding this comment

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

The __tuple comments also apply to array.

Currently structured bindings for `cuda::std::tuple` and `cuda::std::array` was broken.

The reason for that is that the standard requires, that the specializations of `tuple_size` and `tuple_element` reside in namespace std. whereas our specializations resided in namespace `cuda::std`

Work around that by pulling those specializations into namespace std too.

Fixes CUDA Tuple Structured Binding Declaration Broken NVIDIA#316
@miscco miscco force-pushed the tuple_structured_bindings branch from ea09d2d to 466facb Compare October 14, 2022 06:36
@miscco
Copy link
Collaborator Author

miscco commented Oct 17, 2022

@griwes this passed internal CI and I believe your comments are addressed. Can we merge this?

@jrhemstad jrhemstad requested a review from griwes October 25, 2022 14:43
@wmaxey wmaxey added testing: internal ci passed Passed internal NVIDIA CI (DVS). and removed testing: internal ci in progress Currently testing on internal NVIDIA CI (DVS). labels Nov 8, 2022
@miscco miscco merged commit a982592 into NVIDIA:main Nov 10, 2022
@miscco miscco deleted the tuple_structured_bindings branch November 10, 2022 07:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
testing: internal ci passed Passed internal NVIDIA CI (DVS).
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

CUDA Tuple Structured Binding Declaration Broken
3 participants