[SYCL][Doc] Cluster Group Extension Document by AD2605 · Pull Request #13594 · intel/llvm

AD2605 · 2024-04-30T06:06:03Z

Initial public working draft for thread block cluster support in SYCL, intended to get feedback.

Contains the proposal for -

Launching a kernel with cluster group
Accessing the various ids associated with the cluster_group from the kernel
Cluster level barrier
Accessing another workgroup's local memory

Ruyk

Thanks for the proposal, please also add an example (or two) of how it would be used

sycl/doc/extensions/proposed/sycl_ext_oneapi_work_group_cluster.asciidoc

Ruyk · 2024-04-30T11:37:20Z

sycl/doc/extensions/proposed/sycl_ext_oneapi_work_group_cluster.asciidoc

+
+|`size_t get_local_linear_range(void)`
+|Returns the total number of `workItems` within the cluster
+|===


all the above methods should be const right?

Thanks for highlighting this, fixed

sycl/doc/extensions/proposed/sycl_ext_oneapi_work_group_cluster.asciidoc

jbrodman · 2024-04-30T13:49:37Z

Note: This direction is not approved. Do not merge this as the APIs and abstractions are not final.

AerialMantis

Thanks for putting it together, overall this looks really good, I've made a couple of design suggestions.

I would also suggest adding a synopsis which shows the whole cluster_group class, including namespaces, this helps to see everything together, and it would be good to have an example of how this would be used, this helps people understand how everything fits together.

sycl/doc/extensions/proposed/sycl_ext_oneapi_work_group_cluster.asciidoc

AD2605 · 2024-05-09T11:20:34Z

Hi all, thanks for the initial review, I have addressed the comments (except one regarding copyright notice, I am yet to add that) and this now ready for the re-review

CC: @jbrodman

sycl/doc/extensions/proposed/sycl_ext_oneapi_cuda_cluster_group.asciidoc

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

…ress guarantees

…r for cluster group class

sycl/doc/extensions/proposed/sycl_ext_oneapi_cuda_cluster_group.asciidoc

EwanC · 2024-06-26T09:22:01Z

sycl/doc/extensions/proposed/sycl_ext_oneapi_cuda_cluster_group.asciidoc

+
+|`cluster_group<Dimensions> nd_item::ext_oneapi_cuda_get_cluster_group()`
+|Returns the constituent `cluster_group` in the kernel, representing this
+`cluster_group`'s overall position in the `nd_range`


This table renders slightly strangely

Might want this as the column headers in bold, also the monospace isn't quite right.

|Method |Description

This should also be added to a couple other tables in this doc.

I should be better now, and removed the accidental "boldness" of the phrase.

Added "method, description" in other places too

sycl/doc/extensions/proposed/sycl_ext_oneapi_cuda_cluster_group.asciidoc

Co-authored-by: Gordon Brown <gordon@codeplay.com>

…p.asciidoc Co-authored-by: Gordon Brown <gordon@codeplay.com>

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

…ues section

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

…r_fence

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc

sycl/doc/extensions/proposed/sycl_ext_oneapi_cuda_cluster_group.asciidoc

…roup_linear_id_for_local_pointer

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc

sycl/doc/extensions/proposed/sycl_ext_oneapi_cuda_cluster_group.asciidoc

Address feedback Co-authored-by: Gordon Brown <gordon@codeplay.com>

…r_group_linear_id_for_local_pointer

AerialMantis

Looks good!

AD2605 · 2024-07-30T14:11:36Z

@intel/llvm-gatekeepers This is ready to Merge. Thanks

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc

…oup.asciidoc

Ruyk

Minor comment about (C)

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc

Co-authored-by: Ruyman <ruyman@codeplay.com>

Ruyk · 2024-08-01T12:10:02Z

LGTM

Initial public working draft for thread block cluster support in SYCL, intended to get feedback. Contains the proposal for - 1. Launching a kernel with cluster group 2. Accessing the various `ids` associated with the cluster_group from the kernel 3. Cluster level barrier 4. Accessing another workgroup's local memory --------- Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com> Co-authored-by: Gordon Brown <gordon@codeplay.com> Co-authored-by: John Pennycook <john.pennycook@intel.com> Co-authored-by: Ruyman <ruyman@codeplay.com>

initial public working draft for cluster_group

5ca51dd

AD2605 requested a review from a team as a code owner April 30, 2024 06:06

AD2605 marked this pull request as draft April 30, 2024 06:07

Ruyk suggested changes Apr 30, 2024

View reviewed changes

AerialMantis reviewed Apr 30, 2024

View reviewed changes

JackAKirk mentioned this pull request Apr 30, 2024

[SYCL] Implement forward_progress extension API #13389

Merged

AD2605 added 2 commits May 9, 2024 11:57

review comments 1

08dc77d

run ispell

70790e0

