[FPGA][SYCL] Fix max_work_group_size and reqd_work_group_size attribute arguments check #5592

smanna12 · 2022-02-16T13:49:33Z

If the [[intel::max_work_group_size(X, Y, Z)]] attribute is specified on
a declaration along with [[sycl::reqd_work_group_size(X1, Y1, Z1)]]
attribute, this patch checks if values of reqd_work_group_size arguments are
equal or less than values of max_work_group_size attribute arguments.

Some of the test cases were missed during refactoring work with PGA function
attribute [[intel::max_work_group_size()]] on #5392

This patch adds the missing cases and fixes pulldown bug.

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

…te arguments check If the [[intel::max_work_group_size(X, Y, Z)]] attribute is specified on a declaration along with [[sycl::reqd_work_group_size(X1, Y1, Z1)]] attribute, this patch checks if values of reqd_work_group_size arguments are equal or less than values of max_work_group_size attribute arguments. Some of the test cases were missed during refactoring work with PGA function attribute [[intel::max_work_group_size()]] on intel#5392 This patch fixes them. Signed-off-by: Soumi Manna <soumi.manna@intel.com>

smanna12 · 2022-02-18T16:23:20Z

This PR blocks downstream pulldown bug. Can i get a review @AaronBallman , @premanandrao ? Thank you

clang/lib/Sema/SemaDeclAttr.cpp

clang/test/SemaSYCL/intel-max-work-group-size.cpp

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

clang/lib/Sema/SemaDeclAttr.cpp

clang/test/SemaSYCL/intel-max-work-group-size.cpp

clang/test/SemaSYCL/redeclaration-attribute-propagation.cpp

clang/test/SemaSYCL/reqd-work-group-size-device-direct-prop.cpp

clang/test/SemaSYCL/reqd-work-group-size-device.cpp

clang/lib/Sema/SemaDeclAttr.cpp

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

clang/lib/Sema/SemaDeclAttr.cpp

clang/test/SemaSYCL/intel-max-global-work-dim-device.cpp

clang/test/SemaSYCL/intel-max-work-group-size.cpp

clang/lib/Sema/SemaDeclAttr.cpp

premanandrao

@elizabethandrews raised several good points in her recent review; I would like to review again after you have had a chance to address those.

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

clang/lib/Sema/SemaDeclAttr.cpp

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

AaronBallman

LGTM as far as I can see

smanna12 · 2022-03-01T18:05:59Z

LGTM as far as I can see

Thanks @AaronBallman for the reviews!

smanna12 · 2022-03-01T19:11:42Z

@elizabethandrews and @premanandrao, do you have any more concerns?

clang/lib/Sema/SemaDeclAttr.cpp

premanandrao · 2022-03-01T20:13:49Z

@elizabethandrews and @premanandrao, do you have any more concerns?

I just had one nit, but you could do this in a later PR if there are no other review concerns.

clang/test/SemaSYCL/intel-max-work-group-size.cpp

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

smanna12 · 2022-03-02T21:41:45Z

@elizabethandrews, any more concerns from your side?

smanna12 · 2022-03-02T23:16:52Z

The test failures of L0 GEN9 LLVM Test Suite are not related to my patch:
https://github.com/intel/llvm/runs/5398454333?check_suite_focus=true

Failed Tests (8):
SYCL :: Basic/host_platform_avail.cpp
SYCL :: Basic/intel-ext-device.cpp
SYCL :: Basic/interop/get_native_ze.cpp
SYCL :: Basic/query_emulate_subdevice.cpp
SYCL :: Plugin/sycl-ls-gpu-default-any.cpp
SYCL :: Plugin/sycl-ls-gpu-level-zero.cpp
SYCL :: Plugin/sycl-ls.cpp
SYCL :: Regression/device_num.cpp

I am seeing same failures on #5576

elizabethandrews

LGTM! Thank you

smanna12 · 2022-03-02T23:34:33Z

Thank you everyone for the reviews!

smanna12 changed the title ~~[FPGA][SYCL] Fix max_work_group_size and reqd_work_group_size attribu…~~ [FPGA][SYCL] Fix max_work_group_size and reqd_work_group_size attribute arguments check Feb 16, 2022

smanna12 marked this pull request as ready for review February 16, 2022 13:51

smanna12 requested a review from a team as a code owner February 16, 2022 13:51

smanna12 requested review from AaronBallman and premanandrao February 16, 2022 13:51

AaronBallman reviewed Feb 18, 2022

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

elizabethandrews reviewed Feb 18, 2022

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Show resolved Hide resolved

