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

Atomic functions are too verbose #1005

Closed
bernhardmgruber opened this issue May 20, 2020 · 8 comments
Closed

Atomic functions are too verbose #1005

bernhardmgruber opened this issue May 20, 2020 · 8 comments

Comments

@bernhardmgruber
Copy link
Member

While porting mallocMC to alpaka, I replaced a lot of atomic operations in CUDA by the alpaka equivalents. This made the code a lot verboser:

const uint32 old = atomicOr(bitfield, mask);

changes to:

const uint32 old = alpaka::atomic::atomicOp<alpaka::atomic::op::Or>(acc, bitfield, mask);

What is the reason for not providing a straight forward:

const uint32 old = alpaka::atomicOr(acc, bitfield, mask);

?

@sbastrakov
Copy link
Member

I would like it. Guess just with atomic:: namespace, like
const uint32 old = alpaka::atomic::atomicOr(acc, bitfield, mask);

The duplication between namespace name and function name is what may also be discussed, but now it is very consistently done in alpaka.

@BenjaminW3
Copy link
Member

I also think that a shortcut atomicOr would be possible.

@ax3l
Copy link
Member

ax3l commented May 29, 2020

👍 for more using aliases.

Note that we might want this mimic this syntax towards C++20:

Due to the very explicit naming: could we make alpaka::atomic a C++ inline namespace?

@sbastrakov
Copy link
Member

Perhaps we can, and also with many other namespaces of alpaka as lots of classes or functions have the same prefix in their self name as the namespace they are in,

@psychocoderHPC
Copy link
Member

I suggest if we introduce alpaka::atomic::atomicOr to remove the usage of pointer in the interface and use references even if this is different to CUDA or HIP. Using pointer would would normally require to check that no nullptr is passed to the function.

@BenjaminW3 BenjaminW3 added the Priority: 3 Medium label Jul 28, 2020
@bernhardmgruber
Copy link
Member Author

With #1155 the current syntax simplifies to:

const uint32 old = alpaka::atomicOp<alpaka::op::Or>(acc, bitfield, mask);

bernhardmgruber added a commit to bernhardmgruber/alpaka that referenced this issue Oct 27, 2020
bernhardmgruber added a commit to bernhardmgruber/alpaka that referenced this issue Oct 27, 2020
bernhardmgruber added a commit to bernhardmgruber/alpaka that referenced this issue Oct 28, 2020
sbastrakov pushed a commit that referenced this issue Oct 28, 2020
@bernhardmgruber
Copy link
Member Author

This is largely resolved by the namespace refactoring #1034 and explicitely spelled atomic funtions in PR #1185. The final functor rename in #1186 will probably not be visible to users anymore.

@bernhardmgruber
Copy link
Member Author

This is resolved with #1185.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants