Skip to content

[SYCL][FPGA] Drop the duplicate attribute when adding or merging #3389

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 4 commits into from
Mar 29, 2021
Merged

[SYCL][FPGA] Drop the duplicate attribute when adding or merging #3389

merged 4 commits into from
Mar 29, 2021

Conversation

smanna12
Copy link
Contributor

@smanna12 smanna12 commented Mar 22, 2021 •
edited
Loading

This patch drops a duplicate attribute when adding or merging for FPGA attributes below:

  1. [[intel::private_copies()]
  2. [[intel::max_replicates()]]
  3. [[intel::reqd_sub_group_size()]]
  4. [[intel::num_simd_work_items()]]
  5. [[intel::loop_fuse()]]
  6. [[intel::loop_fuse_independent()]]
  7. [[intel::no_global_work_offset()]]
  8. [[intel::scheduler_target_fmax_mhz()]]

Also adds tests for each of the attributes with duplicate arguments.

Fixes #3336

Signed-off-by: Soumi Manna [email protected]

smanna12 added 2 commits March 21, 2021 20:47
@smanna12
[SYCL][FPGA] Ignore duplicate attribute when adding or merging
50ae0fc 
This patch drops a duplicate attribute when adding or merging for FPGA
attributes:

1. [[intel::private_copies()]
2. [[intel::max_replicates()]]
3. [[intel::reqd_sub_group_size()]]
4. [[intel::num_simd_work_items()]]
5. [[intel::loop_fuse()]]
6. [[intel::loop_fuse_independent()]]
7. [[intel::no_global_work_offset()]]
8. [[intel::scheduler_target_fmax_mhz()]]

Also adds tests for each of the attributes with duplicate arguments.

Signed-off-by: Soumi Manna <[email protected]>
@smanna12
Fix format issues
539dbcf 
Signed-off-by: Soumi Manna <[email protected]>
@smanna12 smanna12 marked this pull request as ready for review March 22, 2021 04:14
@smanna12 smanna12 requested a review from AaronBallman March 22, 2021 04:14
@AaronBallman
Copy link
Contributor

I think in general we should not be calling dropAttr as in this patch -- better to not call addAttr in the first place (either by returning nullptr from the merge function or doing an early return from the add function).

@smanna12 smanna12 marked this pull request as draft March 24, 2021 03:17
@smanna12 smanna12 changed the title [SYCL][FPGA] Ignore duplicate attribute when adding or merging [SYCL][FPGA] [WIP]Ignore duplicate attribute when adding or merging Mar 24, 2021
@smanna12 smanna12 changed the title [SYCL][FPGA] [WIP]Ignore duplicate attribute when adding or merging [SYCL][FPGA] [Do NOT REVIEW]Ignore duplicate attribute when adding or merging Mar 26, 2021
@smanna12
address revie comments
e90a3b8 
Signed-off-by: Soumi Manna <[email protected]>
@smanna12 smanna12 changed the title [SYCL][FPGA] [Do NOT REVIEW]Ignore duplicate attribute when adding or merging [SYCL][FPGA]Ignore duplicate attribute when adding or merging Mar 29, 2021
@smanna12 smanna12 changed the title [SYCL][FPGA]Ignore duplicate attribute when adding or merging [SYCL][FPGA] Ignore duplicate attribute when adding or merging Mar 29, 2021
@smanna12 smanna12 marked this pull request as ready for review March 29, 2021 03:11
@smanna12 smanna12 changed the title [SYCL][FPGA] Ignore duplicate attribute when adding or merging [SYCL][FPGA] Drop duplicate attribute when adding or merging Mar 29, 2021
@smanna12 smanna12 changed the title [SYCL][FPGA] Drop duplicate attribute when adding or merging [SYCL][FPGA] Drop the duplicate attribute when adding or merging Mar 29, 2021
@smanna12
Copy link
Contributor Author

smanna12 commented Mar 29, 2021 •
edited
Loading

I think in general we should not be calling dropAttr as in this patch -- better to not call addAttr in the first place (either by returning nullptr from the merge function or doing an early return from the add function).

Done. Thanks @AaronBallman.

AaronBallman
AaronBallman approved these changes Mar 29, 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
Copy link
Contributor Author

LGTM!

Thanks Aaron for the reviews.

@bader bader merged commit 977200f into intel:sycl Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AaronBallman AaronBallman AaronBallman approved these changes

@elizabethandrews elizabethandrews Awaiting requested review from elizabethandrews

@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.

Ignore duplicate attribute when adding or merging.
3 participants
@smanna12 @AaronBallman @bader