Skip to content

Commit 4b9259b

Browse files
skatrakskatrak
committed
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 3787fd9.
1 parent dbeb3d0 commit 4b9259b

File tree

11 files changed

+27
-217
lines changed

11 files changed

+27
-217
lines changed

‎flang/examples/FeatureList/FeatureList.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include <utility>
2424
#include <vector>
2525

26-
using namespace Fortran::common;
2726
using namespace Fortran::frontend;
2827
using namespace Fortran::parser;
2928
using namespace Fortran;
@@ -554,7 +553,7 @@ struct NodeVisitor {
554553
READ_FEATURE(OmpAtomicClause)
555554
READ_FEATURE(OmpAtomicClauseList)
556555
READ_FEATURE(OmpAtomicDefaultMemOrderClause)
557-
READ_FEATURE(OmpAtomicDefaultMemOrderType)
556+
READ_FEATURE(OmpAtomicDefaultMemOrderClause::Type)
558557
READ_FEATURE(OpenMPFlushConstruct)
559558
READ_FEATURE(OpenMPLoopConstruct)
560559
READ_FEATURE(OpenMPExecutableAllocate)

‎flang/include/flang/Common/Fortran.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,6 @@ ENUM_CLASS(CUDASubprogramAttrs, Host, Device, HostDevice, Global, Grid_Global)
8787
// CUDA data attributes; mutually exclusive
8888
ENUM_CLASS(CUDADataAttr, Constant, Device, Managed, Pinned, Shared, Texture)
8989

90-
// OpenMP atomic_default_mem_order clause allowed values
91-
ENUM_CLASS(OmpAtomicDefaultMemOrderType, SeqCst, AcqRel, Relaxed)
92-
9390
// Fortran names may have up to 63 characters (See Fortran 2018 C601).
9491
static constexpr int maxNameLen{63};
9592

‎flang/include/flang/Parser/dump-parse-tree.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ class ParseTreeDumper {
589589
NODE(parser, OmpAtomicClause)
590590
NODE(parser, OmpAtomicClauseList)
591591
NODE(parser, OmpAtomicDefaultMemOrderClause)
592-
NODE_ENUM(common, OmpAtomicDefaultMemOrderType)
592+
NODE_ENUM(OmpAtomicDefaultMemOrderClause, Type)
593593
NODE(parser, OpenMPFlushConstruct)
594594
NODE(parser, OpenMPLoopConstruct)
595595
NODE(parser, OpenMPExecutableAllocate)

‎flang/include/flang/Parser/parse-tree.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3593,8 +3593,8 @@ struct OmpDependClause {
35933593
// ATOMIC_DEFAULT_MEM_ORDER (SEQ_CST | ACQ_REL |
35943594
// RELAXED)
35953595
struct OmpAtomicDefaultMemOrderClause {
3596-
WRAPPER_CLASS_BOILERPLATE(
3597-
OmpAtomicDefaultMemOrderClause, common::OmpAtomicDefaultMemOrderType);
3596+
ENUM_CLASS(Type, SeqCst, AcqRel, Relaxed)
3597+
WRAPPER_CLASS_BOILERPLATE(OmpAtomicDefaultMemOrderClause, Type);
35983598
};
35993599

36003600
// OpenMP Clauses

‎flang/include/flang/Semantics/symbol.h

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -45,38 +45,8 @@ using SymbolVector = std::vector<SymbolRef>;
4545
using MutableSymbolRef = common::Reference<Symbol>;
4646
using MutableSymbolVector = std::vector<MutableSymbolRef>;
4747

48-
// Mixin for details with OpenMP declarative constructs.
49-
class WithOmpDeclarative {
50-
using OmpAtomicOrderType = common::OmpAtomicDefaultMemOrderType;
51-
52-
public:
53-
ENUM_CLASS(RequiresFlag, ReverseOffload, UnifiedAddress, UnifiedSharedMemory,
54-
DynamicAllocators);
55-
using RequiresFlags = common::EnumSet<RequiresFlag, RequiresFlag_enumSize>;
56-
57-
bool has_ompRequires() const { return ompRequires_.has_value(); }
58-
const RequiresFlags *ompRequires() const {
59-
return ompRequires_ ? &*ompRequires_ : nullptr;
60-
}
61-
void set_ompRequires(RequiresFlags flags) { ompRequires_ = flags; }
62-
63-
bool has_ompAtomicDefaultMemOrder() const {
64-
return ompAtomicDefaultMemOrder_.has_value();
65-
}
66-
const OmpAtomicOrderType *ompAtomicDefaultMemOrder() const {
67-
return ompAtomicDefaultMemOrder_ ? &*ompAtomicDefaultMemOrder_ : nullptr;
68-
}
69-
void set_ompAtomicDefaultMemOrder(OmpAtomicOrderType flags) {
70-
ompAtomicDefaultMemOrder_ = flags;
71-
}
72-
73-
private:
74-
std::optional<RequiresFlags> ompRequires_;
75-
std::optional<OmpAtomicOrderType> ompAtomicDefaultMemOrder_;
76-
};
77-
7848
// A module or submodule.
79-
class ModuleDetails : public WithOmpDeclarative {
49+
class ModuleDetails {
8050
public:
8151
ModuleDetails(bool isSubmodule = false) : isSubmodule_{isSubmodule} {}
8252
bool isSubmodule() const { return isSubmodule_; }
@@ -93,7 +63,7 @@ class ModuleDetails : public WithOmpDeclarative {
9363
const Scope *scope_{nullptr};
9464
};
9565

96-
class MainProgramDetails : public WithOmpDeclarative {
66+
class MainProgramDetails {
9767
public:
9868
private:
9969
};
@@ -144,7 +114,7 @@ class OpenACCRoutineInfo {
144114
// A subroutine or function definition, or a subprogram interface defined
145115
// in an INTERFACE block as part of the definition of a dummy procedure
146116
// or a procedure pointer (with just POINTER).
147-
class SubprogramDetails : public WithBindName, public WithOmpDeclarative {
117+
class SubprogramDetails : public WithBindName {
148118
public:
149119
bool isFunction() const { return result_ != nullptr; }
150120
bool isInterface() const { return isInterface_; }

‎flang/lib/Parser/openmp-parsers.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -432,9 +432,9 @@ TYPE_PARSER(sourced(construct<OmpMemoryOrderClause>(
432432
// acq_rel
433433
// relaxed
434434
TYPE_PARSER(construct<OmpAtomicDefaultMemOrderClause>(
435-
"SEQ_CST" >> pure(common::OmpAtomicDefaultMemOrderType::SeqCst) ||
436-
"ACQ_REL" >> pure(common::OmpAtomicDefaultMemOrderType::AcqRel) ||
437-
"RELAXED" >> pure(common::OmpAtomicDefaultMemOrderType::Relaxed)))
435+
"SEQ_CST" >> pure(OmpAtomicDefaultMemOrderClause::Type::SeqCst) ||
436+
"ACQ_REL" >> pure(OmpAtomicDefaultMemOrderClause::Type::AcqRel) ||
437+
"RELAXED" >> pure(OmpAtomicDefaultMemOrderClause::Type::Relaxed)))
438438

439439
// 2.17.7 Atomic construct
440440
// atomic-clause -> memory-order-clause | HINT(hint-expression)

‎flang/lib/Parser/unparse.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2307,7 +2307,17 @@ class UnparseVisitor {
23072307
}
23082308

23092309
void Unparse(const OmpAtomicDefaultMemOrderClause &x) {
2310-
Word(ToUpperCaseLetters(common::EnumToString(x.v)));
2310+
switch (x.v) {
2311+
case OmpAtomicDefaultMemOrderClause::Type::SeqCst:
2312+
Word("SEQ_CST");
2313+
break;
2314+
case OmpAtomicDefaultMemOrderClause::Type::AcqRel:
2315+
Word("ACQ_REL");
2316+
break;
2317+
case OmpAtomicDefaultMemOrderClause::Type::Relaxed:
2318+
Word("RELAXED");
2319+
break;
2320+
}
23112321
}
23122322

23132323
void Unparse(const OmpAtomicClauseList &x) { Walk(" ", x.v, " "); }

‎flang/lib/Semantics/resolve-directives.cpp

Lines changed: 5 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,6 @@
2222
#include <map>
2323
#include <sstream>
2424

25-
template <typename T>
26-
static Fortran::semantics::Scope *GetScope(
27-
Fortran::semantics::SemanticsContext &context, const T &x) {
28-
std::optional<Fortran::parser::CharBlock> source{GetSource(x)};
29-
return source ? &context.FindScope(*source) : nullptr;
30-
}
31-
3225
namespace Fortran::semantics {
3326

3427
template <typename T> class DirectiveAttributeVisitor {
@@ -331,6 +324,11 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
331324
return true;
332325
}
333326

327+
bool Pre(const parser::SpecificationPart &x) {
328+
Walk(std::get<std::list<parser::OpenMPDeclarativeConstruct>>(x.t));
329+
return true;
330+
}
331+
334332
bool Pre(const parser::StmtFunctionStmt &x) {
335333
const auto &parsedExpr{std::get<parser::Scalar<parser::Expr>>(x.t)};
336334
if (const auto *expr{GetExpr(context_, parsedExpr)}) {
@@ -377,38 +375,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
377375
void Post(const parser::OpenMPDeclareSimdConstruct &) { PopContext(); }
378376

379377
bool Pre(const parser::OpenMPRequiresConstruct &x) {
380-
using Flags = WithOmpDeclarative::RequiresFlags;
381-
using Requires = WithOmpDeclarative::RequiresFlag;
382378
PushContext(x.source, llvm::omp::Directive::OMPD_requires);
383-
384-
// Gather information from the clauses.
385-
Flags flags;
386-
std::optional<common::OmpAtomicDefaultMemOrderType> memOrder;
387-
for (const auto &clause : std::get<parser::OmpClauseList>(x.t).v) {
388-
flags |= common::visit(
389-
common::visitors{
390-
[&memOrder](
391-
const parser::OmpClause::AtomicDefaultMemOrder &atomic) {
392-
memOrder = atomic.v.v;
393-
return Flags{};
394-
},
395-
[](const parser::OmpClause::ReverseOffload &) {
396-
return Flags{Requires::ReverseOffload};
397-
},
398-
[](const parser::OmpClause::UnifiedAddress &) {
399-
return Flags{Requires::UnifiedAddress};
400-
},
401-
[](const parser::OmpClause::UnifiedSharedMemory &) {
402-
return Flags{Requires::UnifiedSharedMemory};
403-
},
404-
[](const parser::OmpClause::DynamicAllocators &) {
405-
return Flags{Requires::DynamicAllocators};
406-
},
407-
[](const auto &) { return Flags{}; }},
408-
clause.u);
409-
}
410-
// Merge clauses into parents' symbols details.
411-
AddOmpRequiresToScope(currScope(), flags, memOrder);
412379
return true;
413380
}
414381
void Post(const parser::OpenMPRequiresConstruct &) { PopContext(); }
@@ -705,9 +672,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
705672

706673
bool HasSymbolInEnclosingScope(const Symbol &, Scope &);
707674
std::int64_t ordCollapseLevel{0};
708-
709-
void AddOmpRequiresToScope(Scope &, WithOmpDeclarative::RequiresFlags,
710-
std::optional<common::OmpAtomicDefaultMemOrderType>);
711675
};
712676

713677
template <typename T>
@@ -2211,77 +2175,6 @@ void ResolveOmpParts(
22112175
}
22122176
}
22132177

2214-
void ResolveOmpTopLevelParts(
2215-
SemanticsContext &context, const parser::Program &program) {
2216-
if (!context.IsEnabled(common::LanguageFeature::OpenMP)) {
2217-
return;
2218-
}
2219-
2220-
// Gather REQUIRES clauses from all non-module top-level program unit symbols,
2221-
// combine them together ensuring compatibility and apply them to all these
2222-
// program units. Modules are skipped because their REQUIRES clauses should be
2223-
// propagated via USE statements instead.
2224-
WithOmpDeclarative::RequiresFlags combinedFlags;
2225-
std::optional<common::OmpAtomicDefaultMemOrderType> combinedMemOrder;
2226-
2227-
// Function to go through non-module top level program units and extract
2228-
// REQUIRES information to be processed by a function-like argument.
2229-
auto processProgramUnits{[&](auto processFn) {
2230-
for (const parser::ProgramUnit &unit : program.v) {
2231-
if (!std::holds_alternative<common::Indirection<parser::Module>>(
2232-
unit.u) &&
2233-
!std::holds_alternative<common::Indirection<parser::Submodule>>(
2234-
unit.u)) {
2235-
Symbol *symbol{common::visit(
2236-
[&context](
2237-
auto &x) { return GetScope(context, x.value())->symbol(); },
2238-
unit.u)};
2239-
2240-
common::visit(
2241-
[&](auto &details) {
2242-
if constexpr (std::is_convertible_v<decltype(&details),
2243-
WithOmpDeclarative *>) {
2244-
processFn(*symbol, details);
2245-
}
2246-
},
2247-
symbol->details());
2248-
}
2249-
}
2250-
}};
2251-
2252-
// Combine global REQUIRES information from all program units except modules
2253-
// and submodules.
2254-
processProgramUnits([&](Symbol &symbol, WithOmpDeclarative &details) {
2255-
if (const WithOmpDeclarative::RequiresFlags *
2256-
flags{details.ompRequires()}) {
2257-
combinedFlags |= *flags;
2258-
}
2259-
if (const common::OmpAtomicDefaultMemOrderType *
2260-
memOrder{details.ompAtomicDefaultMemOrder()}) {
2261-
if (combinedMemOrder && *combinedMemOrder != *memOrder) {
2262-
context.Say(symbol.scope()->sourceRange(),
2263-
"Conflicting '%s' REQUIRES clauses found in compilation "
2264-
"unit"_err_en_US,
2265-
parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(
2266-
llvm::omp::Clause::OMPC_atomic_default_mem_order)
2267-
.str()));
2268-
}
2269-
combinedMemOrder = *memOrder;
2270-
}
2271-
});
2272-
2273-
// Update all program units except modules and submodules with the combined
2274-
// global REQUIRES information.
2275-
processProgramUnits([&](Symbol &, WithOmpDeclarative &details) {
2276-
if (combinedFlags.any()) {
2277-
details.set_ompRequires(combinedFlags);
2278-
}
2279-
if (combinedMemOrder) {
2280-
details.set_ompAtomicDefaultMemOrder(*combinedMemOrder);
2281-
}
2282-
});
2283-
}
2284-
22852178
void OmpAttributeVisitor::CheckDataCopyingClause(
22862179
const parser::Name &name, const Symbol &symbol, Symbol::Flag ompFlag) {
22872180
const auto *checkSymbol{&symbol};
@@ -2429,44 +2322,4 @@ void OmpAttributeVisitor::CheckNameInAllocateStmt(
24292322
parser::ToUpperCaseLetters(
24302323
llvm::omp::getOpenMPDirectiveName(GetContext().directive).str()));
24312324
}
2432-
2433-
void OmpAttributeVisitor::AddOmpRequiresToScope(Scope &scope,
2434-
WithOmpDeclarative::RequiresFlags flags,
2435-
std::optional<common::OmpAtomicDefaultMemOrderType> memOrder) {
2436-
Scope *scopeIter = &scope;
2437-
do {
2438-
if (Symbol * symbol{scopeIter->symbol()}) {
2439-
common::visit(
2440-
[&](auto &details) {
2441-
// Store clauses information into the symbol for the parent and
2442-
// enclosing modules, programs, functions and subroutines.
2443-
if constexpr (std::is_convertible_v<decltype(&details),
2444-
WithOmpDeclarative *>) {
2445-
if (flags.any()) {
2446-
if (const WithOmpDeclarative::RequiresFlags *
2447-
otherFlags{details.ompRequires()}) {
2448-
flags |= *otherFlags;
2449-
}
2450-
details.set_ompRequires(flags);
2451-
}
2452-
if (memOrder) {
2453-
if (details.has_ompAtomicDefaultMemOrder() &&
2454-
*details.ompAtomicDefaultMemOrder() != *memOrder) {
2455-
context_.Say(scopeIter->sourceRange(),
2456-
"Conflicting '%s' REQUIRES clauses found in compilation "
2457-
"unit"_err_en_US,
2458-
parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(
2459-
llvm::omp::Clause::OMPC_atomic_default_mem_order)
2460-
.str()));
2461-
}
2462-
details.set_ompAtomicDefaultMemOrder(*memOrder);
2463-
}
2464-
}
2465-
},
2466-
symbol->details());
2467-
}
2468-
scopeIter = &scopeIter->parent();
2469-
} while (!scopeIter->IsGlobal());
2470-
}
2471-
24722325
} // namespace Fortran::semantics

‎flang/lib/Semantics/resolve-directives.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
namespace Fortran::parser {
1313
struct Name;
14-
struct Program;
1514
struct ProgramUnit;
1615
} // namespace Fortran::parser
1716

@@ -22,7 +21,6 @@ class SemanticsContext;
2221
// Name resolution for OpenACC and OpenMP directives
2322
void ResolveAccParts(SemanticsContext &, const parser::ProgramUnit &);
2423
void ResolveOmpParts(SemanticsContext &, const parser::ProgramUnit &);
25-
void ResolveOmpTopLevelParts(SemanticsContext &, const parser::Program &);
2624

2725
} // namespace Fortran::semantics
2826
#endif

‎flang/lib/Semantics/resolve-names.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8699,14 +8699,11 @@ void ResolveNamesVisitor::ResolveExecutionParts(const ProgramTree &node) {
86998699
}
87008700
}
87018701

8702-
void ResolveNamesVisitor::Post(const parser::Program &x) {
8702+
void ResolveNamesVisitor::Post(const parser::Program &) {
87038703
// ensure that all temps were deallocated
87048704
CHECK(!attrs_);
87058705
CHECK(!cudaDataAttr_);
87068706
CHECK(!GetDeclTypeSpec());
8707-
// Top-level resolution to propagate information across program units after
8708-
// each of them has been resolved separately.
8709-
ResolveOmpTopLevelParts(context(), x);
87108707
}
87118708

87128709
// A singleton instance of the scope -> IMPLICIT rules mapping is

‎flang/test/Semantics/OpenMP/requires09.f90

Lines changed: 0 additions & 14 deletions
This file was deleted.

0 commit comments

Comments
 (0)