[llvm-branch-commits] [flang] [flang][OpenMP] move omp end sections validation to semantics (PR #154740)
Tom Eccles via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Aug 21 08:54:26 PDT 2025
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/154740
>From f84070b2b0b1f446dded8a1c93267034169e39fb Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Wed, 20 Aug 2025 17:19:24 +0000
Subject: [PATCH 1/2] [flang][OpenMP] move omp end sections validation to
semantics
See #90452. The old parse tree errors exploded to thousands of unhelpful
lines when there were multiple missing end directives.
Instead, allow a missing end directive in the parse tree then validate
that it is present during semantics (where the error messages are a lot
easier to control).
---
flang/include/flang/Parser/parse-tree.h | 5 ++++-
flang/lib/Lower/OpenMP/OpenMP.cpp | 7 +++++--
flang/lib/Parser/openmp-parsers.cpp | 2 +-
flang/lib/Parser/unparse.cpp | 2 +-
flang/lib/Semantics/check-omp-structure.cpp | 17 +++++++++++++----
.../Semantics/OpenMP/missing-end-directive.f90 | 4 ++++
6 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 1d1a4a163084b..4189a340d32cd 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4893,8 +4893,11 @@ struct OpenMPSectionsConstruct {
CharBlock source;
// Each of the OpenMPConstructs in the list below contains an
// OpenMPSectionConstruct. This is guaranteed by the parser.
+ // The end sections directive is optional here because it is difficult to
+ // generate helpful error messages for a missing end directive wihtin the
+ // parser. Semantics will generate an error if this is absent.
std::tuple<OmpBeginSectionsDirective, std::list<OpenMPConstruct>,
- OmpEndSectionsDirective>
+ std::optional<OmpEndSectionsDirective>>
t;
};
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index ec2ec37e623f8..da5898480da22 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3958,9 +3958,12 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
List<Clause> clauses = makeClauses(
std::get<parser::OmpClauseList>(beginSectionsDirective.t), semaCtx);
const auto &endSectionsDirective =
- std::get<parser::OmpEndSectionsDirective>(sectionsConstruct.t);
+ std::get<std::optional<parser::OmpEndSectionsDirective>>(
+ sectionsConstruct.t);
+ assert(endSectionsDirective &&
+ "Missing end section directive should have been handled in semantics");
clauses.append(makeClauses(
- std::get<parser::OmpClauseList>(endSectionsDirective.t), semaCtx));
+ std::get<parser::OmpClauseList>(endSectionsDirective->t), semaCtx));
mlir::Location currentLocation = converter.getCurrentLocation();
llvm::omp::Directive directive =
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 2093aca38c9d6..62630642edb53 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1917,7 +1917,7 @@ TYPE_PARSER(sourced(construct<OpenMPSectionsConstruct>(
construct<OpenMPSectionConstruct>(maybe(sectionDir), block))),
many(construct<OpenMPConstruct>(
sourced(construct<OpenMPSectionConstruct>(sectionDir, block))))),
- Parser<OmpEndSectionsDirective>{} / endOmpLine)))
+ maybe(Parser<OmpEndSectionsDirective>{} / endOmpLine))))
static bool IsExecutionPart(const OmpDirectiveName &name) {
return name.IsExecutionPart();
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 09dcfe60a46bc..87e699dbc4e8d 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2788,7 +2788,7 @@ class UnparseVisitor {
Walk(std::get<std::list<OpenMPConstruct>>(x.t), "");
BeginOpenMP();
Word("!$OMP END ");
- Walk(std::get<OmpEndSectionsDirective>(x.t));
+ Walk(std::get<std::optional<OmpEndSectionsDirective>>(x.t));
Put("\n");
EndOpenMP();
}
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 835802d81894e..3533cd70f7ad9 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1063,14 +1063,23 @@ void OmpStructureChecker::Leave(const parser::OmpBeginDirective &) {
void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) {
const auto &beginSectionsDir{
std::get<parser::OmpBeginSectionsDirective>(x.t)};
- const auto &endSectionsDir{std::get<parser::OmpEndSectionsDirective>(x.t)};
+ const auto &endSectionsDir{
+ std::get<std::optional<parser::OmpEndSectionsDirective>>(x.t)};
const auto &beginDir{
std::get<parser::OmpSectionsDirective>(beginSectionsDir.t)};
- const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir.t)};
+ PushContextAndClauseSets(beginDir.source, beginDir.v);
+
+ if (!endSectionsDir) {
+ context_.Say(beginSectionsDir.source,
+ "Expected OpenMP END SECTIONS directive"_err_en_US);
+ // Following code assumes the option is present.
+ return;
+ }
+
+ const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir->t)};
CheckMatching<parser::OmpSectionsDirective>(beginDir, endDir);
- PushContextAndClauseSets(beginDir.source, beginDir.v);
- AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir.t));
+ AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir->t));
const auto §ionBlocks{std::get<std::list<parser::OpenMPConstruct>>(x.t)};
for (const parser::OpenMPConstruct &construct : sectionBlocks) {
diff --git a/flang/test/Semantics/OpenMP/missing-end-directive.f90 b/flang/test/Semantics/OpenMP/missing-end-directive.f90
index 3b870d134155b..33481f9d650f4 100644
--- a/flang/test/Semantics/OpenMP/missing-end-directive.f90
+++ b/flang/test/Semantics/OpenMP/missing-end-directive.f90
@@ -6,8 +6,12 @@
!$omp parallel
! ERROR: Expected OpenMP end directive
!$omp task
+! ERROR: Expected OpenMP END SECTIONS directive
+!$omp sections
! ERROR: Expected OpenMP end directive
!$omp parallel
! ERROR: Expected OpenMP end directive
!$omp task
+! ERROR: Expected OpenMP END SECTIONS directive
+!$omp sections
end
>From 245ecd1984c8b6729074d58c853891d0ad0d09dd Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 21 Aug 2025 15:50:55 +0000
Subject: [PATCH 2/2] Fix typo
---
flang/include/flang/Parser/parse-tree.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 4189a340d32cd..6604e42957f84 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4894,7 +4894,7 @@ struct OpenMPSectionsConstruct {
// Each of the OpenMPConstructs in the list below contains an
// OpenMPSectionConstruct. This is guaranteed by the parser.
// The end sections directive is optional here because it is difficult to
- // generate helpful error messages for a missing end directive wihtin the
+ // generate helpful error messages for a missing end directive within the
// parser. Semantics will generate an error if this is absent.
std::tuple<OmpBeginSectionsDirective, std::list<OpenMPConstruct>,
std::optional<OmpEndSectionsDirective>>
More information about the llvm-branch-commits
mailing list