-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[MLIR][OpenMP] NFC: Address tablegen warnings #98485
Conversation
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-mlir-openmp Author: Sergio Afonso (skatrak) ChangesAfter the addition of the These are related to known limitations for which TODO messages exist within these operation's definitions. This patch explicitly inhibits the inheritance of the Full diff: https://github.com/llvm/llvm-project/pull/98485.diff 1 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index aee2937ce5cb7..69fd1f1f0130f 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -133,8 +133,12 @@ def ParallelOp : OpenMP_Op<"parallel", traits = [
RecursiveMemoryEffects
], clauses = [
// TODO: Sort clauses alphabetically.
- OpenMP_IfClause, OpenMP_NumThreadsClause, OpenMP_AllocateClause,
- OpenMP_ReductionClause, OpenMP_ProcBindClause, OpenMP_PrivateClause
+ OpenMP_IfClauseSkip<assemblyFormat = true>,
+ OpenMP_NumThreadsClauseSkip<assemblyFormat = true>,
+ OpenMP_AllocateClauseSkip<assemblyFormat = true>,
+ OpenMP_ReductionClauseSkip<assemblyFormat = true>,
+ OpenMP_ProcBindClauseSkip<assemblyFormat = true>,
+ OpenMP_PrivateClauseSkip<assemblyFormat = true>
], singleRegion = true> {
let summary = "parallel construct";
let description = [{
@@ -154,7 +158,8 @@ def ParallelOp : OpenMP_Op<"parallel", traits = [
// TODO: Use default assembly format inherited from OpenMP_Op once printing
// and parsing of the parallel region is not intermingled with printing and
- // parsing of reduction and private clauses.
+ // parsing of reduction and private clauses. `assemblyFormat` should also be
+ // no longer skipped for clauses added to this operation at that time.
let assemblyFormat = [{
oilist(
`if` `(` $if_expr `)`
@@ -363,8 +368,12 @@ def WsloopOp : OpenMP_Op<"wsloop", traits = [
], clauses = [
// TODO: Complete clause list (allocate, private).
// TODO: Sort clauses alphabetically.
- OpenMP_LinearClause, OpenMP_ReductionClause, OpenMP_ScheduleClause,
- OpenMP_NowaitClause, OpenMP_OrderedClause, OpenMP_OrderClause
+ OpenMP_LinearClauseSkip<assemblyFormat = true>,
+ OpenMP_ReductionClauseSkip<assemblyFormat = true>,
+ OpenMP_ScheduleClauseSkip<assemblyFormat = true>,
+ OpenMP_NowaitClauseSkip<assemblyFormat = true>,
+ OpenMP_OrderedClauseSkip<assemblyFormat = true>,
+ OpenMP_OrderClauseSkip<assemblyFormat = true>
], singleRegion = true> {
let summary = "worksharing-loop construct";
let description = [{
@@ -398,7 +407,8 @@ def WsloopOp : OpenMP_Op<"wsloop", traits = [
// TODO: Use default assembly format inherited from OpenMP_Op once printing
// and parsing of the workshare loop region is not intermingled with printing
- // and parsing of reduction clauses.
+ // and parsing of reduction clauses. `assemblyFormat` should also be no longer
+ // skipped for clauses added to this operation at that time.
let assemblyFormat = [{
oilist(`linear` `(`
custom<LinearClause>($linear_vars, type($linear_vars),
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
After the addition of the `-verify-openmp-ops` tablegen pseudo-backend to report suspected issues with the definition of an operation, some warnings were triggered in the current implementation of OpenMPOps.td. These are related to known limitations for which TODO messages exist within these operation's definitions. This patch explicitly inhibits the inheritance of the `assemblyFormat` property of all clauses added to the `omp.parallel` and `omp.wsloop` operations to avoid these warnings. This should help identifying actual issues that may arise later as the dialect continues to be developed.
1c49246
to
5196f2e
Compare
After the addition of the
-verify-openmp-ops
tablegen pseudo-backend to report suspected issues with the definition of an operation, some warnings were triggered in the current implementation of OpenMPOps.td.These are related to known limitations for which TODO messages exist within these operation's definitions. This patch explicitly inhibits the inheritance of the
assemblyFormat
property of all clauses added to theomp.parallel
andomp.wsloop
operations to avoid these warnings. This should help identifying actual issues that may arise later as the dialect continues to be developed.