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

implement alpaka::concepts::Tag #2403

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

SimeonEhrig
Copy link
Member

@SimeonEhrig SimeonEhrig commented Sep 30, 2024

First alpaka C++20 concept :-) I think there are several points to discuss.

Implement alpaka::concept::Tag which checks the requirements for an alpaka tag.
Remove support Clang 10, 11 and 12 because they are not fully supports C++20 concepts.

#define CREATE_ACC_TAG(tag_name) \
struct tag_name \
struct tag_name : public concepts::Implements<alpaka::ConceptTag, tag_name> \
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to introduce the ConceptTag to make the class unique. Otherwise the only property to identify a alpaka tag is the helper static class function get_name, which is not unique.

Maybe we should think about to rename alpaka::concepts because it confuses now, than we have C++20 concepts.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked what alpaka::concepts::Implements is actual doing and if we can replace it with C++20 concepts.
The answer is no, because it implements a type hierarchy like in Julia with type traits. It is possible to do it with C++ concepts but nothing which is already provided by the STL.

Therefore I suggest to rename alpaka::concepts to alpaka::type_hierarchy and also the corresponding function and structs to avoid confusions with C++20 concepts.

Copy link
Member Author

Choose a reason for hiding this comment

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

In an offline discussion with @fwyzard he suggested alpaka::interfaces. I like the idea, because actual this mechanism imitates traits from other languages but traits has already a meaning in alpaka. Therefore interface is a good solution because it sounds like java interfaces which is pretty similar.

@SimeonEhrig
Copy link
Member Author

@alpaka-group/alpaka-maintainers I was thinking about naming concepts. Here it is easy because alpaka::Tag is unique and not used yet. Maybe the name looks like a base class where the actual tags inherintant, but I think this fine.

I the case of alpaka::Queue this is different. We have already the class alpaka::Queue:

using Queue = alpaka::Queue<Acc, QueueProperty>;

In the case of template class deduction this can confuse:

// use the class alpaka::Queue, deducts the argument from the function call
alpaka::Queue queue = create_queue();

// use the concept alpaka::Queue
alpaka::Queue auto queue2 = create_queue();

The C++ standard makes clear, where classes and where concepts are used. But without the context, like in the documentation, it is hard to distinguish.

Possible solutions:

  • let it as it
  • add a prefix like for templates: alpaka::CTag, alpaka::CAcc, alpaka::CQueue
  • used the alpaka namespace alpaka::concept::Tag, alpaka::concept::Acc, alpaka::concept::Queue

What do you think?

@fwyzard
Copy link
Contributor

fwyzard commented Oct 1, 2024

What is the syntax for using concepts ?

@SimeonEhrig
Copy link
Member Author

What is the syntax for using concepts ?

Have a look in this commit: 37c62ed

Also interesting is, what is the error message. Try to call the functions specialize_tag<NoTag1>() and specialize_tag<NoTag2>() in the AccTagTest.cpp and see what the compiler is telling you.

@SimeonEhrig
Copy link
Member Author

You can also write something like this:

template<typename T>
   requires std::integral<T>
T gcd(T a, T b) {
   if( b == 0 ) return a;
   else return gcd(b, a % b);
}

But I prefer the version, which I used because it uses less boilerplate code and looks more intuitive.

@SimeonEhrig SimeonEhrig force-pushed the ConceptTag branch 3 times, most recently from a61e591 to 3527b00 Compare October 2, 2024 08:30
@psychocoderHPC
Copy link
Member

Possible solutions:

  • let it as it
  • add a prefix like for templates: alpaka::CTag, alpaka::CAcc, alpaka::CQueue
  • used the alpaka namespace alpaka::concept::Tag, alpaka::concept::Acc, alpaka::concept::Queue

What do you think?

I am a friend of namespaces and not mangling all into names. C* is also very often used for classes. This is what I learned long time ago. Namespaces will clearly separate implementations from concepts.

@fwyzard
Copy link
Contributor

fwyzard commented Oct 2, 2024

You can also write something like this:

template<typename T>
   requires std::integral<T>

No, I agree that

template<std::integral T>

looks cleaner.

@fwyzard
Copy link
Contributor

fwyzard commented Oct 2, 2024

used the alpaka namespace alpaka::concept::Tag, alpaka::concept::Acc, alpaka::concept::Queue

👍🏻

@fwyzard
Copy link
Contributor

fwyzard commented Oct 2, 2024

To avoid confusion, we could rename the current namespace and classes called "Concept" to "Interface" ?

@SimeonEhrig
Copy link
Member Author

You can also write something like this:

template<typename T>
   requires std::integral<T>

No, I agree that

template<std::integral T>

looks cleaner.

I agree with you. And I was thinking about, when you need this requires in general. The answer is, if you have more than one template argument in the concept.

template<typename T, typename U>
concept foo = requires { requires std::is_same_v<T,U>; };

template<typename T, typename U>
void bar() requires foo<T,U> {}

But if we can do it, we should use the first syntax.

@SimeonEhrig
Copy link
Member Author

To avoid confusion, we could rename the current namespace and classes called "Concept" to "Interface" ?

I agree with the idea. So let's rename the current alpaka::concepts to alpaka::inteface and reuse the namespace alpaka::concepts for C++20 concepts.

By the way. All our namespaces are singular. But we cannot use alpaka::concept because concept is a key word.

@SimeonEhrig SimeonEhrig force-pushed the ConceptTag branch 2 times, most recently from 85c3302 to 6526078 Compare October 8, 2024 10:38
@SimeonEhrig SimeonEhrig added this to the 2.0.0 milestone Oct 8, 2024
@SimeonEhrig SimeonEhrig marked this pull request as ready for review October 8, 2024 10:40
@SimeonEhrig SimeonEhrig changed the title implement concept alpaka::Tag implement alpaka::concepts::Tag Oct 8, 2024
Copy link
Member

@psychocoderHPC psychocoderHPC left a comment

Choose a reason for hiding this comment

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

alpaka::concepts::Tag should be concepts::Tag.
Except in the tests if we are not in the alpaka namespace.

include/alpaka/acc/TagAccIsEnabled.hpp Outdated Show resolved Hide resolved
- remove support for Clang 10 until 12 because the C++20 Concepts are not fully supported
@psychocoderHPC psychocoderHPC merged commit f4e1747 into alpaka-group:develop Oct 9, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants