[flang][Parser][OpenMP] Fix unparser for cancellation_construct_type #136001
+67
−0
Conversation
This file contains 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
Previously the unparser would print like ``` !$OMP CANCEL CANCELLATION_CONSTRUCT_TYPE(SECTIONS) ``` This is not valid Fortran. I have fixed it to print without the clause name.
tblah
requested review from
sscalpone,
kiranchandramohan,
Leporacanthicus,
kparzysz and
jurahul
llvmbot
added
tablegen
flang
|
@llvm/pr-subscribers-tablegen @llvm/pr-subscribers-flang-openmp Author: Tom Eccles (tblah) ChangesPreviously the unparser would print like This is not valid Fortran. I have fixed it to print without the clause name. Full diff: https://github.com/llvm/llvm-project/pull/136001.diff 6 Files Affected:
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 47dae0ae753d2..35fb0790b633d 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2871,6 +2871,10 @@ class UnparseVisitor {
Put("\n");
EndOpenMP();
}
+ // Clause unparsers are usually generated by tablegen in the form
+ // CLAUSE(VALUE). Here we only want to print VALUE so a custom unparser is
+ // needed.
+ void Unparse(const OmpClause::CancellationConstructType &x) { Walk(x.v); }
void Unparse(const OpenMPCancellationPointConstruct &x) {
BeginOpenMP();
Word("!$OMP ");
diff --git a/flang/test/Parser/OpenMP/cancel.f90 b/flang/test/Parser/OpenMP/cancel.f90
new file mode 100644
index 0000000000000..f5c1309e0d8c1
--- /dev/null
+++ b/flang/test/Parser/OpenMP/cancel.f90
@@ -0,0 +1,53 @@
+!RUN: %flang_fc1 -fdebug-unparse -fopenmp %s | FileCheck %s
+
+! CHECK: SUBROUTINE parallel
+subroutine parallel
+! CHECK: !$OMP PARALLEL
+ !$omp parallel
+! CHECK: !$OMP CANCEL PARALLEL
+ !$omp cancel parallel
+! CHECK: !$OMP END PARALLEL
+ !$omp end parallel
+! CHECK: END SUBROUTINE
+end subroutine
+
+! CHECK: SUBROUTINE sections
+subroutine sections
+! CHECK: !$OMP PARALLEL SECTIONS
+ !$omp parallel sections
+! CHECK: !$OMP CANCEL SECTIONS
+ !$omp cancel sections
+! CHECK: !$OMP END PARALLEL SECTIONS
+ !$omp end parallel sections
+! CHECK: END SUBROUTINE
+end subroutine
+
+! CHECK: SUBROUTINE loop
+subroutine loop
+! CHECK: !$OMP PARALLEL DO
+ !$omp parallel do
+! CHECK: DO i=1_4,10_4
+ do i=1,10
+! CHECK: !$OMP CANCEL DO
+ !$omp cancel do
+! CHECK: END DO
+ enddo
+! CHECK: !$OMP END PARALLEL DO
+ !$omp end parallel do
+! CHECK: END SUBROUTINE
+end subroutine
+
+! CHECK: SUBROUTINE taskgroup
+subroutine taskgroup
+! CHECK: !$OMP TASKGROUP
+ !$omp taskgroup
+! CHECK: !$OMP TASK
+ !$omp task
+! CHECK: !$OMP CANCEL TASKGROUP
+ !$omp cancel taskgroup
+! CHECK: !$OMP END TASK
+ !$omp end task
+! CHECK: !$OMP END TASKGROUP
+ !$omp end taskgroup
+! CHECK: END SUBROUTINE
+end subroutine
diff --git a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
index 707239c7ba364..745eccc9b147f 100644
--- a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
+++ b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
@@ -83,6 +83,9 @@ class Clause<string c> {
// Optional class holding value of the clause in flang AST.
string flangClass = "";
+ // If set to true, don't emit flang Unparser.
+ bit skipFlangUnparser = false;
+
// If set to true, value is optional. Not optional by default.
bit isValueOptional = false;
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index e2a1449d8cc76..1f286abaa9679 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -106,6 +106,7 @@ def OMPC_CancellationConstructType : Clause<"cancellation_construct_type"> {
OMP_CANCELLATION_CONSTRUCT_None
];
let flangClass = "OmpCancellationConstructTypeClause";
+ let skipFlangUnparser = true;
}
def OMPC_Contains : Clause<"contains"> {
let clangClass = "OMPContainsClause";
diff --git a/llvm/include/llvm/TableGen/DirectiveEmitter.h b/llvm/include/llvm/TableGen/DirectiveEmitter.h
index a2c9b2d427cce..78b0b24d4d4c0 100644
--- a/llvm/include/llvm/TableGen/DirectiveEmitter.h
+++ b/llvm/include/llvm/TableGen/DirectiveEmitter.h
@@ -209,6 +209,10 @@ class Clause : public BaseRecord {
return Def->getValueAsListOfDefs("allowedClauseValues");
}
+ bool skipFlangUnparser() const {
+ return Def->getValueAsBit("skipFlangUnparser");
+ }
+
bool isValueOptional() const { return Def->getValueAsBit("isValueOptional"); }
bool isValueList() const { return Def->getValueAsBit("isValueList"); }
diff --git a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
index ab68e028f1e96..9f8404b02a141 100644
--- a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
@@ -912,6 +912,8 @@ static void generateFlangClauseUnparse(const DirectiveLanguage &DirLang,
OS << "\n";
for (const Clause Clause : DirLang.getClauses()) {
+ if (Clause.skipFlangUnparser())
+ continue;
if (!Clause.getFlangClass().empty()) {
if (Clause.isValueOptional() && Clause.getDefaultValue().empty()) {
OS << "void Unparse(const " << DirLang.getFlangClauseBaseClass()
|
|
@llvm/pr-subscribers-flang-parser Author: Tom Eccles (tblah) ChangesPreviously the unparser would print like This is not valid Fortran. I have fixed it to print without the clause name. Full diff: https://github.com/llvm/llvm-project/pull/136001.diff 6 Files Affected:
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 47dae0ae753d2..35fb0790b633d 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2871,6 +2871,10 @@ class UnparseVisitor {
Put("\n");
EndOpenMP();
}
+ // Clause unparsers are usually generated by tablegen in the form
+ // CLAUSE(VALUE). Here we only want to print VALUE so a custom unparser is
+ // needed.
+ void Unparse(const OmpClause::CancellationConstructType &x) { Walk(x.v); }
void Unparse(const OpenMPCancellationPointConstruct &x) {
BeginOpenMP();
Word("!$OMP ");
diff --git a/flang/test/Parser/OpenMP/cancel.f90 b/flang/test/Parser/OpenMP/cancel.f90
new file mode 100644
index 0000000000000..f5c1309e0d8c1
--- /dev/null
+++ b/flang/test/Parser/OpenMP/cancel.f90
@@ -0,0 +1,53 @@
+!RUN: %flang_fc1 -fdebug-unparse -fopenmp %s | FileCheck %s
+
+! CHECK: SUBROUTINE parallel
+subroutine parallel
+! CHECK: !$OMP PARALLEL
+ !$omp parallel
+! CHECK: !$OMP CANCEL PARALLEL
+ !$omp cancel parallel
+! CHECK: !$OMP END PARALLEL
+ !$omp end parallel
+! CHECK: END SUBROUTINE
+end subroutine
+
+! CHECK: SUBROUTINE sections
+subroutine sections
+! CHECK: !$OMP PARALLEL SECTIONS
+ !$omp parallel sections
+! CHECK: !$OMP CANCEL SECTIONS
+ !$omp cancel sections
+! CHECK: !$OMP END PARALLEL SECTIONS
+ !$omp end parallel sections
+! CHECK: END SUBROUTINE
+end subroutine
+
+! CHECK: SUBROUTINE loop
+subroutine loop
+! CHECK: !$OMP PARALLEL DO
+ !$omp parallel do
+! CHECK: DO i=1_4,10_4
+ do i=1,10
+! CHECK: !$OMP CANCEL DO
+ !$omp cancel do
+! CHECK: END DO
+ enddo
+! CHECK: !$OMP END PARALLEL DO
+ !$omp end parallel do
+! CHECK: END SUBROUTINE
+end subroutine
+
+! CHECK: SUBROUTINE taskgroup
+subroutine taskgroup
+! CHECK: !$OMP TASKGROUP
+ !$omp taskgroup
+! CHECK: !$OMP TASK
+ !$omp task
+! CHECK: !$OMP CANCEL TASKGROUP
+ !$omp cancel taskgroup
+! CHECK: !$OMP END TASK
+ !$omp end task
+! CHECK: !$OMP END TASKGROUP
+ !$omp end taskgroup
+! CHECK: END SUBROUTINE
+end subroutine
diff --git a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
index 707239c7ba364..745eccc9b147f 100644
--- a/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
+++ b/llvm/include/llvm/Frontend/Directive/DirectiveBase.td
@@ -83,6 +83,9 @@ class Clause<string c> {
// Optional class holding value of the clause in flang AST.
string flangClass = "";
+ // If set to true, don't emit flang Unparser.
+ bit skipFlangUnparser = false;
+
// If set to true, value is optional. Not optional by default.
bit isValueOptional = false;
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index e2a1449d8cc76..1f286abaa9679 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -106,6 +106,7 @@ def OMPC_CancellationConstructType : Clause<"cancellation_construct_type"> {
OMP_CANCELLATION_CONSTRUCT_None
];
let flangClass = "OmpCancellationConstructTypeClause";
+ let skipFlangUnparser = true;
}
def OMPC_Contains : Clause<"contains"> {
let clangClass = "OMPContainsClause";
diff --git a/llvm/include/llvm/TableGen/DirectiveEmitter.h b/llvm/include/llvm/TableGen/DirectiveEmitter.h
index a2c9b2d427cce..78b0b24d4d4c0 100644
--- a/llvm/include/llvm/TableGen/DirectiveEmitter.h
+++ b/llvm/include/llvm/TableGen/DirectiveEmitter.h
@@ -209,6 +209,10 @@ class Clause : public BaseRecord {
return Def->getValueAsListOfDefs("allowedClauseValues");
}
+ bool skipFlangUnparser() const {
+ return Def->getValueAsBit("skipFlangUnparser");
+ }
+
bool isValueOptional() const { return Def->getValueAsBit("isValueOptional"); }
bool isValueList() const { return Def->getValueAsBit("isValueList"); }
diff --git a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
index ab68e028f1e96..9f8404b02a141 100644
--- a/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/DirectiveEmitter.cpp
@@ -912,6 +912,8 @@ static void generateFlangClauseUnparse(const DirectiveLanguage &DirLang,
OS << "\n";
for (const Clause Clause : DirLang.getClauses()) {
+ if (Clause.skipFlangUnparser())
+ continue;
if (!Clause.getFlangClass().empty()) {
if (Clause.isValueOptional() && Clause.getDefaultValue().empty()) {
OS << "void Unparse(const " << DirLang.getFlangClauseBaseClass()
|
|
I believe the CI failure on linux was unrelated to this patch. |
kparzysz
approved these changes
Apr 17, 2025
| @@ -83,6 +83,9 @@ class Clause<string c> { | |||
| // Optional class holding value of the clause in flang AST. | |||
| string flangClass = ""; | |||
|
|
|||
| // If set to true, don't emit flang Unparser. | |||
| bit skipFlangUnparser = false; | |||
There was a problem hiding this comment.
Could you rename this to isArtificial? There may be more future cases for this, and more places to check this flag.
There was a problem hiding this comment.
I can easily tell what skipFlangUnparser means, but that it would be a consequence of isArtificial seems less obvious.
There was a problem hiding this comment.
IMO, what's more significant is the property of the clause object. skipFlangParser tells you what the consequence is, but not why it's there. There may be more consequences to follow that we haven't yet discovered, or that may appear in the future.
klausler
approved these changes
Apr 17, 2025
| @@ -83,6 +83,9 @@ class Clause<string c> { | |||
| // Optional class holding value of the clause in flang AST. | |||
| string flangClass = ""; | |||
|
|
|||
| // If set to true, don't emit flang Unparser. | |||
| bit skipFlangUnparser = false; | |||
There was a problem hiding this comment.
I can easily tell what skipFlangUnparser means, but that it would be a consequence of isArtificial seems less obvious.
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.
Previously the unparser would print like
This is not valid Fortran. I have fixed it to print without the clause name.