[SYCL] Implemented SYCL 2020 sub-group size functionality. #3444
Conversation
As specified here: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/SubGroup/SYCL_INTEL_sub_group.asciidoc#attributes This patch implements the named_sub_group_size attribute as well as the command line parameter, and creates a new spelling of reqd_sub_group_size (sub_group_size) to work like the SYCL 2020 version.
|
Note that the IR is not finalized, so I have a tentative version for that. I also lack tests as I'd like to get some thoughts on the design/diagnostics ahead of time. I'll in parallel work on adding tests, but any comments you can provide in advance are appreciated! |
|
Ugh... can't believe I didn't notice this, but 'auto' as an identifer here is going to require a bunch of work.... it'll be incoming :) |
|
The enum-patch here is going to be time consuming, so that'll take a while. But other than that comment (and getting 'auto' to work!) I think I have everything in my local repo. |
The feature architect agreed to change auto to automatic due to implementation issues. Also changed over to EnumArgument.
… to work, plus some additional tests
Currently the discussion on the design has lead me to believe that writing this patch is premature. Some of the in-flight changes to the spec will severely change what this implementation looks like, so we're going to put this patch on hold until the spec settles. This is hopefully in a reasonable enough position where we can pick it up later without trouble.
|
Thank you to @smanna12 and @AaronBallman for the reviews! I'm considering us 'done' with this patch for a while, so don't bother reviewing the last commit until we get a more concrete direction. Thanks! |
Still need to do the code gen, but that is broken at least until the rebase on intel#3475, which is awaiting merge.
|
I think I got it all :) I also removed the 'mergeSYCLSimd' bits as I think you meant that as well. Currently trying to survive the merge, which is looking a little rough as well, so it'll be a bit before you get to see it all :) |
|
Whew... that should hopefully do it :) I likely need another review from @mdtoguchi due to the driver changes, plus @AaronBallman or @premanandrao for the CFE part. |
|
It looks like there are some SYCL runtime test failures related to the changes that still need to be addressed? |
|
Yikes, I missed that! Thanks Aaron! |
Really we should probably be checking this, but since it is a pattern that is apparently commonly used, AND is a breaking change, we won't enforce this rule. The standard doesn't require us to (since it is pretty silent on mixing attribute types).
|
Hah, looks like @mdtoguchi moved the code I modified for the driver! I'll do a merge commit, which should fix that and hopefully not run into the flakey problems i've seen here. |
|
In all honesty, I wasn't expecting my code to be merged before yours! |
|
Just blocked by @mdtoguchi at the moment :) Otherwise i seem to have finally made it through the buildbot guantlet. |
bader
changed the title
As specified here:
https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/SubGroup/SYCL_INTEL_sub_group.asciidoc#attributes
This patch implements the named_sub_group_size attribute as well as the
command line parameter, and creates a new spelling of
reqd_sub_group_size (sub_group_size) to work like the SYCL 2020 version.