[flang-commits] [flang] 4b9259b - Revert "[Flang][OpenMP][Sema] Support propagation of REQUIRES information across program units"
Sergio Afonso via flang-commits
flang-commits at lists.llvm.org
Mon Sep 11 05:03:49 PDT 2023
Author: Sergio Afonso
Date: 2023-09-11T13:01:29+01:00
New Revision: 4b9259b9470480cbe140e49858c08fd62f67d7c6
URL: https://github.com/llvm/llvm-project/commit/4b9259b9470480cbe140e49858c08fd62f67d7c6
DIFF: https://github.com/llvm/llvm-project/commit/4b9259b9470480cbe140e49858c08fd62f67d7c6.diff
LOG: Revert "[Flang][OpenMP][Sema] Support propagation of REQUIRES information across program units"
Changes in this commit make some gfortran tests crash the compiler. It is
likely trying to dereference undefined symbol pointers.
This reverts commit 3787fd942f3927345320cc97a479f13e44355805.
Added:
Modified:
flang/examples/FeatureList/FeatureList.cpp
flang/include/flang/Common/Fortran.h
flang/include/flang/Parser/dump-parse-tree.h
flang/include/flang/Parser/parse-tree.h
flang/include/flang/Semantics/symbol.h
flang/lib/Parser/openmp-parsers.cpp
flang/lib/Parser/unparse.cpp
flang/lib/Semantics/resolve-directives.cpp
flang/lib/Semantics/resolve-directives.h
flang/lib/Semantics/resolve-names.cpp
Removed:
flang/test/Semantics/OpenMP/requires09.f90
################################################################################
diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 6f10553cdcb4c0d..7ab294597ee0e0d 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -23,7 +23,6 @@
#include <utility>
#include <vector>
-using namespace Fortran::common;
using namespace Fortran::frontend;
using namespace Fortran::parser;
using namespace Fortran;
@@ -554,7 +553,7 @@ struct NodeVisitor {
READ_FEATURE(OmpAtomicClause)
READ_FEATURE(OmpAtomicClauseList)
READ_FEATURE(OmpAtomicDefaultMemOrderClause)
- READ_FEATURE(OmpAtomicDefaultMemOrderType)
+ READ_FEATURE(OmpAtomicDefaultMemOrderClause::Type)
READ_FEATURE(OpenMPFlushConstruct)
READ_FEATURE(OpenMPLoopConstruct)
READ_FEATURE(OpenMPExecutableAllocate)
diff --git a/flang/include/flang/Common/Fortran.h b/flang/include/flang/Common/Fortran.h
index 15db21bf3473c05..df47e98150ce6e1 100644
--- a/flang/include/flang/Common/Fortran.h
+++ b/flang/include/flang/Common/Fortran.h
@@ -87,9 +87,6 @@ ENUM_CLASS(CUDASubprogramAttrs, Host, Device, HostDevice, Global, Grid_Global)
// CUDA data attributes; mutually exclusive
ENUM_CLASS(CUDADataAttr, Constant, Device, Managed, Pinned, Shared, Texture)
-// OpenMP atomic_default_mem_order clause allowed values
-ENUM_CLASS(OmpAtomicDefaultMemOrderType, SeqCst, AcqRel, Relaxed)
-
// Fortran names may have up to 63 characters (See Fortran 2018 C601).
static constexpr int maxNameLen{63};
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index e7d74dda71a20cf..7a009e8cc708284 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -589,7 +589,7 @@ class ParseTreeDumper {
NODE(parser, OmpAtomicClause)
NODE(parser, OmpAtomicClauseList)
NODE(parser, OmpAtomicDefaultMemOrderClause)
- NODE_ENUM(common, OmpAtomicDefaultMemOrderType)
+ NODE_ENUM(OmpAtomicDefaultMemOrderClause, Type)
NODE(parser, OpenMPFlushConstruct)
NODE(parser, OpenMPLoopConstruct)
NODE(parser, OpenMPExecutableAllocate)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 5d92ecb05841767..d8449c8b812ae2f 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3593,8 +3593,8 @@ struct OmpDependClause {
// ATOMIC_DEFAULT_MEM_ORDER (SEQ_CST | ACQ_REL |
// RELAXED)
struct OmpAtomicDefaultMemOrderClause {
- WRAPPER_CLASS_BOILERPLATE(
- OmpAtomicDefaultMemOrderClause, common::OmpAtomicDefaultMemOrderType);
+ ENUM_CLASS(Type, SeqCst, AcqRel, Relaxed)
+ WRAPPER_CLASS_BOILERPLATE(OmpAtomicDefaultMemOrderClause, Type);
};
// OpenMP Clauses
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index aada3bf94cc1213..7280a4eaa5fca57 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -45,38 +45,8 @@ using SymbolVector = std::vector<SymbolRef>;
using MutableSymbolRef = common::Reference<Symbol>;
using MutableSymbolVector = std::vector<MutableSymbolRef>;
-// Mixin for details with OpenMP declarative constructs.
-class WithOmpDeclarative {
- using OmpAtomicOrderType = common::OmpAtomicDefaultMemOrderType;
-
-public:
- ENUM_CLASS(RequiresFlag, ReverseOffload, UnifiedAddress, UnifiedSharedMemory,
- DynamicAllocators);
- using RequiresFlags = common::EnumSet<RequiresFlag, RequiresFlag_enumSize>;
-
- bool has_ompRequires() const { return ompRequires_.has_value(); }
- const RequiresFlags *ompRequires() const {
- return ompRequires_ ? &*ompRequires_ : nullptr;
- }
- void set_ompRequires(RequiresFlags flags) { ompRequires_ = flags; }
-
- bool has_ompAtomicDefaultMemOrder() const {
- return ompAtomicDefaultMemOrder_.has_value();
- }
- const OmpAtomicOrderType *ompAtomicDefaultMemOrder() const {
- return ompAtomicDefaultMemOrder_ ? &*ompAtomicDefaultMemOrder_ : nullptr;
- }
- void set_ompAtomicDefaultMemOrder(OmpAtomicOrderType flags) {
- ompAtomicDefaultMemOrder_ = flags;
- }
-
-private:
- std::optional<RequiresFlags> ompRequires_;
- std::optional<OmpAtomicOrderType> ompAtomicDefaultMemOrder_;
-};
-
// A module or submodule.
-class ModuleDetails : public WithOmpDeclarative {
+class ModuleDetails {
public:
ModuleDetails(bool isSubmodule = false) : isSubmodule_{isSubmodule} {}
bool isSubmodule() const { return isSubmodule_; }
@@ -93,7 +63,7 @@ class ModuleDetails : public WithOmpDeclarative {
const Scope *scope_{nullptr};
};
-class MainProgramDetails : public WithOmpDeclarative {
+class MainProgramDetails {
public:
private:
};
@@ -144,7 +114,7 @@ class OpenACCRoutineInfo {
// A subroutine or function definition, or a subprogram interface defined
// in an INTERFACE block as part of the definition of a dummy procedure
// or a procedure pointer (with just POINTER).
-class SubprogramDetails : public WithBindName, public WithOmpDeclarative {
+class SubprogramDetails : public WithBindName {
public:
bool isFunction() const { return result_ != nullptr; }
bool isInterface() const { return isInterface_; }
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index b220d578b3bbc7e..b30a3a1eb2a151f 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -432,9 +432,9 @@ TYPE_PARSER(sourced(construct<OmpMemoryOrderClause>(
// acq_rel
// relaxed
TYPE_PARSER(construct<OmpAtomicDefaultMemOrderClause>(
- "SEQ_CST" >> pure(common::OmpAtomicDefaultMemOrderType::SeqCst) ||
- "ACQ_REL" >> pure(common::OmpAtomicDefaultMemOrderType::AcqRel) ||
- "RELAXED" >> pure(common::OmpAtomicDefaultMemOrderType::Relaxed)))
+ "SEQ_CST" >> pure(OmpAtomicDefaultMemOrderClause::Type::SeqCst) ||
+ "ACQ_REL" >> pure(OmpAtomicDefaultMemOrderClause::Type::AcqRel) ||
+ "RELAXED" >> pure(OmpAtomicDefaultMemOrderClause::Type::Relaxed)))
// 2.17.7 Atomic construct
// atomic-clause -> memory-order-clause | HINT(hint-expression)
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 398545d315e5cd5..d7626c0ea762937 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2307,7 +2307,17 @@ class UnparseVisitor {
}
void Unparse(const OmpAtomicDefaultMemOrderClause &x) {
- Word(ToUpperCaseLetters(common::EnumToString(x.v)));
+ switch (x.v) {
+ case OmpAtomicDefaultMemOrderClause::Type::SeqCst:
+ Word("SEQ_CST");
+ break;
+ case OmpAtomicDefaultMemOrderClause::Type::AcqRel:
+ Word("ACQ_REL");
+ break;
+ case OmpAtomicDefaultMemOrderClause::Type::Relaxed:
+ Word("RELAXED");
+ break;
+ }
}
void Unparse(const OmpAtomicClauseList &x) { Walk(" ", x.v, " "); }
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 8375d095624ad05..38e195d20a34367 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -22,13 +22,6 @@
#include <map>
#include <sstream>
-template <typename T>
-static Fortran::semantics::Scope *GetScope(
- Fortran::semantics::SemanticsContext &context, const T &x) {
- std::optional<Fortran::parser::CharBlock> source{GetSource(x)};
- return source ? &context.FindScope(*source) : nullptr;
-}
-
namespace Fortran::semantics {
template <typename T> class DirectiveAttributeVisitor {
@@ -331,6 +324,11 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
return true;
}
+ bool Pre(const parser::SpecificationPart &x) {
+ Walk(std::get<std::list<parser::OpenMPDeclarativeConstruct>>(x.t));
+ 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)}) {
@@ -377,38 +375,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
void Post(const parser::OpenMPDeclareSimdConstruct &) { PopContext(); }
bool Pre(const parser::OpenMPRequiresConstruct &x) {
- using Flags = WithOmpDeclarative::RequiresFlags;
- using Requires = WithOmpDeclarative::RequiresFlag;
PushContext(x.source, llvm::omp::Directive::OMPD_requires);
-
- // Gather information from the clauses.
- Flags flags;
- std::optional<common::OmpAtomicDefaultMemOrderType> memOrder;
- for (const auto &clause : std::get<parser::OmpClauseList>(x.t).v) {
- flags |= common::visit(
- common::visitors{
- [&memOrder](
- const parser::OmpClause::AtomicDefaultMemOrder &atomic) {
- memOrder = atomic.v.v;
- return Flags{};
- },
- [](const parser::OmpClause::ReverseOffload &) {
- return Flags{Requires::ReverseOffload};
- },
- [](const parser::OmpClause::UnifiedAddress &) {
- return Flags{Requires::UnifiedAddress};
- },
- [](const parser::OmpClause::UnifiedSharedMemory &) {
- return Flags{Requires::UnifiedSharedMemory};
- },
- [](const parser::OmpClause::DynamicAllocators &) {
- return Flags{Requires::DynamicAllocators};
- },
- [](const auto &) { return Flags{}; }},
- clause.u);
- }
- // Merge clauses into parents' symbols details.
- AddOmpRequiresToScope(currScope(), flags, memOrder);
return true;
}
void Post(const parser::OpenMPRequiresConstruct &) { PopContext(); }
@@ -705,9 +672,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
bool HasSymbolInEnclosingScope(const Symbol &, Scope &);
std::int64_t ordCollapseLevel{0};
-
- void AddOmpRequiresToScope(Scope &, WithOmpDeclarative::RequiresFlags,
- std::optional<common::OmpAtomicDefaultMemOrderType>);
};
template <typename T>
@@ -2211,77 +2175,6 @@ void ResolveOmpParts(
}
}
-void ResolveOmpTopLevelParts(
- SemanticsContext &context, const parser::Program &program) {
- if (!context.IsEnabled(common::LanguageFeature::OpenMP)) {
- return;
- }
-
- // Gather REQUIRES clauses from all non-module top-level program unit symbols,
- // combine them together ensuring compatibility and apply them to all these
- // program units. Modules are skipped because their REQUIRES clauses should be
- // propagated via USE statements instead.
- WithOmpDeclarative::RequiresFlags combinedFlags;
- std::optional<common::OmpAtomicDefaultMemOrderType> combinedMemOrder;
-
- // Function to go through non-module top level program units and extract
- // REQUIRES information to be processed by a function-like argument.
- auto processProgramUnits{[&](auto processFn) {
- for (const parser::ProgramUnit &unit : program.v) {
- if (!std::holds_alternative<common::Indirection<parser::Module>>(
- unit.u) &&
- !std::holds_alternative<common::Indirection<parser::Submodule>>(
- unit.u)) {
- Symbol *symbol{common::visit(
- [&context](
- auto &x) { return GetScope(context, x.value())->symbol(); },
- unit.u)};
-
- common::visit(
- [&](auto &details) {
- if constexpr (std::is_convertible_v<decltype(&details),
- WithOmpDeclarative *>) {
- processFn(*symbol, details);
- }
- },
- symbol->details());
- }
- }
- }};
-
- // Combine global REQUIRES information from all program units except modules
- // and submodules.
- processProgramUnits([&](Symbol &symbol, WithOmpDeclarative &details) {
- if (const WithOmpDeclarative::RequiresFlags *
- flags{details.ompRequires()}) {
- combinedFlags |= *flags;
- }
- if (const common::OmpAtomicDefaultMemOrderType *
- memOrder{details.ompAtomicDefaultMemOrder()}) {
- if (combinedMemOrder && *combinedMemOrder != *memOrder) {
- context.Say(symbol.scope()->sourceRange(),
- "Conflicting '%s' REQUIRES clauses found in compilation "
- "unit"_err_en_US,
- parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(
- llvm::omp::Clause::OMPC_atomic_default_mem_order)
- .str()));
- }
- combinedMemOrder = *memOrder;
- }
- });
-
- // Update all program units except modules and submodules with the combined
- // global REQUIRES information.
- processProgramUnits([&](Symbol &, WithOmpDeclarative &details) {
- if (combinedFlags.any()) {
- details.set_ompRequires(combinedFlags);
- }
- if (combinedMemOrder) {
- details.set_ompAtomicDefaultMemOrder(*combinedMemOrder);
- }
- });
-}
-
void OmpAttributeVisitor::CheckDataCopyingClause(
const parser::Name &name, const Symbol &symbol, Symbol::Flag ompFlag) {
const auto *checkSymbol{&symbol};
@@ -2429,44 +2322,4 @@ void OmpAttributeVisitor::CheckNameInAllocateStmt(
parser::ToUpperCaseLetters(
llvm::omp::getOpenMPDirectiveName(GetContext().directive).str()));
}
-
-void OmpAttributeVisitor::AddOmpRequiresToScope(Scope &scope,
- WithOmpDeclarative::RequiresFlags flags,
- std::optional<common::OmpAtomicDefaultMemOrderType> memOrder) {
- Scope *scopeIter = &scope;
- do {
- if (Symbol * symbol{scopeIter->symbol()}) {
- common::visit(
- [&](auto &details) {
- // Store clauses information into the symbol for the parent and
- // enclosing modules, programs, functions and subroutines.
- if constexpr (std::is_convertible_v<decltype(&details),
- WithOmpDeclarative *>) {
- if (flags.any()) {
- if (const WithOmpDeclarative::RequiresFlags *
- otherFlags{details.ompRequires()}) {
- flags |= *otherFlags;
- }
- details.set_ompRequires(flags);
- }
- if (memOrder) {
- if (details.has_ompAtomicDefaultMemOrder() &&
- *details.ompAtomicDefaultMemOrder() != *memOrder) {
- context_.Say(scopeIter->sourceRange(),
- "Conflicting '%s' REQUIRES clauses found in compilation "
- "unit"_err_en_US,
- parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(
- llvm::omp::Clause::OMPC_atomic_default_mem_order)
- .str()));
- }
- details.set_ompAtomicDefaultMemOrder(*memOrder);
- }
- }
- },
- symbol->details());
- }
- scopeIter = &scopeIter->parent();
- } while (!scopeIter->IsGlobal());
-}
-
} // namespace Fortran::semantics
diff --git a/flang/lib/Semantics/resolve-directives.h b/flang/lib/Semantics/resolve-directives.h
index 839165aaf30eb81..6ba7a062529421a 100644
--- a/flang/lib/Semantics/resolve-directives.h
+++ b/flang/lib/Semantics/resolve-directives.h
@@ -11,7 +11,6 @@
namespace Fortran::parser {
struct Name;
-struct Program;
struct ProgramUnit;
} // namespace Fortran::parser
@@ -22,7 +21,6 @@ class SemanticsContext;
// Name resolution for OpenACC and OpenMP directives
void ResolveAccParts(SemanticsContext &, const parser::ProgramUnit &);
void ResolveOmpParts(SemanticsContext &, const parser::ProgramUnit &);
-void ResolveOmpTopLevelParts(SemanticsContext &, const parser::Program &);
} // namespace Fortran::semantics
#endif
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 865c198424696a9..0b4b940fa1d1c70 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -8699,14 +8699,11 @@ void ResolveNamesVisitor::ResolveExecutionParts(const ProgramTree &node) {
}
}
-void ResolveNamesVisitor::Post(const parser::Program &x) {
+void ResolveNamesVisitor::Post(const parser::Program &) {
// ensure that all temps were deallocated
CHECK(!attrs_);
CHECK(!cudaDataAttr_);
CHECK(!GetDeclTypeSpec());
- // Top-level resolution to propagate information across program units after
- // each of them has been resolved separately.
- ResolveOmpTopLevelParts(context(), x);
}
// A singleton instance of the scope -> IMPLICIT rules mapping is
diff --git a/flang/test/Semantics/OpenMP/requires09.f90 b/flang/test/Semantics/OpenMP/requires09.f90
deleted file mode 100644
index 2fa5d950b9c2d8a..000000000000000
--- a/flang/test/Semantics/OpenMP/requires09.f90
+++ /dev/null
@@ -1,14 +0,0 @@
-! RUN: %python %S/../test_errors.py %s %flang -fopenmp
-! OpenMP Version 5.0
-! 2.4 Requires directive
-! All atomic_default_mem_order clauses in 'requires' directives found within a
-! compilation unit must specify the same ordering.
-
-subroutine f
- !$omp requires atomic_default_mem_order(seq_cst)
-end subroutine f
-
-!ERROR: Conflicting 'ATOMIC_DEFAULT_MEM_ORDER' REQUIRES clauses found in compilation unit
-subroutine g
- !$omp requires atomic_default_mem_order(relaxed)
-end subroutine g
More information about the flang-commits
mailing list