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

vectorAddMMAP target does not compile due to virtual memory feature set_access_mode #344

Closed
Q-Minh opened this issue May 27, 2022 · 9 comments
Labels
ms-windows Related to the Microsoft Windows operating system resolved-on-development

Comments

@Q-Minh
Copy link

Q-Minh commented May 27, 2022

The example target vectorAddMMAP has the error cuda-api-wrappers\examples\modified_cuda_samples\vectorAddMMAP\vectorAddMMAP.cpp(163,107): error C2665: 'cuda::memory::virtual_::set_access_mode': none of the 2 overloads could convert all the argument types when compiling with MSVC using VS2019.

@eyalroz
Copy link
Owner

eyalroz commented May 27, 2022

Why couldn't they? What are the types MSVC is seeing? :-(

@Q-Minh
Copy link
Author

Q-Minh commented May 28, 2022

There is an ambiguous call to either:

template <template <typename... Ts> class ContiguousContainer>
inline void set_access_mode(
	mapping_t mapping,
	const ContiguousContainer<device_t>& devices,
	access_mode_t access_mode);

or

template <template <typename... Ts> class ContiguousContainer>
void set_access_mode(
	region_t fully_mapped_region,
	const ContiguousContainer<device_t>& devices,
	access_mode_t access_mode);

in vectorAddMMAP.cpp line 163. This is quite weird, to be honest, looks like a compiler bug to me, since the signatures are quite obviously different, with mapping_t and region_t having no related inheritance tree or anything.

@eyalroz
Copy link
Owner

eyalroz commented May 28, 2022

Maybe it's an issue with MSVC's support for template-template parameters.

What happens if we separate the two function names? i.e. set_mapping_access_mode() and set_region_access_mode()?

@Q-Minh
Copy link
Author

Q-Minh commented May 28, 2022

Okay, so the actual problem was that std::iterator_traits<Iter> was not able to specialize Iter = enumerator<...>::const_iterator, because there were some typedefs/aliases missing. The fix is then to add:

using difference_type = std::ptrdiff_t;
using value_type = value_type;
using pointer = value_type*;
using reference = value_type&;
using iterator_category = std::forward_iterator_tag;

in the declaration of struct enumerator<Container>::iterator, and

using difference_type = std::ptrdiff_t;
using value_type = value_type;
using pointer = value_type const*;
using reference = value_type const&;
using iterator_category = std::forward_iterator_tag;

in the declaration of struct enumerator<Container>::const_iterator.

Now MSVC can disambiguate between the 2 overloads of set_access_mode(...).

@eyalroz
Copy link
Owner

eyalroz commented May 28, 2022

Are you sure we need value_type? That looks redundant.

@Q-Minh
Copy link
Author

Q-Minh commented May 28, 2022

Yeah, pretty sure, because std::iterator_traits<Iter> takes the iterator itself as template parameter, not the container, and it attempts to directly access member typedefs on the Iter type passed in. Based on cppreference, there are 5 required member typedefs, of which value_type is a part of.

eyalroz added a commit that referenced this issue May 28, 2022
…essary for MSVC to properly choose between then in `memory::virtual_::set_access_mode()`
@eyalroz
Copy link
Owner

eyalroz commented May 28, 2022

How's this?

@Q-Minh
Copy link
Author

Q-Minh commented May 28, 2022

How's this?

Looks good! I don't know about gcc and clang, but at least with msvc, you don't need to specify enumerator:: to capture the value_type of the enclosing enumerator class. However, if you do want to specify it, I would write enumerator<Container>::value_type instead.

@eyalroz
Copy link
Owner

eyalroz commented May 28, 2022

@Q-Minh : Sure.

@eyalroz eyalroz closed this as completed May 28, 2022
@eyalroz eyalroz reopened this May 28, 2022
eyalroz added a commit that referenced this issue May 28, 2022
…SVC to properly choose between then in `memory::virtual_::set_access_mode()`
@eyalroz eyalroz added bug resolved-on-development ms-windows Related to the Microsoft Windows operating system and removed bug labels May 28, 2022
eyalroz added a commit that referenced this issue May 29, 2022
…SVC to properly choose between then in `memory::virtual_::set_access_mode()`
eyalroz added a commit that referenced this issue Jun 2, 2022
…SVC to properly choose between then in `memory::virtual_::set_access_mode()`
eyalroz added a commit that referenced this issue Jun 20, 2022
…SVC to properly choose between then in `memory::virtual_::set_access_mode()`
eyalroz added a commit that referenced this issue Jun 20, 2022
…SVC to properly choose between then in `memory::virtual_::set_access_mode()`
eyalroz added a commit that referenced this issue Jun 20, 2022
…SVC to properly choose between then in `memory::virtual_::set_access_mode()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ms-windows Related to the Microsoft Windows operating system resolved-on-development
Projects
None yet
Development

No branches or pull requests

2 participants