elizabethandrews reviewed Feb 18, 2022

View reviewed changes

clang/test/SemaSYCL/intel-max-work-group-size.cpp Show resolved Hide resolved

smanna12 added 3 commits February 18, 2022 11:44

address review comments

651869c

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

Remove redundant change

369ae6b

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

Merge remote-tracking branch 'my_remote/sycl' into Fix_Bug_Max_Reqd_Attr

3017916

smanna12 requested review from AaronBallman and elizabethandrews February 22, 2022 12:54

address review comments

5b68889

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

elizabethandrews reviewed Feb 23, 2022

View reviewed changes

premanandrao reviewed Feb 23, 2022

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

smanna12 added 4 commits February 23, 2022 10:24

Address review comments

e7570fc

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

add reqd_work_group_size spellings and orderings

2cb7c5c

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

Fix formats

5413321

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

Fix formats

238abe1

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

smanna12 commented Feb 25, 2022

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Show resolved Hide resolved

smanna12 requested a review from GarveyJoe February 25, 2022 20:08

elizabethandrews reviewed Feb 25, 2022

View reviewed changes

premanandrao reviewed Feb 25, 2022

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

premanandrao reviewed Feb 25, 2022

View reviewed changes

smanna12 added 3 commits February 27, 2022 07:25

Address review comments

05e1303

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

Update comments

fcc2654

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

add comments for different spellings of reqqd_work_group attribute

741832e

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

smanna12 added 2 commits February 28, 2022 06:22

Merge remote-tracking branch 'my_remote/sycl' into Fix_Bug_Max_Reqd_Attr

67c6b60

Merge remote-tracking branch 'my_remote/sycl' into Fix_Bug_Max_Reqd_Attr

15f47a5

smanna12 requested review from premanandrao and elizabethandrews February 28, 2022 18:46

AaronBallman reviewed Mar 1, 2022

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Show resolved Hide resolved

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

smanna12 added 2 commits March 1, 2022 09:24

Fix Coding style guideline

a8c96aa

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

Fix Coding style guideline

154d81f

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

AaronBallman previously approved these changes Mar 1, 2022

View reviewed changes

smanna12 mentioned this pull request Mar 1, 2022

Do we allow user to use OpenCL and SYCL at same time #5700

Closed

premanandrao reviewed Mar 1, 2022

View reviewed changes

clang/lib/Sema/SemaDeclAttr.cpp Outdated Show resolved Hide resolved

elizabethandrews reviewed Mar 1, 2022

View reviewed changes

clang/test/SemaSYCL/intel-max-work-group-size.cpp Outdated Show resolved Hide resolved

Fix test

6f224d0

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

smanna12 dismissed AaronBallman’s stale review via 6f224d0 March 2, 2022 15:39

remove exrtra space

4fb6c9b

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

smanna12 requested review from premanandrao and elizabethandrews March 2, 2022 15:43

smanna12 added 2 commits March 2, 2022 08:42

address review comments and format issues

330fc3b

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

Fix Format errors

f9406ee

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

premanandrao approved these changes Mar 2, 2022

View reviewed changes

Merge remote-tracking branch 'my_remote/sycl' into Fix_Bug_Max_Reqd_Attr

e45f09d

elizabethandrews approved these changes Mar 2, 2022

View reviewed changes

Fznamznon mentioned this pull request Mar 3, 2022

[SYCL] Emit integration footer and header for device_global variables #5576

Merged

bader merged commit 7f37250 into intel:sycl Mar 3, 2022

smanna12 deleted the Fix_Bug_Max_Reqd_Attr branch March 3, 2022 13:15

[FPGA][SYCL] Fix max_work_group_size and reqd_work_group_size attribute arguments check #5592

[FPGA][SYCL] Fix max_work_group_size and reqd_work_group_size attribute arguments check #5592

Uh oh!

Conversation

smanna12 commented Feb 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smanna12 commented Feb 18, 2022

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!

premanandrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

smanna12 commented Mar 1, 2022

Uh oh!

smanna12 commented Mar 1, 2022

Uh oh!

Uh oh!

premanandrao commented Mar 1, 2022

Uh oh!

Uh oh!

smanna12 commented Mar 2, 2022

Uh oh!

smanna12 commented Mar 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elizabethandrews left a comment

Choose a reason for hiding this comment

Uh oh!

smanna12 commented Mar 2, 2022

Uh oh!

Uh oh!

smanna12 commented Feb 16, 2022 •

edited

Loading

smanna12 commented Mar 2, 2022 •

edited

Loading