[SYCL] Correct the tablegen for checking mutually exclusive stmt attrs #3519
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The work that was done upstream turned out to be insufficient for checking statement attribute mutual exclusion because attributed statements do not collect their attributes one-at-a-time in the same way that declarations do. So the design that was attempting to check for mutual exclusion as each attribute was processed would not ever catch a mutual exclusion. The new approach is to check all of attributes that are to be applied to the attributed statement in a group. This required generating another DiagnoseMutualExclusions() function into AttrParsedAttrImpl.inc.
AaronBallman
requested review from
elizabethandrews and
premanandrao
as code owners
AaronBallman
commented
Apr 9, 2021
| @@ -1952,6 +1952,10 @@ def SYCLIntelFPGADisableLoopPipelining : DeclOrStmtAttr { | |||
| } | |||
| def : MutualExclusions<[SYCLIntelFPGAInitiationInterval, | |||
| SYCLIntelFPGADisableLoopPipelining]>; | |||
| def : MutualExclusions<[SYCLIntelFPGAIVDep, | |||
| SYCLIntelFPGADisableLoopPipelining]>; | |||
| def : MutualExclusions<[SYCLIntelFPGAMaxConcurrency, | |||
There was a problem hiding this comment.
NB: this is done for completeness in this review but will be in conflict with #3512. Assuming that one lands first, I'll correct the merge conflict in this review.
| @@ -615,7 +609,12 @@ class ParsedAttr final | |||
| bool diagnoseAppertainsTo(class Sema &S, const Decl *D) const; | |||
| bool diagnoseAppertainsTo(class Sema &S, const Stmt *St) const; | |||
| bool diagnoseMutualExclusion(class Sema &S, const Decl *D) const; | |||
| bool diagnoseMutualExclusion(class Sema &S, const Stmt *St) const; | |||
| // This function stub exists for parity with the declaration checking code so | |||
There was a problem hiding this comment.
I do not like this design but I could not think of a more elegant solution. If you have better ideas that don't result in code duplication in checkCommonAttributeFeatures(), I'm all ears.
Comment on lines
-3656
to
+3657
| raw_ostream &MergeOS) { | ||
| raw_ostream &MergeDeclOS, | ||
| raw_ostream &MergeStmtOS) { |
There was a problem hiding this comment.
I'm also not in love with this function generating code into three separate streams, but I couldn't think of a better approach that avoids walking the entire list of records from Attr.td multiple times (which slows down compile speeds because there are a fair amount of records in that file these days).
|
My plan for this is to push the community bits to community after we're happy with the patch here. |
erichkeane
reviewed
Apr 9, 2021
|
@tfzhu, here is another hang on CUDA buildbot. Do you know what is going on? Testing: 0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80..[2021-04-09 16:24:24,148] lit INFO:
LIT test no output within 1200 seconds, start back-trace
[2021-04-09 16:24:24,148] lit INFO: ============================= Start back-trace =============================
sys_bbs+ 21777 0.0 0.0 33168 3000 ? S 16:24 0:00 /bin/sh -c ps aux | grep /localdisk2/sycl_ci/buildbot/worker/Lit_With_Cuda/llvm.obj/tools/sycl/test
sys_bbs+ 21780 0.0 0.0 34844 2436 ? S 16:24 0:00 grep /localdisk2/sycl_ci/buildbot/worker/Lit_With_Cuda/llvm.obj/tools/sycl/test
[2021-04-09 16:24:24,176] lit WARNING: The PID of hang binary not found
[2021-04-09 16:24:24,176] lit INFO: ============================== End back-trace ==============================
[2021-04-09 16:24:24,176] lit INFO: === stage lit end ===
ninja: build stopped: interrupted by user.BTW, I'm not sure if the scrip that tries to find the PID of the processes work for |
premanandrao
approved these changes
Apr 12, 2021
alexbatashev
pushed a commit
to alexbatashev/llvm
that referenced
this pull request
Apr 14, 2021
* 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The work that was done upstream turned out to be insufficient for
checking statement attribute mutual exclusion because attributed
statements do not collect their attributes one-at-a-time in the same
way that declarations do. So the design that was attempting to check
for mutual exclusion as each attribute was processed would not ever
catch a mutual exclusion.
The new approach is to check all of attributes that are to be applied
to the attributed statement in a group. This required generating
another DiagnoseMutualExclusions() function into AttrParsedAttrImpl.inc.