[flang-commits] [flang] [flang][OpenMP] Move branching verification to semantic checks (PR #193324)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Wed Apr 22 07:16:44 PDT 2026
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/193324
>From 113e055baf7c92192e240fea7d1bcc49ccf0f393 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Sat, 18 Apr 2026 09:08:55 -0500
Subject: [PATCH 1/3] [flang][OpenMP] Move ALLOCATE + privatize check to
semantic checks
Move the check from symbol resolution to semantic checks.
The check now seems to be more accurate, catching some cases that were
not detected before.
---
flang/include/flang/Parser/openmp-utils.h | 28 ++++++++++
flang/lib/Parser/openmp-utils.cpp | 18 ++++++
flang/lib/Semantics/check-omp-structure.cpp | 62 ++++++++++++++++++++-
flang/lib/Semantics/check-omp-structure.h | 9 ++-
flang/lib/Semantics/resolve-directives.cpp | 35 ------------
flang/test/Lower/OpenMP/sections.f90 | 14 +++--
flang/test/Semantics/OpenMP/taskgroup01.f90 | 2 +
7 files changed, 124 insertions(+), 44 deletions(-)
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index cee3963be4281..e2e5ec19c2c34 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -36,6 +36,34 @@ template <typename T> constexpr auto addr_if(const std::optional<T> &x) {
return x ? &*x : nullptr;
}
+namespace detail {
+struct DirectiveSpecificationScope {
+ using ODS = OmpDirectiveSpecification;
+ template <typename T> static const ODS &GetODS(const T &x) {
+ if constexpr ( //
+ std::is_base_of_v<OmpBlockConstruct, T> ||
+ std::is_same_v<OpenMPLoopConstruct, T> ||
+ std::is_same_v<OpenMPSectionsConstruct, T>) {
+ return x.BeginDir();
+ } else if constexpr (WrapperTrait<T>) {
+ return GetODS(x.v);
+ } else if constexpr (UnionTrait<T>) {
+ return std::visit(
+ [](auto &&s) -> decltype(auto) { return GetODS(s); }, x.u);
+ } else {
+ static_assert(std::is_same_v<OpenMPSectionConstruct, T>);
+ llvm_unreachable("This function does not work for SECTION");
+ }
+ }
+ static inline const ODS &GetODS(const ODS &x) { return x; }
+};
+} // namespace detail
+
+const OmpDirectiveSpecification &GetOmpDirectiveSpecification(
+ const OpenMPConstruct &x);
+const OmpDirectiveSpecification &GetOmpDirectiveSpecification(
+ const OpenMPDeclarativeConstruct &x);
+
namespace detail {
struct DirectiveNameScope {
static OmpDirectiveName MakeName(CharBlock source = {},
diff --git a/flang/lib/Parser/openmp-utils.cpp b/flang/lib/Parser/openmp-utils.cpp
index a3bd266249f30..beb4d5c87757c 100644
--- a/flang/lib/Parser/openmp-utils.cpp
+++ b/flang/lib/Parser/openmp-utils.cpp
@@ -25,6 +25,24 @@
namespace Fortran::parser::omp {
+const OmpDirectiveSpecification &GetOmpDirectiveSpecification(
+ const OpenMPConstruct &x) {
+ return std::visit(
+ [](auto &&s) -> decltype(auto) {
+ return detail::DirectiveSpecificationScope::GetODS(s);
+ },
+ x.u);
+}
+
+const OmpDirectiveSpecification &GetOmpDirectiveSpecification(
+ const OpenMPDeclarativeConstruct &x) {
+ return std::visit(
+ [](auto &&s) -> decltype(auto) {
+ return detail::DirectiveSpecificationScope::GetODS(s);
+ },
+ x.u);
+}
+
std::string GetUpperName(llvm::omp::Clause id, unsigned version) {
llvm::StringRef name{llvm::omp::getOpenMPClauseName(id, version)};
return parser::ToUpperCaseLetters(name);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 33ea727343cf4..b4765bdeb009d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -336,6 +336,14 @@ class OmpUnitedTaskDesignatorChecker {
SemanticsContext &context_;
};
+bool OmpStructureChecker::IsAllowedClause(llvm::omp::Clause clauseId) {
+ if (GetDirectiveNest(ContextSelectorNest) > 0) {
+ return true;
+ }
+ return llvm::omp::isAllowedClauseForDirective(
+ GetContext().directive, clauseId, context_.langOptions().OpenMPVersion);
+}
+
bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) {
// Do not do clause checks while processing METADIRECTIVE.
// Context selectors can contain clauses that are not given as a part
@@ -772,6 +780,9 @@ void OmpStructureChecker::Enter(const parser::OpenMPConstruct &x) {
return CheckDirectiveSpelling(source, id);
});
parser::Walk(x, visitor);
+ if (GetOmpDirectiveName(x).v != llvm::omp::Directive::OMPD_section) {
+ dirStack_.push_back(&GetOmpDirectiveSpecification(x));
+ }
// Simd Construct with Ordered Construct Nesting check
// We cannot use CurrentDirectiveIsNested() here because
@@ -788,12 +799,15 @@ void OmpStructureChecker::Enter(const parser::OpenMPConstruct &x) {
}
}
-void OmpStructureChecker::Leave(const parser::OpenMPConstruct &) {
+void OmpStructureChecker::Leave(const parser::OpenMPConstruct &x) {
for (const auto &[sym, source] : deferredNonVariables_) {
context_.SayWithDecl(
*sym, source, "'%s' must be a variable"_err_en_US, sym->name());
}
deferredNonVariables_.clear();
+ if (GetOmpDirectiveName(x).v != llvm::omp::Directive::OMPD_section) {
+ dirStack_.pop_back();
+ }
}
void OmpStructureChecker::Enter(const parser::OpenMPDeclarativeConstruct &x) {
@@ -803,11 +817,13 @@ void OmpStructureChecker::Enter(const parser::OpenMPDeclarativeConstruct &x) {
});
parser::Walk(x, visitor);
+ dirStack_.push_back(&GetOmpDirectiveSpecification(x));
EnterDirectiveNest(DeclarativeNest);
}
void OmpStructureChecker::Leave(const parser::OpenMPDeclarativeConstruct &x) {
ExitDirectiveNest(DeclarativeNest);
+ dirStack_.pop_back();
}
void OmpStructureChecker::AddEndDirectiveClauses(
@@ -2010,6 +2026,50 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Allocate &x) {
}
}
}
+
+ // If the clause is not allowed, don't diagnose specific problems with it.
+ if (!IsAllowedClause(llvm::omp::Clause::OMPC_allocate)) {
+ return;
+ }
+
+ llvm::omp::Directive dirId{GetContext().directive};
+ // For any list item that is specified in the allocate clause on a directive
+ // other than the allocators directive, a data-sharing attribute clause that
+ // may create a private copy of that list item must be specified on the same
+ // directive.
+ if (dirId != llvm::omp::Directive::OMPD_allocators &&
+ dirId != llvm::omp::Directive::OMPD_section) {
+ static llvm::omp::Clause privatizingDSAClauses[] = {
+ llvm::omp::Clause::OMPC_private,
+ llvm::omp::Clause::OMPC_firstprivate,
+ llvm::omp::Clause::OMPC_lastprivate,
+ llvm::omp::Clause::OMPC_is_device_ptr,
+ llvm::omp::Clause::OMPC_use_device_ptr,
+ };
+ const parser::OmpDirectiveSpecification &spec{*dirStack_.back()};
+
+ std::set<const Symbol *> privatized;
+ for (auto dsaClause : privatizingDSAClauses) {
+ if (auto *found{parser::omp::FindClause(spec, dsaClause)}) {
+ for (auto &object : GetOmpObjectList(*found)->v) {
+ if (auto *symbol{GetObjectSymbol(object)}) {
+ privatized.insert(symbol);
+ }
+ }
+ }
+ }
+
+ for (auto &object : GetOmpObjectList(x)->v) {
+ if (auto *symbol{GetObjectSymbol(object)}) {
+ if (!privatized.count(symbol)) {
+ context_.Say(
+ GetObjectSource(object).value_or(GetContext().clauseSource),
+ "The ALLOCATE clause requires that '%s' must be listed in a private data-sharing attribute clause on the same directive"_err_en_US,
+ symbol->name());
+ }
+ }
+ }
+ }
}
void OmpStructureChecker::Enter(const parser::OpenMPDeclareMapperConstruct &x) {
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index d3bbab6156f8b..3425b9af61b4b 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -93,8 +93,6 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
void Enter(const parser::OpenMPConstruct &);
void Leave(const parser::OpenMPConstruct &);
- void Enter(const parser::OpenMPInteropConstruct &);
- void Leave(const parser::OpenMPInteropConstruct &);
void Enter(const parser::OpenMPDeclarativeConstruct &);
void Leave(const parser::OpenMPDeclarativeConstruct &);
@@ -112,6 +110,8 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
void Leave(const parser::OpenMPAssumeConstruct &);
void Enter(const parser::OpenMPDeclarativeAssumes &);
void Leave(const parser::OpenMPDeclarativeAssumes &);
+ void Enter(const parser::OpenMPInteropConstruct &);
+ void Leave(const parser::OpenMPInteropConstruct &);
void Enter(const parser::OmpBlockConstruct &);
void Leave(const parser::OmpBlockConstruct &);
void Leave(const parser::OmpBeginDirective &);
@@ -267,6 +267,7 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
const parser::OmpTraitSetSelector &, const parser::OmpTraitSelector &);
// check-omp-structure.cpp
+ bool IsAllowedClause(llvm::omp::Clause clauseId);
bool CheckAllowedClause(llvmOmpClause clause);
void CheckVariableListItem(const SymbolSourceMap &symbols);
void CheckDirectiveSpelling(
@@ -404,6 +405,10 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
std::vector<LoopConstruct> loopStack_;
// Scopes for scoping units.
std::vector<const Scope *> scopeStack_;
+ // Stack of directive specifications (except for SECTION).
+ // This is to allow visitor functions to see all specified clauses, since
+ // they are only recorded in DirContext as they are processed.
+ std::vector<const parser::OmpDirectiveSpecification *> dirStack_;
enum class PartKind : int {
// There are also other "parts", such as internal-subprogram-part, etc,
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index b97f7ce58a1c0..a110607c3c72e 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1082,7 +1082,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
Symbol::Flags dataCopyingAttributeFlags{
Symbol::Flag::OmpCopyIn, Symbol::Flag::OmpCopyPrivate};
- std::vector<const parser::Name *> allocateNames_; // on one directive
UnorderedSymbolSet privateDataSharingAttributeObjects_; // on one directive
UnorderedSymbolSet stmtFunctionExprSymbols_;
std::multimap<const parser::Label,
@@ -1101,11 +1100,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
};
std::vector<PartKind> partStack_;
- void AddAllocateName(const parser::Name *&object) {
- allocateNames_.push_back(object);
- }
- void ClearAllocateNames() { allocateNames_.clear(); }
-
void AddPrivateDataSharingAttributeObjects(SymbolRef object) {
privateDataSharingAttributeObjects_.insert(object);
}
@@ -2006,35 +2000,10 @@ bool OmpAttributeVisitor::Pre(const parser::OmpBlockConstruct &x) {
IssueNonConformanceWarning(dirId, dirSpec.source, 52);
ClearDataSharingAttributeObjects();
ClearPrivateDataSharingAttributeObjects();
- ClearAllocateNames();
return true;
}
void OmpAttributeVisitor::Post(const parser::OmpBlockConstruct &x) {
- const parser::OmpDirectiveSpecification &dirSpec{x.BeginDir()};
- llvm::omp::Directive dirId{dirSpec.DirId()};
- unsigned version{context_.langOptions().OpenMPVersion};
-
- if (llvm::omp::isPrivatizingConstruct(dirId, version)) {
- bool hasPrivate;
- for (const auto *allocName : allocateNames_) {
- hasPrivate = false;
- for (auto privateObj : privateDataSharingAttributeObjects_) {
- const Symbol &symbolPrivate{*privateObj};
- if (allocName->source == symbolPrivate.name()) {
- hasPrivate = true;
- break;
- }
- }
- if (!hasPrivate) {
- context_.Say(allocName->source,
- "The ALLOCATE clause requires that '%s' must be listed in a "
- "private "
- "data-sharing attribute clause on the same directive"_err_en_US,
- allocName->ToString());
- }
- }
- }
PopContext();
}
@@ -2931,10 +2900,6 @@ void OmpAttributeVisitor::ResolveOmpDesignator(
if (privateDataSharingAttributeFlags.test(ompFlag)) {
CheckObjectIsPrivatizable(*name, *symbol, ompFlag);
}
-
- if (ompFlag == Symbol::Flag::OmpAllocate) {
- AddAllocateName(name);
- }
}
// Save the original symbol. For privatizing clauses, ensure enclosing
// constructs properly capture the variable.
diff --git a/flang/test/Lower/OpenMP/sections.f90 b/flang/test/Lower/OpenMP/sections.f90
index 599e570597f3f..514a82d386948 100644
--- a/flang/test/Lower/OpenMP/sections.f90
+++ b/flang/test/Lower/OpenMP/sections.f90
@@ -12,13 +12,15 @@
!CHECK: %[[CONST_1:.*]] = arith.constant 4 : i64
!CHECK: %[[PRIVATE_ETA:.*]] = fir.alloca f32 {bindc_name = "eta", pinned, uniq_name = "_QFEeta"}
!CHECK: %[[PRIVATE_ETA_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_ETA]] {uniq_name = "_QFEeta"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+!CHECK: %[[PRIVATE_COUNT:.*]] = fir.alloca i32 {bindc_name = "count", pinned, uniq_name = "_QFEcount"}
+!CHECK: %[[PRIVATE_COUNT_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_COUNT]] {uniq_name = "_QFEcount"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
!CHECK: %[[PRIVATE_DOUBLE_COUNT:.*]] = fir.alloca i32 {bindc_name = "double_count", pinned, uniq_name = "_QFEdouble_count"}
!CHECK: %[[PRIVATE_DOUBLE_COUNT_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_DOUBLE_COUNT]] {uniq_name = "_QFEdouble_count"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
!CHECK: omp.sections allocate(%[[CONST_1]] : i64 -> %[[COUNT_DECL]]#0 : !fir.ref<i32>) {
!CHECK: omp.section {
!CHECK: %[[CONST5:.*]] = arith.constant 5 : i32
-!CHECK: hlfir.assign %[[CONST5]] to %[[COUNT_DECL]]#0 : i32, !fir.ref<i32>
-!CHECK: %[[TEMP_COUNT:.*]] = fir.load %[[COUNT_DECL]]#0 : !fir.ref<i32>
+!CHECK: hlfir.assign %[[CONST5]] to %[[PRIVATE_COUNT_DECL]]#0 : i32, !fir.ref<i32>
+!CHECK: %[[TEMP_COUNT:.*]] = fir.load %[[PRIVATE_COUNT_DECL]]#0 : !fir.ref<i32>
!CHECK: %[[TEMP_DOUBLE_COUNT:.*]] = fir.load %[[PRIVATE_DOUBLE_COUNT_DECL]]#0 : !fir.ref<i32>
!CHECK: %[[RESULT:.*]] = arith.muli %[[TEMP_COUNT]], %[[TEMP_DOUBLE_COUNT]] : i32
!CHECK: %[[RESULT_CONVERT:.*]] = fir.convert %[[RESULT]] : (i32) -> f32
@@ -37,13 +39,13 @@
!CHECK: %[[CONST:.*]] = arith.constant 7.000000e+00 : f32
!CHECK: %[[RESULT:.*]] = arith.subf %[[TEMP]], %[[CONST]] {{.*}}: f32
!CHECK: hlfir.assign %[[RESULT]] to %[[PRIVATE_ETA_DECL]]#0 : f32, !fir.ref<f32>
-!CHECK: %[[TEMP_COUNT1:.*]] = fir.load %[[COUNT_DECL]]#0 : !fir.ref<i32>
+!CHECK: %[[TEMP_COUNT1:.*]] = fir.load %[[PRIVATE_COUNT_DECL]]#0 : !fir.ref<i32>
!CHECK: %[[TEMP_COUNT:.*]] = fir.convert %[[TEMP_COUNT1]] : (i32) -> f32
!CHECK: %[[TEMP_ETA:.*]] = fir.load %[[PRIVATE_ETA_DECL]]#0 : !fir.ref<f32>
!CHECK: %[[TEMP_COUNT2:.*]] = arith.mulf %[[TEMP_COUNT]], %[[TEMP_ETA]] {{.*}}: f32
!CHECK: %[[RESULT:.*]] = fir.convert %[[TEMP_COUNT2]] : (f32) -> i32
-!CHECK: hlfir.assign %[[RESULT]] to %[[COUNT_DECL]]#0 : i32, !fir.ref<i32>
-!CHECK: %[[TEMP_COUNT3:.*]] = fir.load %[[COUNT_DECL]]#0 : !fir.ref<i32>
+!CHECK: hlfir.assign %[[RESULT]] to %[[PRIVATE_COUNT_DECL]]#0 : i32, !fir.ref<i32>
+!CHECK: %[[TEMP_COUNT3:.*]] = fir.load %[[PRIVATE_COUNT_DECL]]#0 : !fir.ref<i32>
!CHECK: %[[TEMP_COUNT4:.*]] = fir.convert %[[TEMP_COUNT3]] : (i32) -> f32
!CHECK: %[[TEMP_ETA:.*]] = fir.load %[[PRIVATE_ETA_DECL]]#0 : !fir.ref<f32>
!CHECK: %[[TEMP_COUNT5:.*]] = arith.subf %[[TEMP_COUNT4]], %[[TEMP_ETA]] {{.*}}: f32
@@ -62,7 +64,7 @@
program sample
use omp_lib
integer :: count = 0, double_count = 1
- !$omp sections private (eta, double_count) allocate(omp_high_bw_mem_alloc: count)
+ !$omp sections private (eta, count, double_count) allocate(omp_high_bw_mem_alloc: count)
!$omp section
count = 1 + 4
eta = count * double_count
diff --git a/flang/test/Semantics/OpenMP/taskgroup01.f90 b/flang/test/Semantics/OpenMP/taskgroup01.f90
index ded5d47525af4..afeaf462659ee 100644
--- a/flang/test/Semantics/OpenMP/taskgroup01.f90
+++ b/flang/test/Semantics/OpenMP/taskgroup01.f90
@@ -21,9 +21,11 @@
!$omp end parallel
!$omp parallel private(xyz)
+ !ERROR: The ALLOCATE clause requires that 'xyz' must be listed in a private data-sharing attribute clause on the same directive
!$omp taskgroup allocate(xyz)
!$omp task
print *, "The "
+ !ERROR: The ALLOCATE clause requires that 'abc' must be listed in a private data-sharing attribute clause on the same directive
!$omp taskgroup allocate(omp_large_cap_mem_space: abc)
!$omp task
print *, "almighty sun"
>From 54257d3b3eecf14ceb83b816a69c7d5484a0a825 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Sat, 18 Apr 2026 10:42:58 -0500
Subject: [PATCH 2/3] [flang][OpenMP] Move directive deprecation check to
semantic checks
Replace IssueNonConformanceWarning in resolve-directives.cpp with
CheckDirectiveDeprecation in check-omp-structure.cpp.
---
flang/lib/Semantics/check-omp-structure.cpp | 31 ++++++++++
flang/lib/Semantics/check-omp-structure.h | 1 +
flang/lib/Semantics/resolve-directives.cpp | 59 -------------------
.../Semantics/OpenMP/clause-validity01.f90 | 4 +-
flang/test/Semantics/OpenMP/deprecation.f90 | 12 ++--
5 files changed, 40 insertions(+), 67 deletions(-)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index b4765bdeb009d..849c0bd876d07 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -569,6 +569,34 @@ void OmpStructureChecker::CheckDirectiveSpelling(
}
}
+void OmpStructureChecker::CheckDirectiveDeprecation(
+ const parser::OpenMPConstruct &x) {
+ parser::OmpDirectiveName dirName{GetOmpDirectiveName(x)};
+ unsigned version{context_.langOptions().OpenMPVersion};
+ // We only want to emit the warning when the version being used has the
+ // directive deprecated
+ if (version >= 52) {
+ // Check MASTER.
+ llvm::SmallVector<llvm::omp::Directive> leafs{
+ llvm::omp::getLeafConstructsOrSelf(dirName.v)};
+ if (llvm::is_contained(leafs, llvm::omp::Directive::OMPD_master)) {
+ for (auto &id : leafs) {
+ if (id == llvm::omp::Directive::OMPD_master) {
+ id = llvm::omp::Directive::OMPD_masked;
+ }
+ }
+
+ auto preferredId{llvm::omp::getCompoundConstruct(leafs)};
+ context_.Warn(common::UsageWarning::OpenMPUsage, dirName.source,
+ "OpenMP directive %s has been deprecated, please use %s instead"_warn_en_US,
+ GetUpperName(dirName.v, version), GetUpperName(preferredId, version));
+ }
+ }
+
+ // Executable allocate is checked separately because these can be nested in
+ // one another, but only the top-level directive should cause a warning.
+}
+
void OmpStructureChecker::CheckMultipleOccurrence(
semantics::UnorderedSymbolSet &listVars,
const std::list<parser::Name> &nameList, const parser::CharBlock &item,
@@ -780,6 +808,9 @@ void OmpStructureChecker::Enter(const parser::OpenMPConstruct &x) {
return CheckDirectiveSpelling(source, id);
});
parser::Walk(x, visitor);
+
+ CheckDirectiveDeprecation(x);
+
if (GetOmpDirectiveName(x).v != llvm::omp::Directive::OMPD_section) {
dirStack_.push_back(&GetOmpDirectiveSpecification(x));
}
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 3425b9af61b4b..ab4e32858ca53 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -272,6 +272,7 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
void CheckVariableListItem(const SymbolSourceMap &symbols);
void CheckDirectiveSpelling(
parser::CharBlock spelling, llvm::omp::Directive id);
+ void CheckDirectiveDeprecation(const parser::OpenMPConstruct &x);
void AnalyzeObject(const parser::OmpObject &object);
void AnalyzeObjects(const parser::OmpObjectList &objects);
void CheckMultipleOccurrence(semantics::UnorderedSymbolSet &listVars,
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index a110607c3c72e..40e2e9f6adcd9 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1144,8 +1144,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
void AddOmpRequiresToScope(Scope &,
const WithOmpDeclarative::RequiresClauses *,
const common::OmpMemoryOrderType *);
- void IssueNonConformanceWarning(llvm::omp::Directive D,
- parser::CharBlock source, unsigned EmitFromVersion);
void CreateImplicitSymbols(const parser::Name &, const Symbol *symbol);
@@ -1994,10 +1992,6 @@ bool OmpAttributeVisitor::Pre(const parser::OmpBlockConstruct &x) {
const parser::OmpDirectiveSpecification &dirSpec{x.BeginDir()};
llvm::omp::Directive dirId{dirSpec.DirId()};
PushContext(dirSpec.source, dirId);
-
- if (dirId == llvm::omp::Directive::OMPD_master ||
- dirId == llvm::omp::Directive::OMPD_parallel_master)
- IssueNonConformanceWarning(dirId, dirSpec.source, 52);
ClearDataSharingAttributeObjects();
ClearPrivateDataSharingAttributeObjects();
return true;
@@ -2019,14 +2013,6 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
const parser::OmpDirectiveSpecification &beginSpec{x.BeginDir()};
const parser::OmpDirectiveName &beginName{beginSpec.DirName()};
PushContext(beginName.source, beginName.v);
-
- if (beginName.v == llvm::omp::OMPD_master_taskloop ||
- beginName.v == llvm::omp::OMPD_master_taskloop_simd ||
- beginName.v == llvm::omp::OMPD_parallel_master_taskloop ||
- beginName.v == llvm::omp::OMPD_parallel_master_taskloop_simd) {
- unsigned version{context_.langOptions().OpenMPVersion};
- IssueNonConformanceWarning(beginName.v, beginName.source, version);
- }
ClearDataSharingAttributeObjects();
if (beginName.v == llvm::omp::Directive::OMPD_do) {
@@ -3342,51 +3328,6 @@ void OmpAttributeVisitor::AddOmpRequiresToScope(Scope &scope,
}
}
-void OmpAttributeVisitor::IssueNonConformanceWarning(llvm::omp::Directive D,
- parser::CharBlock source, unsigned EmitFromVersion) {
- std::string warnStr;
- llvm::raw_string_ostream warnStrOS(warnStr);
- unsigned version{context_.langOptions().OpenMPVersion};
- // We only want to emit the warning when the version being used has the
- // directive deprecated
- if (version < EmitFromVersion) {
- return;
- }
- warnStrOS << "OpenMP directive " << parser::omp::GetUpperName(D, version)
- << " has been deprecated";
-
- auto setAlternativeStr = [&warnStrOS](llvm::StringRef alt) {
- warnStrOS << ", please use " << alt << " instead.";
- };
- switch (D) {
- case llvm::omp::OMPD_master:
- setAlternativeStr("MASKED");
- break;
- case llvm::omp::OMPD_master_taskloop:
- setAlternativeStr("MASKED TASKLOOP");
- break;
- case llvm::omp::OMPD_master_taskloop_simd:
- setAlternativeStr("MASKED TASKLOOP SIMD");
- break;
- case llvm::omp::OMPD_parallel_master:
- setAlternativeStr("PARALLEL MASKED");
- break;
- case llvm::omp::OMPD_parallel_master_taskloop:
- setAlternativeStr("PARALLEL MASKED TASKLOOP");
- break;
- case llvm::omp::OMPD_parallel_master_taskloop_simd:
- setAlternativeStr("PARALLEL_MASKED TASKLOOP SIMD");
- break;
- case llvm::omp::OMPD_allocate:
- setAlternativeStr("ALLOCATORS");
- break;
- default:
- break;
- }
- context_.Warn(common::UsageWarning::OpenMPUsage, source, "%s"_warn_en_US,
- warnStrOS.str());
-}
-
#ifndef NDEBUG
static llvm::raw_ostream &operator<<(
diff --git a/flang/test/Semantics/OpenMP/clause-validity01.f90 b/flang/test/Semantics/OpenMP/clause-validity01.f90
index 9371a2fc6fd58..7ba40ba1f8313 100644
--- a/flang/test/Semantics/OpenMP/clause-validity01.f90
+++ b/flang/test/Semantics/OpenMP/clause-validity01.f90
@@ -483,14 +483,14 @@
! 2.13.1 master
!$omp parallel
- !WARNING: OpenMP directive MASTER has been deprecated, please use MASKED instead. [-Wopen-mp-usage]
+ !WARNING: OpenMP directive MASTER has been deprecated, please use MASKED instead [-Wopen-mp-usage]
!$omp master
a=3.14
!$omp end master
!$omp end parallel
!$omp parallel
- !WARNING: OpenMP directive MASTER has been deprecated, please use MASKED instead. [-Wopen-mp-usage]
+ !WARNING: OpenMP directive MASTER has been deprecated, please use MASKED instead [-Wopen-mp-usage]
!ERROR: NUM_THREADS clause is not allowed on the MASTER directive
!$omp master num_threads(4)
a=3.14
diff --git a/flang/test/Semantics/OpenMP/deprecation.f90 b/flang/test/Semantics/OpenMP/deprecation.f90
index 4acccb1bf4b7f..a5f71413ab9c5 100644
--- a/flang/test/Semantics/OpenMP/deprecation.f90
+++ b/flang/test/Semantics/OpenMP/deprecation.f90
@@ -4,7 +4,7 @@
subroutine test_master()
integer :: c = 1
-!WARNING: OpenMP directive MASTER has been deprecated, please use MASKED instead. [-Wopen-mp-usage]
+!WARNING: OpenMP directive MASTER has been deprecated, please use MASKED instead [-Wopen-mp-usage]
!$omp master
c = c + 1
!$omp end master
@@ -12,7 +12,7 @@ subroutine test_master()
subroutine test_parallel_master
integer :: c = 2
-!WARNING: OpenMP directive PARALLEL MASTER has been deprecated, please use PARALLEL MASKED instead. [-Wopen-mp-usage]
+!WARNING: OpenMP directive PARALLEL MASTER has been deprecated, please use PARALLEL MASKED instead [-Wopen-mp-usage]
!$omp parallel master
c = c + 2
!$omp end parallel master
@@ -20,7 +20,7 @@ subroutine test_parallel_master
subroutine test_master_taskloop_simd()
integer :: i, j = 1
-!WARNING: OpenMP directive MASTER TASKLOOP SIMD has been deprecated, please use MASKED TASKLOOP SIMD instead. [-Wopen-mp-usage]
+!WARNING: OpenMP directive MASTER TASKLOOP SIMD has been deprecated, please use MASKED TASKLOOP SIMD instead [-Wopen-mp-usage]
!$omp master taskloop simd
do i=1,10
j = j + 1
@@ -30,7 +30,7 @@ subroutine test_master_taskloop_simd()
subroutine test_master_taskloop
integer :: i, j = 1
-!WARNING: OpenMP directive MASTER TASKLOOP has been deprecated, please use MASKED TASKLOOP instead. [-Wopen-mp-usage]
+!WARNING: OpenMP directive MASTER TASKLOOP has been deprecated, please use MASKED TASKLOOP instead [-Wopen-mp-usage]
!$omp master taskloop
do i=1,10
j = j + 1
@@ -40,7 +40,7 @@ subroutine test_master_taskloop
subroutine test_parallel_master_taskloop_simd
integer :: i, j = 1
-!WARNING: OpenMP directive PARALLEL MASTER TASKLOOP SIMD has been deprecated, please use PARALLEL_MASKED TASKLOOP SIMD instead. [-Wopen-mp-usage]
+!WARNING: OpenMP directive PARALLEL MASTER TASKLOOP SIMD has been deprecated, please use PARALLEL MASKED TASKLOOP SIMD instead [-Wopen-mp-usage]
!$omp parallel master taskloop simd
do i=1,10
j = j + 1
@@ -50,7 +50,7 @@ subroutine test_parallel_master_taskloop_simd
subroutine test_parallel_master_taskloop
integer :: i, j = 1
-!WARNING: OpenMP directive PARALLEL MASTER TASKLOOP has been deprecated, please use PARALLEL MASKED TASKLOOP instead. [-Wopen-mp-usage]
+!WARNING: OpenMP directive PARALLEL MASTER TASKLOOP has been deprecated, please use PARALLEL MASKED TASKLOOP instead [-Wopen-mp-usage]
!$omp parallel master taskloop
do i=1,10
j = j + 1
>From e838b31ad6dbf83d0e5b102dce9ae3ee0810c20f Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 21 Apr 2026 08:11:08 -0500
Subject: [PATCH 3/3] [flang][OpenMP] Move branching verification to semantic
checks
Move the check for branching into and out of an OpenMP construct from
symbol resolution into semantic checks.
Instead of using directive contexts to check for crossing a construct
boundary, use construct pointers and source ranges.
---
flang/lib/Semantics/check-omp-loop.cpp | 20 +--
flang/lib/Semantics/check-omp-structure.cpp | 148 ++++++++++++++++--
flang/lib/Semantics/check-omp-structure.h | 61 +++++++-
flang/lib/Semantics/resolve-directives.cpp | 132 ----------------
.../Semantics/OpenMP/parallel-master-goto.f90 | 1 -
5 files changed, 199 insertions(+), 163 deletions(-)
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index 1299686a99ad2..f81dd6d0afd8d 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -380,8 +380,6 @@ void OmpStructureChecker::CheckNestedConstruct(
}
void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
- loopStack_.push_back(&x);
-
const parser::OmpDirectiveName &beginName{x.BeginDir().DirName()};
PushContextAndClauseSets(beginName.source, beginName.v);
@@ -619,14 +617,6 @@ void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
ExitDirectiveNest(SIMDNest);
}
dirContext_.pop_back();
-
- assert(!loopStack_.empty() && "Expecting non-empty loop stack");
-#ifndef NDEBUG
- const LoopConstruct &top{loopStack_.back()};
- auto *loopc{std::get_if<const parser::OpenMPLoopConstruct *>(&top)};
- assert(loopc != nullptr && *loopc == &x && "Mismatched loop constructs");
-#endif
- loopStack_.pop_back();
}
void OmpStructureChecker::Enter(const parser::OmpEndLoopDirective &x) {
@@ -840,17 +830,17 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Looprange &x) {
void OmpStructureChecker::Enter(const parser::DoConstruct &x) {
Base::Enter(x);
- loopStack_.push_back(&x);
+ constructStack_.push_back(&x);
}
void OmpStructureChecker::Leave(const parser::DoConstruct &x) {
- assert(!loopStack_.empty() && "Expecting non-empty loop stack");
+ assert(!constructStack_.empty() && "Expecting non-empty construct stack");
#ifndef NDEBUG
- const LoopConstruct &top = loopStack_.back();
+ const LoopOrConstruct &top = constructStack_.back();
auto *doc{std::get_if<const parser::DoConstruct *>(&top)};
- assert(doc != nullptr && *doc == &x && "Mismatched loop constructs");
+ assert(doc != nullptr && *doc == &x && "Mismatched constructs");
#endif
- loopStack_.pop_back();
+ constructStack_.pop_back();
Base::Leave(x);
}
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 8b5cd35929689..d01d603156402 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -179,6 +179,14 @@ void OmpStructureChecker::Leave(const parser::BlockConstruct &x) {
}
}
+void OmpStructureChecker::Enter(const parser::InternalSubprogram &) {
+ ClearLabels();
+}
+
+void OmpStructureChecker::Enter(const parser::ModuleSubprogram &) {
+ ClearLabels();
+}
+
void OmpStructureChecker::Enter(const parser::SpecificationPart &) {
partStack_.push_back(PartKind::SpecificationPart);
}
@@ -428,6 +436,119 @@ void OmpStructureChecker::AnalyzeObjects(const parser::OmpObjectList &objects) {
}
}
+const parser::OpenMPConstruct *
+OmpStructureChecker::GetCurrentConstruct() const {
+ for (const LoopOrConstruct &c : llvm::reverse(constructStack_)) {
+ if (auto *omp{std::get_if<const parser::OpenMPConstruct *>(&c)}) {
+ return *omp;
+ }
+ }
+ return nullptr;
+}
+
+void OmpStructureChecker::CheckSourceLabel(const parser::Label &label) {
+ // Get the context to check if the statement causing a jump to the 'label' is
+ // in an enclosing OpenMP construct
+ const parser::OpenMPConstruct *thisConstruct{GetCurrentConstruct()};
+ sourceLabels_.emplace(
+ label, std::make_pair(currentStatementSource_, thisConstruct));
+ // Check if the statement with 'label' to which a jump is being introduced
+ // has already been encountered
+ auto it{targetLabels_.find(label)};
+ if (it != targetLabels_.end()) {
+ // Check if both the statement with 'label' and the statement that causes a
+ // jump to the 'label' are in the same block.
+ CheckLabelContext(currentStatementSource_, it->second.first, thisConstruct,
+ it->second.second);
+ }
+}
+
+// Check for invalid branch into or out of OpenMP structured blocks
+void OmpStructureChecker::CheckLabelContext(const parser::CharBlock source,
+ const parser::CharBlock target, const parser::OpenMPConstruct *srcOmp,
+ const parser::OpenMPConstruct *tgtOmp) {
+ auto isSameOrIncludes = [&](const parser::OpenMPConstruct *lhs,
+ const parser::OpenMPConstruct *rhs) -> bool {
+ assert(lhs && "Expecting first argument");
+ if (!rhs) {
+ return false;
+ }
+ if (lhs == rhs) {
+ return true;
+ }
+
+ auto getSource{[](const parser::OpenMPConstruct &c) -> parser::CharBlock {
+ if (auto src{parser::GetSource(c)}) {
+ return *src;
+ }
+ return {};
+ }};
+
+ return getSource(*lhs).Contains(getSource(*rhs));
+ };
+
+ unsigned version{context_.langOptions().OpenMPVersion};
+ if (tgtOmp && !isSameOrIncludes(tgtOmp, srcOmp)) {
+ parser::OmpDirectiveName name{GetOmpDirectiveName(*tgtOmp)};
+ context_
+ .Say(source, "invalid branch into an OpenMP structured block"_err_en_US)
+ .Attach(target, "In the enclosing %s directive branched into"_en_US,
+ parser::omp::GetUpperName(name.v, version));
+ }
+ if (srcOmp && !isSameOrIncludes(srcOmp, tgtOmp)) {
+ parser::OmpDirectiveName name{GetOmpDirectiveName(*srcOmp)};
+ context_
+ .Say(source,
+ "invalid branch leaving an OpenMP structured block"_err_en_US)
+ .Attach(target, "Outside the enclosing %s directive"_en_US,
+ parser::omp::GetUpperName(name.v, version));
+ }
+}
+
+// Keep track of labels in the statements that causes jumps to target labels
+void OmpStructureChecker::Leave(const parser::GotoStmt &x) {
+ CheckSourceLabel(x.v);
+}
+
+void OmpStructureChecker::Leave(const parser::ComputedGotoStmt &x) {
+ for (auto &label : std::get<std::list<parser::Label>>(x.t)) {
+ CheckSourceLabel(label);
+ }
+}
+
+void OmpStructureChecker::Leave(const parser::ArithmeticIfStmt &x) {
+ CheckSourceLabel(std::get<1>(x.t));
+ CheckSourceLabel(std::get<2>(x.t));
+ CheckSourceLabel(std::get<3>(x.t));
+}
+
+void OmpStructureChecker::Leave(const parser::AssignedGotoStmt &x) {
+ for (auto &label : std::get<std::list<parser::Label>>(x.t)) {
+ CheckSourceLabel(label);
+ }
+}
+
+void OmpStructureChecker::Leave(const parser::AltReturnSpec &x) {
+ CheckSourceLabel(x.v);
+}
+
+void OmpStructureChecker::Leave(const parser::ErrLabel &x) {
+ CheckSourceLabel(x.v);
+}
+
+void OmpStructureChecker::Leave(const parser::EndLabel &x) {
+ CheckSourceLabel(x.v);
+}
+
+void OmpStructureChecker::Leave(const parser::EorLabel &x) {
+ CheckSourceLabel(x.v);
+}
+
+void OmpStructureChecker::ClearLabels() {
+ sourceLabels_.clear();
+ targetLabels_.clear();
+}
+
bool OmpStructureChecker::IsCloselyNestedRegion(const OmpDirectiveSet &set) {
// Definition of close nesting:
//
@@ -805,6 +926,8 @@ template <typename T>
DirectiveSpellingVisitor(T &&) -> DirectiveSpellingVisitor<T>;
void OmpStructureChecker::Enter(const parser::OpenMPConstruct &x) {
+ constructStack_.push_back(&x);
+
DirectiveSpellingVisitor visitor(
[this](parser::CharBlock source, llvm::omp::Directive id) {
return CheckDirectiveSpelling(source, id);
@@ -844,6 +967,7 @@ void OmpStructureChecker::Leave(const parser::OpenMPConstruct &x) {
if (GetOmpDirectiveName(x).v != llvm::omp::Directive::OMPD_section) {
dirStack_.pop_back();
}
+ constructStack_.pop_back();
}
void OmpStructureChecker::Enter(const parser::OpenMPDeclarativeConstruct &x) {
@@ -4757,8 +4881,8 @@ void OmpStructureChecker::CheckDoacross(const parser::OmpDoacross &doa) {
// number of DO constructs on the stack. This is checked elsewhere.
std::set<const Symbol *> inductionVars;
- for (const LoopConstruct &loop : llvm::reverse(loopStack_)) {
- if (auto *doc{std::get_if<const parser::DoConstruct *>(&loop)}) {
+ for (const LoopOrConstruct &c : llvm::reverse(constructStack_)) {
+ if (auto *doc{std::get_if<const parser::DoConstruct *>(&c)}) {
// Do-construct, collect the induction variable.
if (auto &control{(*doc)->GetLoopControl()}) {
if (auto *b{std::get_if<parser::LoopControl::Bounds>(&control->u)}) {
@@ -4767,15 +4891,17 @@ void OmpStructureChecker::CheckDoacross(const parser::OmpDoacross &doa) {
}
} else {
// Omp-loop-construct, check if it's do/simd with an ORDERED clause.
- auto *loopc{std::get_if<const parser::OpenMPLoopConstruct *>(&loop)};
- assert(loopc && "Expecting OpenMPLoopConstruct");
- const parser::OmpDirectiveSpecification &beginSpec{(*loopc)->BeginDir()};
- llvm::omp::Directive loopDir{beginSpec.DirId()};
- if (loopDir == llvm::omp::OMPD_do || loopDir == llvm::omp::OMPD_simd) {
- // If it has ORDERED clause, stop the traversal.
- if (parser::omp::FindClause(
- beginSpec, llvm::omp::Clause::OMPC_ordered)) {
- break;
+ auto *omp{std::get_if<const parser::OpenMPConstruct *>(&c)};
+ assert(omp && "Expecting OpenMPConstruct");
+ if (auto *loop{parser::Unwrap<parser::OpenMPLoopConstruct>(*omp)}) {
+ const parser::OmpDirectiveSpecification &beginSpec{loop->BeginDir()};
+ llvm::omp::Directive loopDir{beginSpec.DirId()};
+ if (loopDir == llvm::omp::OMPD_do || loopDir == llvm::omp::OMPD_simd) {
+ // If it has ORDERED clause, stop the traversal.
+ if (parser::omp::FindClause(
+ beginSpec, llvm::omp::Clause::OMPC_ordered)) {
+ break;
+ }
}
}
}
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index ab4e32858ca53..53ef2ee8251c5 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -85,6 +85,8 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
bool Enter(const parser::EndFunctionStmt &);
bool Enter(const parser::BlockConstruct &);
void Leave(const parser::BlockConstruct &);
+ void Enter(const parser::InternalSubprogram &);
+ void Enter(const parser::ModuleSubprogram &);
void Enter(const parser::SpecificationPart &);
void Leave(const parser::SpecificationPart &);
@@ -180,10 +182,23 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
void Enter(const parser::OmpContextSelector &);
void Leave(const parser::OmpContextSelector &);
+ template <typename A> void Enter(const parser::Statement<A> &);
+ void Leave(const parser::GotoStmt &);
+ void Leave(const parser::ComputedGotoStmt &);
+ void Leave(const parser::ArithmeticIfStmt &);
+ void Leave(const parser::AssignedGotoStmt &);
+ void Leave(const parser::AltReturnSpec &);
+ void Leave(const parser::ErrLabel &);
+ void Leave(const parser::EndLabel &);
+ void Leave(const parser::EorLabel &);
+
#define GEN_FLANG_CLAUSE_CHECK_ENTER
#include "llvm/Frontend/OpenMP/OMP.inc"
private:
+ using LoopOrConstruct = std::variant<const parser::DoConstruct *,
+ const parser::OpenMPConstruct *>;
+
// Most of these functions are defined in check-omp-structure.cpp, but
// some groups have their own files.
@@ -275,6 +290,12 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
void CheckDirectiveDeprecation(const parser::OpenMPConstruct &x);
void AnalyzeObject(const parser::OmpObject &object);
void AnalyzeObjects(const parser::OmpObjectList &objects);
+
+ const parser::OpenMPConstruct *GetCurrentConstruct() const;
+ void CheckSourceLabel(const parser::Label &);
+ void CheckLabelContext(const parser::CharBlock, const parser::CharBlock,
+ const parser::OpenMPConstruct *, const parser::OpenMPConstruct *);
+ void ClearLabels();
void CheckMultipleOccurrence(semantics::UnorderedSymbolSet &listVars,
const std::list<parser::Name> &nameList, const parser::CharBlock &item,
const std::string &clauseName);
@@ -401,14 +422,15 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
parser::CharBlock visitedAtomicSource_;
SymbolSourceMap deferredNonVariables_;
- using LoopConstruct = std::variant<const parser::DoConstruct *,
- const parser::OpenMPLoopConstruct *>;
- std::vector<LoopConstruct> loopStack_;
+ // Stack of nested DO loops and OpenMP constructs.
+ // This is used to verify DO loop nest for DOACROSS, and branches into
+ // and out of OpenMP constructs.
+ std::vector<LoopOrConstruct> constructStack_;
// Scopes for scoping units.
std::vector<const Scope *> scopeStack_;
// Stack of directive specifications (except for SECTION).
// This is to allow visitor functions to see all specified clauses, since
- // they are only recorded in DirContext as they are processed.
+ // they are only recorded in DirectiveContext as they are processed.
std::vector<const parser::OmpDirectiveSpecification *> dirStack_;
enum class PartKind : int {
@@ -418,8 +440,39 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
ExecutionPart,
};
std::vector<PartKind> partStack_;
+
+ std::multimap<const parser::Label,
+ std::pair<parser::CharBlock, const parser::OpenMPConstruct *>>
+ sourceLabels_;
+ std::map<const parser::Label,
+ std::pair<parser::CharBlock, const parser::OpenMPConstruct *>>
+ targetLabels_;
+ parser::CharBlock currentStatementSource_;
};
+template <typename A>
+void OmpStructureChecker::Enter(const parser::Statement<A> &statement) {
+ currentStatementSource_ = statement.source;
+ // Keep track of the labels in all the labelled statements
+ if (statement.label) {
+ auto label{statement.label.value()};
+ // Get the context to check if the labelled statement is in an
+ // enclosing OpenMP construct
+ auto *thisConstruct{GetCurrentConstruct()};
+ targetLabels_.emplace(
+ label, std::make_pair(currentStatementSource_, thisConstruct));
+ // Check if a statement that causes a jump to the 'label'
+ // has already been encountered
+ auto range{sourceLabels_.equal_range(label)};
+ for (auto it{range.first}; it != range.second; ++it) {
+ // Check if both the statement with 'label' and the statement that
+ // causes a jump to the 'label' are in the same scope
+ CheckLabelContext(it->second.first, currentStatementSource_,
+ it->second.second, thisConstruct);
+ }
+ }
+}
+
/// Find a duplicate entry in the range, and return an iterator to it.
/// If there are no duplicate entries, return nullopt.
template <typename LessTy, typename RangeTy, typename IterTy>
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 40e2e9f6adcd9..cbd91e70c5ce5 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -80,11 +80,6 @@ template <typename T> class DirectiveAttributeVisitor {
CHECK(!dirContext_.empty());
return dirContext_.back();
}
- std::optional<DirContext> GetContextIf() {
- return dirContext_.empty()
- ? std::nullopt
- : std::make_optional<DirContext>(dirContext_.back());
- }
void PushContext(const parser::CharBlock &source, T dir, Scope &scope) {
if constexpr (std::is_same_v<T, llvm::acc::Directive>) {
dirContext_.emplace_back(source, dir, scope);
@@ -477,29 +472,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
template <typename A> bool Pre(const A &) { return true; }
template <typename A> void Post(const A &) {}
- template <typename A> bool Pre(const parser::Statement<A> &statement) {
- currentStatementSource_ = statement.source;
- // Keep track of the labels in all the labelled statements
- if (statement.label) {
- auto label{statement.label.value()};
- // Get the context to check if the labelled statement is in an
- // enclosing OpenMP construct
- std::optional<DirContext> thisContext{GetContextIf()};
- targetLabels_.emplace(
- label, std::make_pair(currentStatementSource_, thisContext));
- // Check if a statement that causes a jump to the 'label'
- // has already been encountered
- auto range{sourceLabels_.equal_range(label)};
- for (auto it{range.first}; it != range.second; ++it) {
- // Check if both the statement with 'label' and the statement that
- // causes a jump to the 'label' are in the same scope
- CheckLabelContext(it->second.first, currentStatementSource_,
- it->second.second, thisContext);
- }
- }
- return true;
- }
-
bool Pre(const parser::SpecificationPart &) {
partStack_.push_back(PartKind::SpecificationPart);
return true;
@@ -512,18 +484,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
}
void Post(const parser::ExecutionPart &) { partStack_.pop_back(); }
- bool Pre(const parser::InternalSubprogram &) {
- // Clear the labels being tracked in the previous scope
- ClearLabels();
- return true;
- }
-
- bool Pre(const parser::ModuleSubprogram &) {
- // Clear the labels being tracked in the previous scope
- ClearLabels();
- return true;
- }
-
bool Pre(const parser::StmtFunctionStmt &x) {
const auto &parsedExpr{std::get<parser::Scalar<parser::Expr>>(x.t)};
if (const auto *expr{GetExpr(context_, parsedExpr)}) {
@@ -920,30 +880,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
void Post(const parser::Name &);
- // Keep track of labels in the statements that causes jumps to target labels
- void Post(const parser::GotoStmt &gotoStmt) { CheckSourceLabel(gotoStmt.v); }
- void Post(const parser::ComputedGotoStmt &computedGotoStmt) {
- for (auto &label : std::get<std::list<parser::Label>>(computedGotoStmt.t)) {
- CheckSourceLabel(label);
- }
- }
- void Post(const parser::ArithmeticIfStmt &arithmeticIfStmt) {
- CheckSourceLabel(std::get<1>(arithmeticIfStmt.t));
- CheckSourceLabel(std::get<2>(arithmeticIfStmt.t));
- CheckSourceLabel(std::get<3>(arithmeticIfStmt.t));
- }
- void Post(const parser::AssignedGotoStmt &assignedGotoStmt) {
- for (auto &label : std::get<std::list<parser::Label>>(assignedGotoStmt.t)) {
- CheckSourceLabel(label);
- }
- }
- void Post(const parser::AltReturnSpec &altReturnSpec) {
- CheckSourceLabel(altReturnSpec.v);
- }
- void Post(const parser::ErrLabel &errLabel) { CheckSourceLabel(errLabel.v); }
- void Post(const parser::EndLabel &endLabel) { CheckSourceLabel(endLabel.v); }
- void Post(const parser::EorLabel &eorLabel) { CheckSourceLabel(eorLabel.v); }
-
void ResolveOmpObjectsForMapClause(
Symbol::Flag mapFlag, const parser::OmpObjectList &objList) {
for (const auto &ompObj : objList.v) {
@@ -1084,13 +1020,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
UnorderedSymbolSet privateDataSharingAttributeObjects_; // on one directive
UnorderedSymbolSet stmtFunctionExprSymbols_;
- std::multimap<const parser::Label,
- std::pair<parser::CharBlock, std::optional<DirContext>>>
- sourceLabels_;
- std::map<const parser::Label,
- std::pair<parser::CharBlock, std::optional<DirContext>>>
- targetLabels_;
- parser::CharBlock currentStatementSource_;
enum class PartKind : int {
// There are also other "parts", such as internal-subprogram-part, etc,
@@ -1133,13 +1062,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
const parser::Name &, const Symbol &, Symbol::Flag);
void CheckObjectIsPrivatizable(
const parser::Name &, const Symbol &, Symbol::Flag);
- void CheckSourceLabel(const parser::Label &);
- void CheckLabelContext(const parser::CharBlock, const parser::CharBlock,
- std::optional<DirContext>, std::optional<DirContext>);
- void ClearLabels() {
- sourceLabels_.clear();
- targetLabels_.clear();
- };
void AddOmpRequiresToScope(Scope &,
const WithOmpDeclarative::RequiresClauses *,
@@ -3237,60 +3159,6 @@ void OmpAttributeVisitor::CheckObjectIsPrivatizable(
}
}
-void OmpAttributeVisitor::CheckSourceLabel(const parser::Label &label) {
- // Get the context to check if the statement causing a jump to the 'label' is
- // in an enclosing OpenMP construct
- std::optional<DirContext> thisContext{GetContextIf()};
- sourceLabels_.emplace(
- label, std::make_pair(currentStatementSource_, thisContext));
- // Check if the statement with 'label' to which a jump is being introduced
- // has already been encountered
- auto it{targetLabels_.find(label)};
- if (it != targetLabels_.end()) {
- // Check if both the statement with 'label' and the statement that causes a
- // jump to the 'label' are in the same scope
- CheckLabelContext(currentStatementSource_, it->second.first, thisContext,
- it->second.second);
- }
-}
-
-// Check for invalid branch into or out of OpenMP structured blocks
-void OmpAttributeVisitor::CheckLabelContext(const parser::CharBlock source,
- const parser::CharBlock target, std::optional<DirContext> sourceContext,
- std::optional<DirContext> targetContext) {
- auto dirContextsSame = [](DirContext &lhs, DirContext &rhs) -> bool {
- // Sometimes nested constructs share a scope but are different contexts.
- // The directiveSource comparison is for OmpSection. Sections do not have
- // their own scopes and two different sections both have the same directive.
- // Their source however is different. This string comparison is unfortunate
- // but should only happen for GOTOs inside of SECTION.
- return (lhs.scope == rhs.scope) && (lhs.directive == rhs.directive) &&
- (lhs.directiveSource == rhs.directiveSource);
- };
- unsigned version{context_.langOptions().OpenMPVersion};
- if (targetContext &&
- (!sourceContext ||
- (!dirContextsSame(*targetContext, *sourceContext) &&
- !DoesScopeContain(
- &targetContext->scope, sourceContext->scope)))) {
- context_
- .Say(source, "invalid branch into an OpenMP structured block"_err_en_US)
- .Attach(target, "In the enclosing %s directive branched into"_en_US,
- parser::omp::GetUpperName(targetContext->directive, version));
- }
- if (sourceContext &&
- (!targetContext ||
- (!dirContextsSame(*sourceContext, *targetContext) &&
- !DoesScopeContain(
- &sourceContext->scope, targetContext->scope)))) {
- context_
- .Say(source,
- "invalid branch leaving an OpenMP structured block"_err_en_US)
- .Attach(target, "Outside the enclosing %s directive"_en_US,
- parser::omp::GetUpperName(sourceContext->directive, version));
- }
-}
-
void OmpAttributeVisitor::AddOmpRequiresToScope(Scope &scope,
const WithOmpDeclarative::RequiresClauses *reqs,
const common::OmpMemoryOrderType *memOrder) {
diff --git a/flang/test/Semantics/OpenMP/parallel-master-goto.f90 b/flang/test/Semantics/OpenMP/parallel-master-goto.f90
index 01d14aaa46d30..4adc38728fb8f 100644
--- a/flang/test/Semantics/OpenMP/parallel-master-goto.f90
+++ b/flang/test/Semantics/OpenMP/parallel-master-goto.f90
@@ -4,7 +4,6 @@
!$omp parallel
do i = 1, 2
!ERROR: invalid branch into an OpenMP structured block
-!ERROR: invalid branch leaving an OpenMP structured block
goto 10
end do
!$omp master
More information about the flang-commits
mailing list