Skip to content

[SYCL] Default work-group sizes based on max #952

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

Merged
merged 2 commits into from
Mar 25, 2020

Conversation

nyalloc
Copy link
Contributor

@nyalloc nyalloc commented Dec 19, 2019

These changes ensures that the default work-group size does not exceed the allowed maximum sizes.

Signed-off-by: Stuart Adams stuart.adams@codeplay.com

Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

Thanks for working on this fix.
Could you a regression test, please?

@@ -681,15 +681,12 @@ static void adjustNDRangePerKernel(NDRDescT &NDR, RT::PiKernel Kernel,

if (WGSize[0] == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this condition valid to represent all "default work-group size" cases?

Copy link
Contributor

@romanovvlad romanovvlad Mar 2, 2020

Choose a reason for hiding this comment

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

WGSize[0] == 0 means user hasn't asked for a specific local size using attribute((reqd_work_group_size(X, Y, Z)) only.

@romanovvlad
Copy link
Contributor

@StuartDAdams
ping

vladimirlaz pushed a commit to vladimirlaz/llvm that referenced this pull request Feb 7, 2020
  CONFLICT (content): Merge conflict in clang/lib/Frontend/CompilerInvocation.cpp
@bader
Copy link
Contributor

bader commented Feb 28, 2020

@StuartDAdams, ping.

@steffenlarsen
Copy link
Contributor

I have responded to some of the concerns and I agree with @AlexeySachkov that this may not be the most stable solution. I've suggested a different solution and will do regression tests once we reach a conclusion on what to do.

@pvchupin
Copy link
Contributor

pvchupin commented Feb 28, 2020

@bader, @romanovvlad can you please review and comment on the latest approach. Please summarize what you think needs to be addressed.

@pvchupin pvchupin assigned bader and romanovvlad and unassigned bader and AlexeySachkov Feb 28, 2020
@pvchupin
Copy link
Contributor

pvchupin commented Mar 6, 2020

@romanovvlad please review

@bader bader requested a review from romanovvlad March 22, 2020 10:58
@nyalloc nyalloc force-pushed the stuart/default-work-group-size branch from 9897d54 to 247634f Compare March 24, 2020 10:13
@nyalloc nyalloc force-pushed the stuart/default-work-group-size branch from 4439522 to f61ebb5 Compare March 24, 2020 11:47
@nyalloc nyalloc requested a review from romanovvlad March 24, 2020 13:13
romanovvlad
romanovvlad previously approved these changes Mar 24, 2020
@bader
Copy link
Contributor

bader commented Mar 24, 2020

@StuartDAdams, it looks like I merged some conflicting changes recently. Could you update the RP, please?

@nyalloc
Copy link
Contributor Author

nyalloc commented Mar 24, 2020

@StuartDAdams, it looks like I merged some conflicting changes recently. Could you update the RP, please?

Of course, I will do this ASAP.

Signed-off-by: Stuart Adams <stuart.adams@codeplay.com>
@nyalloc nyalloc force-pushed the stuart/default-work-group-size branch from f61ebb5 to 3044baf Compare March 24, 2020 17:31
@nyalloc nyalloc requested a review from bader March 24, 2020 17:35
Signed-off-by: Stuart Adams <stuart.adams@codeplay.com>
@nyalloc nyalloc force-pushed the stuart/default-work-group-size branch from 4be2092 to 646c65c Compare March 24, 2020 17:39
@bader bader requested a review from romanovvlad March 24, 2020 17:41
@bader bader merged commit b48f08f into intel:sycl Mar 25, 2020
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Mar 27, 2020
…hinx

* upstream/sycl: (357 commits)
  [Support] Implement a simple tabular data management library (intel#1358)
  [Support] Implement a property set I/O library (intel#1357)
  [SYCL] Fix buffer constructor using iterators (intel#1386)
  [SYCL][FPGA] Enable a set of loop attributes (intel#1312)
  [Driver][SYCL][FPGA] Proper dependency output location when given /Fo<dir> (intel#1346)
  [SPIR-V] Enabling SPIR-V builtin lookup in device SYCL mode (intel#1384)
  [SYCL][NFC] Unify setting kernel arguments (intel#1379)
  [SYCL][Doc] First revision of standard layout relaxation extension (intel#1344)
  [SYCL] Fixed sub-buffer alloca search (intel#1385)
  [SYCL][FPGA] Emit multiple IR variants for the IVDep attribute (intel#1383)
  [SYCL] Add experimental flag to enable front-end optimizations (intel#1376)
  [SYCL] Remove unexpected double in complex SPIR-V for float support (intel#1381)
  [SYCL] Default work-group sizes based on max (intel#952)
  [SYCL][CUDA] Fix usage of multiple backends in the same program (intel#1252)
  [SPIR-V] Add SPIR-V builtin definitions to the builtin lookup.
  [SPIR-V] Add macro definition when -fdeclare-spirv-builtins is activated
  [SYCL] Fix sycl_generic printing
  [SYCL] Support intel::reqd_work_group_size (intel#1328)
  [SYCL][NFC] Make the RT::PiPlugin object private (intel#1375)
  [SPIRV] Add convergent attribute to SPIR-V built-ins (intel#1373)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants