Skip to content

[SYCL] [FPGA] Add mutual diagnostic of max_concurrency attribute in conjunction of disable_loop_pipelining attribute #3512

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 8 commits into from
Apr 13, 2021
Merged

[SYCL] [FPGA] Add mutual diagnostic of max_concurrency attribute in conjunction of disable_loop_pipelining attribute #3512

merged 8 commits into from
Apr 13, 2021

Conversation

smanna12
Copy link
Contributor

@smanna12 smanna12 commented Apr 8, 2021 •
edited
Loading

The support for disable_loop_pipelining was implemented on #3441

This patch

  1. removes FIXME and adds mutual diagnostic of max_concurrency and disable_loop_pipelining attributes.
  2. updates document
  3. adds tests

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

@smanna12
Add mutual diagnostic of max_concurrency attribute in conjunction of …
2fa6c7b 
…disable_loop_pipelining attribute

The support for disable_loop_pipelining was implemented on intel#3441

This patch
  1. removes FIXME
  2. Updtaes document
  3. adds tests
  4. fixes typos

Signed-off-by: Soumi Manna <[email protected]>
@smanna12 smanna12 changed the title Add mutual diagnostic of max_concurrency attribute in conjunction of … Add mutual diagnostic of max_concurrency attribute in conjunction of disable_loop_pipelining attribute Apr 8, 2021
@smanna12 smanna12 changed the title Add mutual diagnostic of max_concurrency attribute in conjunction of disable_loop_pipelining attribute [SYCL] [FPGA] Add mutual diagnostic of max_concurrency attribute in conjunction of disable_loop_pipelining attribute Apr 8, 2021
@smanna12 smanna12 marked this pull request as ready for review April 8, 2021 13:20
@smanna12 smanna12 requested a review from zahiraam April 8, 2021 13:20
AaronBallman
AaronBallman reviewed Apr 8, 2021
View reviewed changes
@smanna12 smanna12 marked this pull request as draft April 8, 2021 15:10
@smanna12
Copy link
Contributor Author

smanna12 commented Apr 8, 2021

I am seeing some problems with the merge, Please do not review.

@AaronBallman
Copy link
Contributor

I am seeing some problems with the merge, Please do not review.

I'm looking at those errors as well as they seem to be related to the recent tablegen changes.

@smanna12
Copy link
Contributor Author

smanna12 commented Apr 8, 2021

I am seeing some problems with the merge, Please do not review.

I'm looking at those errors as well as they seem to be related to the recent tablegen changes.

Yes, many warnings are generating like this:
warning: ‘virtual bool clang::ParsedAttrInfo::diagMutualExclusion(clang::Sema&, const clang::ParsedAttr&, const clang::Stmt*) const’ was hidden [-Woverloaded-virtual]
virtual bool diagMutualExclusion(Sema &S, const ParsedAttr &A,

@AaronBallman, are you seeing same?

@AaronBallman
Copy link
Contributor

I am seeing some problems with the merge, Please do not review.

I'm looking at those errors as well as they seem to be related to the recent tablegen changes.

Yes, many warnings are generating like this:
warning: ‘virtual bool clang::ParsedAttrInfo::diagMutualExclusion(clang::Sema&, const clang::ParsedAttr&, const clang::Stmt*) const’ was hidden [-Woverloaded-virtual]
virtual bool diagMutualExclusion(Sema &S, const ParsedAttr &A,

@AaronBallman, are you seeing same?

The warnings are fixed upstream by: llvm/llvm-project@2458aa0#diff-58111c7c545672c3a67b1b31a37d4103b35d9c584a46c74b0be14cc37741491b

@smanna12
Copy link
Contributor Author

smanna12 commented Apr 8, 2021

I am seeing some problems with the merge, Please do not review.

I'm looking at those errors as well as they seem to be related to the recent tablegen changes.

Yes, many warnings are generating like this:
warning: ‘virtual bool clang::ParsedAttrInfo::diagMutualExclusion(clang::Sema&, const clang::ParsedAttr&, const clang::Stmt*) const’ was hidden [-Woverloaded-virtual]
virtual bool diagMutualExclusion(Sema &S, const ParsedAttr &A,
@AaronBallman, are you seeing same?

The warnings are fixed upstream by: llvm/llvm-project@2458aa0#diff-58111c7c545672c3a67b1b31a37d4103b35d9c584a46c74b0be14cc37741491b

I need to do rebase to be up with upstream.

@AaronBallman
Copy link
Contributor

I am seeing some problems with the merge, Please do not review.

I'm looking at those errors as well as they seem to be related to the recent tablegen changes.

Yes, many warnings are generating like this:
warning: ‘virtual bool clang::ParsedAttrInfo::diagMutualExclusion(clang::Sema&, const clang::ParsedAttr&, const clang::Stmt*) const’ was hidden [-Woverloaded-virtual]
virtual bool diagMutualExclusion(Sema &S, const ParsedAttr &A,
@AaronBallman, are you seeing same?

The warnings are fixed upstream by: llvm/llvm-project@2458aa0#diff-58111c7c545672c3a67b1b31a37d4103b35d9c584a46c74b0be14cc37741491b

I need to do rebase to be up with upstream.

Errr, it'd help if I linked the correct review -- this one fixed those issues: llvm/llvm-project@65c22ac#diff-58111c7c545672c3a67b1b31a37d4103b35d9c584a46c74b0be14cc37741491b

@smanna12
Copy link
Contributor Author

smanna12 commented Apr 8, 2021 •
edited
Loading

I am seeing some problems with the merge, Please do not review.

I'm looking at those errors as well as they seem to be related to the recent tablegen changes.

Yes, many warnings are generating like this:
warning: ‘virtual bool clang::ParsedAttrInfo::diagMutualExclusion(clang::Sema&, const clang::ParsedAttr&, const clang::Stmt*) const’ was hidden [-Woverloaded-virtual]
virtual bool diagMutualExclusion(Sema &S, const ParsedAttr &A,
@AaronBallman, are you seeing same?

The warnings are fixed upstream by: llvm/llvm-project@2458aa0#diff-58111c7c545672c3a67b1b31a37d4103b35d9c584a46c74b0be14cc37741491b

I need to do rebase to be up with upstream.

Errr, it'd help if I linked the correct review -- this one fixed those issues: llvm/llvm-project@65c22ac#diff-58111c7c545672c3a67b1b31a37d4103b35d9c584a46c74b0be14cc37741491b

Thanks @AaronBallman. I cherry-picked the change to fix all warnings about -Woverloaded-virtual.

@smanna12 smanna12 marked this pull request as ready for review April 8, 2021 19:29
@smanna12 smanna12 requested a review from AaronBallman April 8, 2021 19:46
AaronBallman
AaronBallman reviewed Apr 8, 2021
View reviewed changes
@smanna12
remove unrealted chnages
137f83c 
Signed-off-by: Soumi Manna <[email protected]>
@smanna12
Copy link
Contributor Author

smanna12 commented Apr 9, 2021

buildbot/Lit_with_Cuda is failing with warning like this:

lit WARNING: The PID of hang binary not found

premanandrao
premanandrao approved these changes Apr 9, 2021
View reviewed changes
Copy link
Contributor

@premanandrao premanandrao left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@bader
Copy link
Contributor

bader commented Apr 9, 2021

buildbot/Lit_with_Cuda is failing with warning like this:

lit WARNING: The PID of hang binary not found

@tfzhu, could you help to resolve that issue, please?

AaronBallman
AaronBallman approved these changes Apr 9, 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!

@AaronBallman AaronBallman mentioned this pull request Apr 9, 2021
Merged
@smanna12
Copy link
Contributor Author

smanna12 commented Apr 9, 2021

buildbot/Lit_with_Cuda is failing with warning like this:
lit WARNING: The PID of hang binary not found

@tfzhu, could you help to resolve that issue, please?

@tfzhu, any update on it? Thanks

@smanna12
Copy link
Contributor Author

buildbot/Lit_with_Cuda is failing with warning like this:
lit WARNING: The PID of hang binary not found

@tfzhu, could you help to resolve that issue, please?

@bader, The issues are resolved. Thanks.

@bader bader merged commit 7324b3e into intel:sycl Apr 13, 2021
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 14, 2021
Merge remote-tracking branch 'upstream/sycl' into sycl2020_interop_pt3
02e8c01 
* upstream/sycl: (39 commits)
  [CI] Switch to default clang-format version. (intel#3540)
  [Driver][NFC] Cleanup some option setting for SYCL offload (intel#3542)
  [GitHub Actions] Update main branch sync schedule
  [SYCL][NFC] Fix potential namespace conflicts with PSTL in tuple.hpp (intel#3541)
  [SYCL] Bump sycl library minor version (intel#3538)
  [SYCL][CUDA] Implemented cuda_piextUSMEnqueueMemAdvise (intel#3365)
  [SYCL][FPGA] Add mutual diagnostic of max_concurrency attribute in conjunction of disable_loop_pipelining attribute (intel#3512)
  [SYCL] [MATRIX] Enable joint_matrix_load, joint_matrix_store, and joint_matrix_mad for AMX (intel#3503)
  [ESIMD] Skip rewriting functions used through function pointers (intel#3527)
  [SYCL] Fix address space for spec constants buffer (intel#3521)
  [SYCL] Correct the tablegen for checking mutually exclusive stmt attrs (intel#3519)
  [SYCL][PI][L0][NFC] Refactor setting of LastCommandEvent (intel#3528)
  [SYCL] Fix group local memory sharing issue (intel#3489)
  [SYCL][NFC] Fix post-commit failure (intel#3532)
  [SYCL][Doc] Remove extension mechanism (intel#3526)
  [SYCL] Move sycl.hpp in install directory and adjust driver to match (intel#3523)
  [SYCL][ESIMD] Update ESIMD docs to address recent user comments: (intel#3516)
  [NFCI][SYCL] Correct -fdeclare-spirv-builtins to use marshalling (intel#3515)
  [SYCL] Rework MarkDevice and children (intel#3475)
  [SYCL] Fix StringLiteral Ctor issue from intel#3504. (intel#3520)
  ...
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

@premanandrao premanandrao premanandrao approved these changes

@elizabethandrews elizabethandrews Awaiting requested review from elizabethandrews

@zahiraam zahiraam Awaiting requested review from zahiraam

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

Successfully merging this pull request may close these issues.
4 participants
@smanna12 @AaronBallman @bader @premanandrao