Skip to content

[SYCL] [FPGA] Fix num_simd_work_items and reqd_work_group_size argument check #3728

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 21 commits into from
May 16, 2021
Merged

[SYCL] [FPGA] Fix num_simd_work_items and reqd_work_group_size argument check #3728

merged 21 commits into from
May 16, 2021

Conversation

smanna12
Copy link
Contributor

@smanna12 smanna12 commented May 11, 2021 •
edited
Loading

The arguments to reqd_work_group_size are ordered based on which index
increments the fastest. In OpenCL, the first argument is the index that
increments the fastest, and in SYCL, the last argument is the index that
increments the fastest.

In OpenCL, all three arguments are required.

In SYCL, the attribute accepts either one, two, or three arguments; in each
form, the last (or only) argument is the index that increments fastest.
The number of arguments passed to the attribute must match the dimensionality
of the kernel the attribute is applied to.

If the reqd_work_group_size attribute is specified on a declaration along
with num_simd_work_items, the required work group size specified
by num_simd_work_items attribute must evenly divide the index that
increments fastest in the reqd_work_group_size attribute.

This patches fixes the problem on #3275 where we checked the wrong argument for SYCL and OpenCL attributes.

Signed-off-by: Soumi Manna soumi.manna@intel.com

@smanna12
Fix Bug
68f33f2 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 changed the title Fix Bug [SYCL] [Do not Review] Fix num_simd_work_items and reqd_work_group_size argument check May 11, 2021
smanna12 added 9 commits May 11, 2021 15:13
@smanna12
Fix clang format issues
704696a 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
Merge remote-tracking branch 'my_remote/sycl' into FixArgumentOrder
6a37844
@smanna12
Update patch
a58fa56 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
Fix clang format issues
b83b6ad 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
check null values for reqd_work_group_size attribute
e23fc8b 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
Fix clang format issues
b163449 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
update patch
e9d7812 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
update patch
9c8643c 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
Update patch and add template cases
128a7f9 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 changed the title [SYCL] [Do not Review] Fix num_simd_work_items and reqd_work_group_size argument check [SYCL] [FPGA] Fix num_simd_work_items and reqd_work_group_size argument check May 12, 2021
@smanna12 smanna12 requested a review from AaronBallman May 12, 2021 12:53
@smanna12 smanna12 marked this pull request as ready for review May 12, 2021 13:08
AaronBallman
AaronBallman reviewed May 12, 2021
View reviewed changes
AaronBallman
AaronBallman reviewed May 12, 2021
View reviewed changes
@smanna12 smanna12 mentioned this pull request May 12, 2021
Open
smanna12 added 3 commits May 12, 2021 17:23
@smanna12
address review comments
e4bc13f 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
update doc
4d5363c 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
fix typos
8dbf4e2 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 requested a review from AaronBallman May 13, 2021 13:03
AaronBallman
AaronBallman reviewed May 13, 2021
View reviewed changes
smanna12 added 2 commits May 13, 2021 10:36
@smanna12
address review comments
ddd73e9 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
fix format issues
95579a5 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 requested a review from AaronBallman May 14, 2021 13:49
smanna12 added 3 commits May 14, 2021 10:26
@smanna12
GNU attribute uses SYCL arguments ordering in SYCL mode and OpenCL or…
fa4e4d7 
…dering in OPenCL mode

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
fix calng formt issues
0057e9a 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
update doc
c4e8cc1 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
AaronBallman
AaronBallman reviewed May 14, 2021
View reviewed changes
smanna12 added 2 commits May 14, 2021 11:55
@smanna12
address review comments
bcd55f2 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12
update comments
1464c30 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
elizabethandrews
elizabethandrews reviewed May 14, 2021
View reviewed changes
@smanna12
Address review comments
9526b18 
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
AaronBallman
AaronBallman approved these changes May 14, 2021
View reviewed changes
Copy link
Contributor

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM!

@smanna12 smanna12 mentioned this pull request May 14, 2021
Open
@smanna12 smanna12 requested a review from elizabethandrews May 14, 2021 19:38
@smanna12
Copy link
Contributor Author

LGTM!

Thanks @AaronBallman for the reviews and the discussion.

elizabethandrews
elizabethandrews approved these changes May 14, 2021
View reviewed changes
Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

Thanks!

@smanna12
Copy link
Contributor Author

Thanks!

@elizabethandrews, thanks for the reviews.

@smanna12
Copy link
Contributor Author

smanna12 commented May 15, 2021 •
edited
Loading

@bader, do you know why buildbot/Lit_With_Cuda is stuck? It has not been started for a day now on my other PR #3326

@bader
Copy link
Contributor

bader commented May 15, 2021

@bader, do you know why buildbot/Lit_With_Cuda is stuck? It has not been started for a day now on my other PR #3326

I don't know, CI team should be able to answer this, but I see it's started already.
NOTE: each push to remote repository triggers new set of pre-commit tests, so I recommend to keep the number of pushes as low as possible to avoid unnecessary load on CI system.

@smanna12
Copy link
Contributor Author

smanna12 commented May 15, 2021 •
edited
Loading

@bader, do you know why buildbot/Lit_With_Cuda is stuck? It has not been started for a day now on my other PR #3326

I don't know, CI team should be able to answer this, but I see it's started already.
NOTE: each push to remote repository triggers new set of pre-commit tests, so I recommend to keep the number of pushes as low as possible to avoid unnecessary load on CI system.

Thanks @bader for the recommendation. All testing has finished successfully. Could you please merge this PR when you are able? This affects FPGA emulator. Thanks.

@bader bader merged commit 94a7dd8 into intel:sycl May 16, 2021
@smanna12 smanna12 mentioned this pull request Feb 27, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmlueck gmlueck gmlueck left review comments

@AaronBallman AaronBallman AaronBallman approved these changes

@elizabethandrews elizabethandrews elizabethandrews approved these changes

@premanandrao premanandrao Awaiting requested review from premanandrao

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@smanna12 @bader @gmlueck @AaronBallman @elizabethandrews