AD2605 requested review from AerialMantis and Ruyk May 9, 2024 11:05

AD2605 added 3 commits May 9, 2024 14:01

add copyright notice

f0403d1

add glossary to explain compute capability acronym

c3182cc

add missing queue parameter in example

cc78630

gmlueck reviewed May 21, 2024

View reviewed changes

AD2605 and others added 4 commits May 24, 2024 07:54

change linear_id to linear_id_type

d660749

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

Change "should" to "must"

13b410b

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

template ClusterRange and add Known issues section about forward prog…

19b0c52

…ress guarantees

change return type of id queries to size_t and change confusing wording

fa5d894

AD2605 mentioned this pull request Jun 10, 2024

[SYCL] [ABI-Break] Partial implementation of sycl_ext_oneapi_cuda_cluster_group #14113

Merged

AD2605 added 3 commits June 14, 2024 09:28

rename ClusterRange to cluster_size

fb5ca79

Add memory fence, atomics, distributed shared memory and group barrie…

e510708

…r for cluster group class

add line on ensuring start and exit of workgroups

f418e3a

AD2605 marked this pull request as ready for review June 20, 2024 14:45

AerialMantis reviewed Jun 25, 2024

View reviewed changes

sycl/doc/extensions/proposed/sycl_ext_oneapi_cuda_cluster_group.asciidoc Outdated Show resolved Hide resolved

EwanC reviewed Jun 26, 2024

View reviewed changes

AerialMantis reviewed Jun 26, 2024

View reviewed changes

AD2605 and others added 2 commits June 26, 2024 18:17

change linear to flattened

5874ec5

Co-authored-by: Gordon Brown <gordon@codeplay.com>

Update sycl/doc/extensions/proposed/sycl_ext_oneapi_cuda_cluster_grou…

b3f9d24

…p.asciidoc Co-authored-by: Gordon Brown <gordon@codeplay.com>

gmlueck reviewed Jul 25, 2024

View reviewed changes

AD2605 and others added 6 commits July 26, 2024 11:24

remove extra `

31f5cdf

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

add missing closing brace

dcc9a2e

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

not refer a C++ type as plural

16e22e0

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

remove mistakenly copied lines, moved fence to the earlier synopsis

f54224f

Add comment about not differentiating address spaces in the known iss…

f544043

…ues section

change the example to confirm with the BKM

7bc66b2

gmlueck reviewed Jul 26, 2024

View reviewed changes

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc Outdated Show resolved Hide resolved

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc Outdated Show resolved Hide resolved

AD2605 and others added 3 commits July 26, 2024 14:19

remove duplicate another

470cbdb

Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>

restore ext_codeplay_cuda_cluster_group from ext_codeplay_cuda_cluste…

64a9384

…r_fence

fix left out occurence of cuda_cluster_fence

3ebeb12

gmlueck approved these changes Jul 26, 2024

View reviewed changes

AerialMantis reviewed Jul 26, 2024

View reviewed changes

rename get_linear_cluster_group_id_for_local_pointer to get_cluster_g…

48d6888

…roup_linear_id_for_local_pointer

AerialMantis reviewed Jul 26, 2024

View reviewed changes

AD2605 and others added 3 commits July 29, 2024 06:49

Apply suggestions from code review

c5916c6

Address feedback Co-authored-by: Gordon Brown <gordon@codeplay.com>

change syntax or Note, known issues, and change wording of get_cluste…

824af1e

…r_group_linear_id_for_local_pointer

remove bold text from Revision history

b68c406

AerialMantis approved these changes Jul 30, 2024

View reviewed changes

rolandschulz approved these changes Jul 30, 2024

View reviewed changes

AerialMantis reviewed Aug 1, 2024

View reviewed changes

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc Outdated Show resolved Hide resolved

Update sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_gr…

bf13172

…oup.asciidoc

Ruyk suggested changes Aug 1, 2024

View reviewed changes

sycl/doc/extensions/proposed/sycl_ext_codeplay_cuda_cluster_group.asciidoc Outdated Show resolved Hide resolved

Change Copyright from Intel to Codeplay

daa5380

Co-authored-by: Ruyman <ruyman@codeplay.com>

sommerlukas changed the title ~~[SYCL] Cluster Group Extension Document~~ [SYCL][Doc] Cluster Group Extension Document Aug 1, 2024

sommerlukas merged commit 20bcfea into intel:sycl Aug 1, 2024

Conversation

AD2605 commented Apr 30, 2024

Uh oh!

Ruyk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ruyk Apr 30, 2024

Choose a reason for hiding this comment

Uh oh!

AD2605 May 9, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jbrodman commented Apr 30, 2024

Uh oh!

AerialMantis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AD2605 commented May 9, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EwanC Jun 26, 2024

Choose a reason for hiding this comment

Uh oh!

AD2605 Jun 27, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!