Skip to content

[SYCL][FPGA] Refactor of [[intel::max_global_work_dim()]] attribute #3326

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 35 commits into from
May 27, 2021

Conversation

smanna12
Copy link
Contributor

@smanna12 smanna12 commented Mar 9, 2021

This patch

  1. refactors FPGA function attribute [[intel::max_global_work_dim()]]
    to better fit for community standards.

  2. refactors the way we handled duplicate attributes and
    mutually exclusive attributes logic when present on a given declaration.

  3. handles redeclarations or template instantiations properly.

  4. adds tests

  5. Before the refactoring patch, we silently accepted this test case below:

    struct TRIFuncObjBad {
    [[intel::max_work_group_size(8, 8, 8)]]
    [[cl::reqd_work_group_size(4, 4, 4)]]
    [[intel::max_global_work_dim(0)]]
    void operator()() const {}
    };

This PR fixes the problem.

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

smanna12 added 2 commits March 8, 2021 20:29
This patch

1. refactors FPGA function attribute using intel#3224:
   [[intel::max_global_work_dim()]] attribute to better fit for community standards.
2. refactors the way we handle duplicate attributes and
   mutually exclusive attributes logic when present on a given declaration.
3. handles redeclarations or template instantiations properly.
4. adds tests
5. adds new test cases where the value of 'max_global_work_dim' attribute
   equals to 0, we shall ensure that if max_work_group_size and
   reqd_work_group_size attributes exist, they hold equal values (1, 1, 1).

   Before the new refactoring patch, we silently accepeted this test case:

   struct TRIFuncObjBad {
     [[intel::max_work_group_size(8, 8, 8)]]
     [[cl::reqd_work_group_size(4, 4, 4)]]
     [[intel::max_global_work_dim(0)]]
      void operator()() const {}
   };

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 requested a review from AaronBallman March 9, 2021 14:09
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
AaronBallman
AaronBallman previously approved these changes Mar 9, 2021
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!

Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 marked this pull request as ready for review March 9, 2021 20:05
@smanna12 smanna12 requested a review from AaronBallman March 10, 2021 18:09
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 requested a review from AaronBallman March 11, 2021 03:01
@smanna12
Copy link
Contributor Author

smanna12 commented Mar 20, 2021

Can i get a review on the patch? Thanks

smanna12 added 2 commits May 13, 2021 13:54
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 marked this pull request as ready for review May 13, 2021 21:10
@smanna12 smanna12 changed the title [SYCL][FPGA] [Do not Review] Refactor of [[intel::max_global_work_dim()]] attribute [SYCL][FPGA] Refactor of [[intel::max_global_work_dim()]] attribute May 13, 2021
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 requested a review from AaronBallman May 13, 2021 21:17
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 requested a review from premanandrao May 14, 2021 17:41
smanna12 added 3 commits May 24, 2021 09:07
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
smanna12 added 3 commits May 25, 2021 06:33
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
@smanna12 smanna12 requested a review from AaronBallman May 25, 2021 13:57
smanna12 added 2 commits May 26, 2021 08:02
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
smanna12 added 2 commits May 26, 2021 10:07
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
Signed-off-by: Soumi Manna <soumi.manna@intel.com>
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, thank you!

@smanna12
Copy link
Contributor Author

Thank you for the reviews.

@bader bader merged commit 9b61592 into intel:sycl May 27, 2021
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.

4 participants