[Flang] Handle the source information (scopes) for some OpenMP constructs #109097
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
This patch adds the source information to all the atomic nodes in the openmp parser. When the construct starts at the line 1, the construct source information is used as the program source. This fixes llvm#85593, which used to crash when accessing the source of the construct.
and Atomic constructs This patch fixes llvm#82943, which used to crash when the threadprivate directive appears as the first directive without the program statement. Fix: Adding a source range from the current statement to the threadprivate and atomic will take care of the crash. See: https://reviews.llvm.org/D153634 for more details
Thirumalai-Shaktivel
added
flang
Member
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-openmp Author: Thirumalai Shaktivel (Thirumalai-Shaktivel) ChangesFixes: #82943 Full diff: https://github.com/llvm/llvm-project/pull/109097.diff 5 Files Affected:
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 52789d6e5f0f6b..04f78aa614005e 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -566,33 +566,36 @@ TYPE_PARSER(construct<OmpEndAtomic>(startOmpLine >> "END ATOMIC"_tok))
// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] READ [MEMORY-ORDER-CLAUSE-LIST]
TYPE_PARSER("ATOMIC" >>
- construct<OmpAtomicRead>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
- verbatim("READ"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
- statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+ sourced(construct<OmpAtomicRead>(
+ Parser<OmpAtomicClauseList>{} / maybe(","_tok), verbatim("READ"_tok),
+ Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
+ maybe(Parser<OmpEndAtomic>{} / endOmpLine))))
// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] CAPTURE [MEMORY-ORDER-CLAUSE-LIST]
TYPE_PARSER("ATOMIC" >>
- construct<OmpAtomicCapture>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
- verbatim("CAPTURE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
- statement(assignmentStmt), statement(assignmentStmt),
- Parser<OmpEndAtomic>{} / endOmpLine))
+ sourced(construct<OmpAtomicCapture>(
+ Parser<OmpAtomicClauseList>{} / maybe(","_tok), verbatim("CAPTURE"_tok),
+ Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
+ statement(assignmentStmt), Parser<OmpEndAtomic>{} / endOmpLine)))
// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] UPDATE [MEMORY-ORDER-CLAUSE-LIST]
TYPE_PARSER("ATOMIC" >>
- construct<OmpAtomicUpdate>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
- verbatim("UPDATE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
- statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+ sourced(construct<OmpAtomicUpdate>(
+ Parser<OmpAtomicClauseList>{} / maybe(","_tok), verbatim("UPDATE"_tok),
+ Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
+ maybe(Parser<OmpEndAtomic>{} / endOmpLine))))
// OMP ATOMIC [atomic-clause-list]
-TYPE_PARSER(construct<OmpAtomic>(verbatim("ATOMIC"_tok),
+TYPE_PARSER(sourced(construct<OmpAtomic>(verbatim("ATOMIC"_tok),
Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
- maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+ maybe(Parser<OmpEndAtomic>{} / endOmpLine))))
// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] WRITE [MEMORY-ORDER-CLAUSE-LIST]
TYPE_PARSER("ATOMIC" >>
- construct<OmpAtomicWrite>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
- verbatim("WRITE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
- statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+ sourced(construct<OmpAtomicWrite>(
+ Parser<OmpAtomicClauseList>{} / maybe(","_tok), verbatim("WRITE"_tok),
+ Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
+ maybe(Parser<OmpEndAtomic>{} / endOmpLine))))
// Atomic Construct
TYPE_PARSER(construct<OpenMPAtomicConstruct>(Parser<OmpAtomicRead>{}) ||
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index b99f308e1c7fab..fd31553fff59ae 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1505,6 +1505,23 @@ class OmpVisitor : public virtual DeclarationVisitor {
void Post(const parser::OmpEndCriticalDirective &) {
messageHandler().set_currStmtSource(std::nullopt);
}
+ bool Pre(const parser::OpenMPDeclarativeConstruct &x) {
+ AddOmpSourceRange(x.source);
+ return true;
+ }
+ void Post(const parser::OpenMPDeclarativeConstruct &) {
+ messageHandler().set_currStmtSource(std::nullopt);
+ }
+ bool Pre(const parser::OpenMPAtomicConstruct &x) {
+ return common::visit(common::visitors{[&](const auto &u) -> bool {
+ AddOmpSourceRange(u.source);
+ return true;
+ }},
+ x.u);
+ }
+ void Post(const parser::OpenMPAtomicConstruct &) {
+ messageHandler().set_currStmtSource(std::nullopt);
+ }
};
bool OmpVisitor::NeedsScope(const parser::OpenMPBlockConstruct &x) {
diff --git a/flang/test/Semantics/OpenMP/atomic06-empty.f90 b/flang/test/Semantics/OpenMP/atomic06-empty.f90
new file mode 100644
index 00000000000000..226e8d1bb91a5b
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/atomic06-empty.f90
@@ -0,0 +1,6 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Test the source code starting with omp syntax
+
+!$omp atomic write
+i = 123
+end
diff --git a/flang/test/Semantics/OpenMP/declare-simd-empty.f90 b/flang/test/Semantics/OpenMP/declare-simd-empty.f90
new file mode 100644
index 00000000000000..b61fb53730a23d
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-simd-empty.f90
@@ -0,0 +1,6 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Test the source code starting with omp syntax
+
+!$omp declare simd
+integer :: x
+end
diff --git a/flang/test/Semantics/OpenMP/threadprivate08-empty.f90 b/flang/test/Semantics/OpenMP/threadprivate08-empty.f90
new file mode 100644
index 00000000000000..38a855dceb8366
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/threadprivate08-empty.f90
@@ -0,0 +1,5 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Test the source code starting with omp syntax
+
+!$omp threadprivate(a)
+end
|
Member
|
@llvm/pr-subscribers-flang-parser Author: Thirumalai Shaktivel (Thirumalai-Shaktivel) ChangesFixes: #82943 Full diff: https://github.com/llvm/llvm-project/pull/109097.diff 5 Files Affected:
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 52789d6e5f0f6b..04f78aa614005e 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -566,33 +566,36 @@ TYPE_PARSER(construct<OmpEndAtomic>(startOmpLine >> "END ATOMIC"_tok))
// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] READ [MEMORY-ORDER-CLAUSE-LIST]
TYPE_PARSER("ATOMIC" >>
- construct<OmpAtomicRead>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
- verbatim("READ"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
- statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+ sourced(construct<OmpAtomicRead>(
+ Parser<OmpAtomicClauseList>{} / maybe(","_tok), verbatim("READ"_tok),
+ Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
+ maybe(Parser<OmpEndAtomic>{} / endOmpLine))))
// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] CAPTURE [MEMORY-ORDER-CLAUSE-LIST]
TYPE_PARSER("ATOMIC" >>
- construct<OmpAtomicCapture>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
- verbatim("CAPTURE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
- statement(assignmentStmt), statement(assignmentStmt),
- Parser<OmpEndAtomic>{} / endOmpLine))
+ sourced(construct<OmpAtomicCapture>(
+ Parser<OmpAtomicClauseList>{} / maybe(","_tok), verbatim("CAPTURE"_tok),
+ Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
+ statement(assignmentStmt), Parser<OmpEndAtomic>{} / endOmpLine)))
// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] UPDATE [MEMORY-ORDER-CLAUSE-LIST]
TYPE_PARSER("ATOMIC" >>
- construct<OmpAtomicUpdate>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
- verbatim("UPDATE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
- statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+ sourced(construct<OmpAtomicUpdate>(
+ Parser<OmpAtomicClauseList>{} / maybe(","_tok), verbatim("UPDATE"_tok),
+ Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
+ maybe(Parser<OmpEndAtomic>{} / endOmpLine))))
// OMP ATOMIC [atomic-clause-list]
-TYPE_PARSER(construct<OmpAtomic>(verbatim("ATOMIC"_tok),
+TYPE_PARSER(sourced(construct<OmpAtomic>(verbatim("ATOMIC"_tok),
Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
- maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+ maybe(Parser<OmpEndAtomic>{} / endOmpLine))))
// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] WRITE [MEMORY-ORDER-CLAUSE-LIST]
TYPE_PARSER("ATOMIC" >>
- construct<OmpAtomicWrite>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
- verbatim("WRITE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
- statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+ sourced(construct<OmpAtomicWrite>(
+ Parser<OmpAtomicClauseList>{} / maybe(","_tok), verbatim("WRITE"_tok),
+ Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
+ maybe(Parser<OmpEndAtomic>{} / endOmpLine))))
// Atomic Construct
TYPE_PARSER(construct<OpenMPAtomicConstruct>(Parser<OmpAtomicRead>{}) ||
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index b99f308e1c7fab..fd31553fff59ae 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1505,6 +1505,23 @@ class OmpVisitor : public virtual DeclarationVisitor {
void Post(const parser::OmpEndCriticalDirective &) {
messageHandler().set_currStmtSource(std::nullopt);
}
+ bool Pre(const parser::OpenMPDeclarativeConstruct &x) {
+ AddOmpSourceRange(x.source);
+ return true;
+ }
+ void Post(const parser::OpenMPDeclarativeConstruct &) {
+ messageHandler().set_currStmtSource(std::nullopt);
+ }
+ bool Pre(const parser::OpenMPAtomicConstruct &x) {
+ return common::visit(common::visitors{[&](const auto &u) -> bool {
+ AddOmpSourceRange(u.source);
+ return true;
+ }},
+ x.u);
+ }
+ void Post(const parser::OpenMPAtomicConstruct &) {
+ messageHandler().set_currStmtSource(std::nullopt);
+ }
};
bool OmpVisitor::NeedsScope(const parser::OpenMPBlockConstruct &x) {
diff --git a/flang/test/Semantics/OpenMP/atomic06-empty.f90 b/flang/test/Semantics/OpenMP/atomic06-empty.f90
new file mode 100644
index 00000000000000..226e8d1bb91a5b
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/atomic06-empty.f90
@@ -0,0 +1,6 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Test the source code starting with omp syntax
+
+!$omp atomic write
+i = 123
+end
diff --git a/flang/test/Semantics/OpenMP/declare-simd-empty.f90 b/flang/test/Semantics/OpenMP/declare-simd-empty.f90
new file mode 100644
index 00000000000000..b61fb53730a23d
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-simd-empty.f90
@@ -0,0 +1,6 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Test the source code starting with omp syntax
+
+!$omp declare simd
+integer :: x
+end
diff --git a/flang/test/Semantics/OpenMP/threadprivate08-empty.f90 b/flang/test/Semantics/OpenMP/threadprivate08-empty.f90
new file mode 100644
index 00000000000000..38a855dceb8366
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/threadprivate08-empty.f90
@@ -0,0 +1,5 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! Test the source code starting with omp syntax
+
+!$omp threadprivate(a)
+end
|
Thirumalai-Shaktivel
requested review from
NimishMishra,
harishch4,
kiranchandramohan,
kiranktp and
tblah
Contributor
|
A commit message description would be good, describing the fix. Can we add that? |
Thirumalai-Shaktivel
force-pushed
the
llvm/source_01
branch
from
e54abd5 to
0903cbc
Compare
Member
Author
|
@NimishMishra, I have added the description. Do you want me to add anything else? |
NimishMishra
approved these changes
Oct 9, 2024
Contributor
NimishMishra
left a comment
NimishMishra
left a comment
There was a problem hiding this comment.
Thanks for linking the fixed issues. LGTM
Member
Author
|
Thanks for the reviews! |
Thirumalai-Shaktivel
force-pushed
the
llvm/source_01
branch
from
0903cbc to
983ac74
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Fixes: #82943
Fixes: #82942
Fixes: #85593