[SYCL][CUDA][libclc] Add asynchronous barrier

[t4c1]

Adds extension proposal and implementation for asynchronous barrier (for now the implementation is for CUDA backend sm 80+ only).

Tests for this are here: intel/llvm-test-suite#737

[Pennycook]

This looks like a useful extension. Most of my comments are about making it more obvious that it's CUDA-specific to help set user expectations.

Split barriers are useful, and we (Intel) are interested in exposing them for other hardware. Cm already exposes some support for split barriers on Intel GPUs via its cm_sbarrier function; we should think about what a portable split barrier interface would look like.

[t4c1]

Cm already exposes some support for split barriers on Intel GPUs via its cm_sbarrier function;

I did not find anything about that. Can you point me to either extension doc or the implementation?

we should think about what a portable split barrier interface would look like.

Currently this extension exposes all functionality from CUDA. However I am not sure if we need to require all this functionality from all implementations. For example I would not count no_complete variants of the arrive operation or copy_async_arrive as the most essential parts of the barrier.

Getting some input from whoever designed cm_sbarrier would also be nice.

[Pennycook]

I did not find anything about that. Can you point me to either extension doc or the implementation?

I can't find a good resource, either. This isn't a SYCL extension, but you might be able to glean some details from the CM compiler: https://github.com/intel/cm-compiler/search?q=sbarrier

Currently this extension exposes all functionality from CUDA. However I am not sure if we need to require all this functionality from all implementations. For example I would not count no_complete variants of the arrive operation or copy_async_arrive as the most essential parts of the barrier.

Personally, I think it would be cleaner to have two extensions to support this case: one CUDA-backend-specific extension that gives access to capabilities only supported by CUDA (but that may be useful to those writing SYCL and tuning some sections of their code for NVIDIA devices); and another, more general extension that exposes split barriers in a portable way.

Providing a class that works everywhere but has member functions that only work on some devices has the potential to be confusing.

[smaslov-intel]

does this really need to have that "reach" interface compared to std::barrier that it draws similarity with?

[t4c1]

I am not sure what do you mean by "reach interface".

[smaslov-intel]

I mean that std::barrier is happy with just arrive/wait/drop, and no token. Can we simplify this API (it's likely will add chances that other backends would be able to support it too)?

[t4c1]

While removing this token would make interface more similar to std::barrier, I disagree that doing so would make it easier to implement for other backends. If other backends do not need this token they can pass around a dummy value. Meanwhile implementing this without the token for CUDA would lead to more complicated implementation and most likely additional limitations, such as only one barrier being usable at once.

[smaslov-intel]

If other backends do not need this token they can pass around a dummy value.

But the token is used as an input in the core wait API, it can't be dummy.

Meanwhile implementing this without the token for CUDA would lead to more complicated implementation and most likely additional limitations, such as only one barrier being usable at once.

CUDA implementation can use "token" under the hood, of course.

I don't have any practical suggestions, just expressing a desire for a simpler and a more standard interface for the feature. I will rely on @Pennycook review/approval for the feature definition and then review the implementation.

[t4c1]

But the token is used as an input in the core wait API, it can't be dummy.

I guess I was not clear enough. I meant that an implementation that does need the token can return a dummy value from arrive and ignore that token in the wait call.

CUDA implementation can use "token" under the hood, of course.

I have a feeling that hiding the token under the hood would, depending on how it is implemented, either limit the functionality, or be inefficient or complicated. Although I can't provide any concrete arguments from the top of my head. I need to think about this some more.

[Pennycook]

I don't quite agree to that, but OK. should the name of the extension be "ext_cuda_async_barrier" instead of "ext_oneapi_cuda_async_barrier" then?

The SYCL 2020 naming conventions for extensions encourages "sycl_ext_<vendorstring>_<featurename>" , where the vendor string is used to avoid collisions. Our vendor strings are "oneapi" and "intel".

"ext_oneapi_cuda_" is supposed to convey that it's a oneAPI extension, but specific to the CUDA backend.

[t4c1]

I guess now this has all the approvals needed for merge?

[steffenlarsen]

I guess now this has all the approvals needed for merge?

I believe we need @pvchupin's approval as well.

[pvchupin]

@gmlueck, can you review/approve please?

[gmlueck]

@gmlueck, can you review/approve please?

@Pennycook has been taking the lead on reviewing the spec for this, and I trust his judgement. I happened to notice a small mistake when I read through, though. I'm happy when that is fixed.

[t4c1]

@pvchupin @smaslov-intel @Pennycook The last change in this PR was minimal and this was previously approved, so can we get this approved and merged?

[pvchupin]

@t4c1, please fix post commit issues: https://github.com/intel/llvm/runs/6478611698

[pvchupin]

@t4c1, please check also LIT fail on windows: https://github.com/intel/llvm/runs/6478612314?check_suite_focus=true
Also fails in post-commit of this PR.