[flang] Add parsing of DO CONCURRENT REDUCE clause #92518
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-semantics Author: None (khaki3) ChangesDerived from #92480. This PR supports parsing of the DO CONCURRENT REDUCE clause in Fortran 2023. Following the style of the OpenMP parser in MLIR, the front end accepts both arbitrary operations and procedures for the REDUCE clause. But later Semantics can notify type errors and resolve procedure names. Patch is 29.49 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/92518.diff 11 Files Affected:
diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 3ca92da4f6467..28689b5d3c4b0 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -410,10 +410,12 @@ struct NodeVisitor {
READ_FEATURE(LetterSpec)
READ_FEATURE(LiteralConstant)
READ_FEATURE(IntLiteralConstant)
+ READ_FEATURE(ReduceOperation)
READ_FEATURE(LocalitySpec)
READ_FEATURE(LocalitySpec::DefaultNone)
READ_FEATURE(LocalitySpec::Local)
READ_FEATURE(LocalitySpec::LocalInit)
+ READ_FEATURE(LocalitySpec::Reduce)
READ_FEATURE(LocalitySpec::Shared)
READ_FEATURE(LockStmt)
READ_FEATURE(LockStmt::LockStat)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 68ae50c312cde..15948bb073664 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -436,10 +436,12 @@ class ParseTreeDumper {
NODE(parser, LetterSpec)
NODE(parser, LiteralConstant)
NODE(parser, IntLiteralConstant)
+ NODE(parser, ReduceOperation)
NODE(parser, LocalitySpec)
NODE(LocalitySpec, DefaultNone)
NODE(LocalitySpec, Local)
NODE(LocalitySpec, LocalInit)
+ NODE(LocalitySpec, Reduce)
NODE(LocalitySpec, Shared)
NODE(parser, LockStmt)
NODE(LockStmt, LockStat)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 0a40aa8b8f616..68a4319a85047 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -1870,6 +1870,13 @@ struct ProcComponentRef {
WRAPPER_CLASS_BOILERPLATE(ProcComponentRef, Scalar<StructureComponent>);
};
+// R1522 procedure-designator ->
+// procedure-name | proc-component-ref | data-ref % binding-name
+struct ProcedureDesignator {
+ UNION_CLASS_BOILERPLATE(ProcedureDesignator);
+ std::variant<Name, ProcComponentRef> u;
+};
+
// R914 coindexed-named-object -> data-ref
struct CoindexedNamedObject {
BOILERPLATE(CoindexedNamedObject);
@@ -2236,16 +2243,29 @@ struct ConcurrentHeader {
t;
};
+// F'2023 R1131 reduce-operation ->
+// + | * | .AND. | .OR. | .EQV. | .NEQV. |
+// MAX | MIN | IAND | IOR | IEOR
+struct ReduceOperation {
+ UNION_CLASS_BOILERPLATE(ReduceOperation);
+ std::variant<DefinedOperator, ProcedureDesignator> u;
+};
+
// R1130 locality-spec ->
// LOCAL ( variable-name-list ) | LOCAL_INIT ( variable-name-list ) |
+// REDUCE ( reduce-operation : variable-name-list ) |
// SHARED ( variable-name-list ) | DEFAULT ( NONE )
struct LocalitySpec {
UNION_CLASS_BOILERPLATE(LocalitySpec);
WRAPPER_CLASS(Local, std::list<Name>);
WRAPPER_CLASS(LocalInit, std::list<Name>);
+ struct Reduce {
+ TUPLE_CLASS_BOILERPLATE(Reduce);
+ std::tuple<ReduceOperation, std::list<Name>> t;
+ };
WRAPPER_CLASS(Shared, std::list<Name>);
EMPTY_CLASS(DefaultNone);
- std::variant<Local, LocalInit, Shared, DefaultNone> u;
+ std::variant<Local, LocalInit, Reduce, Shared, DefaultNone> u;
};
// R1123 loop-control ->
@@ -3180,13 +3200,6 @@ WRAPPER_CLASS(ExternalStmt, std::list<Name>);
// R1519 intrinsic-stmt -> INTRINSIC [::] intrinsic-procedure-name-list
WRAPPER_CLASS(IntrinsicStmt, std::list<Name>);
-// R1522 procedure-designator ->
-// procedure-name | proc-component-ref | data-ref % binding-name
-struct ProcedureDesignator {
- UNION_CLASS_BOILERPLATE(ProcedureDesignator);
- std::variant<Name, ProcComponentRef> u;
-};
-
// R1525 alt-return-spec -> * label
WRAPPER_CLASS(AltReturnSpec, Label);
diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h
index 50f7b68d80cb1..8ccf93c803845 100644
--- a/flang/include/flang/Semantics/symbol.h
+++ b/flang/include/flang/Semantics/symbol.h
@@ -714,6 +714,7 @@ class Symbol {
CrayPointer, CrayPointee,
LocalityLocal, // named in LOCAL locality-spec
LocalityLocalInit, // named in LOCAL_INIT locality-spec
+ LocalityReduce, // named in REDUCE locality-spec
LocalityShared, // named in SHARED locality-spec
InDataStmt, // initialized in a DATA statement, =>object, or /init/
InNamelist, // in a Namelist group
diff --git a/flang/lib/Parser/executable-parsers.cpp b/flang/lib/Parser/executable-parsers.cpp
index 382a593416872..6bacdb34f8c70 100644
--- a/flang/lib/Parser/executable-parsers.cpp
+++ b/flang/lib/Parser/executable-parsers.cpp
@@ -252,13 +252,23 @@ TYPE_PARSER(parenthesized(construct<ConcurrentHeader>(
TYPE_PARSER(construct<ConcurrentControl>(name / "=", scalarIntExpr / ":",
scalarIntExpr, maybe(":" >> scalarIntExpr)))
+// F'2023 R1131 reduce-operation ->
+// + | * | .AND. | .OR. | .EQV. | .NEQV. |
+// MAX | MIN | IAND | IOR | IEOR
+TYPE_PARSER(construct<ReduceOperation>(Parser<DefinedOperator>{}) ||
+ construct<ReduceOperation>(Parser<ProcedureDesignator>{}))
+
// R1130 locality-spec ->
// LOCAL ( variable-name-list ) | LOCAL_INIT ( variable-name-list ) |
+// REDUCE ( reduce-operation : variable-name-list ) |
// SHARED ( variable-name-list ) | DEFAULT ( NONE )
TYPE_PARSER(construct<LocalitySpec>(construct<LocalitySpec::Local>(
"LOCAL" >> parenthesized(listOfNames))) ||
construct<LocalitySpec>(construct<LocalitySpec::LocalInit>(
"LOCAL_INIT"_sptok >> parenthesized(listOfNames))) ||
+ construct<LocalitySpec>(construct<LocalitySpec::Reduce>(
+ "REDUCE"_sptok >> "("_tok >> Parser<ReduceOperation>{} / ":",
+ listOfNames / ")")) ||
construct<LocalitySpec>(construct<LocalitySpec::Shared>(
"SHARED" >> parenthesized(listOfNames))) ||
construct<LocalitySpec>(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 1639e900903fe..969b9c3a3802b 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -1038,6 +1038,10 @@ class UnparseVisitor {
void Unparse(const LocalitySpec::LocalInit &x) {
Word("LOCAL_INIT("), Walk(x.v, ", "), Put(')');
}
+ void Unparse(const LocalitySpec::Reduce &x) {
+ Word("REDUCE("), Walk(std::get<parser::ReduceOperation>(x.t)), Put(':');
+ Walk(std::get<std::list<parser::Name>>(x.t), ", "), Put(')');
+ }
void Unparse(const LocalitySpec::Shared &x) {
Word("SHARED("), Walk(x.v, ", "), Put(')');
}
diff --git a/flang/lib/Semantics/check-do-forall.cpp b/flang/lib/Semantics/check-do-forall.cpp
index c1eab090a4bb1..450a6ccda172b 100644
--- a/flang/lib/Semantics/check-do-forall.cpp
+++ b/flang/lib/Semantics/check-do-forall.cpp
@@ -683,6 +683,89 @@ class DoContext {
}
}
+ void CheckReduce(
+ const parser::LocalitySpec::Reduce &reduce) const {
+ const parser::ReduceOperation &reduceOperation =
+ std::get<parser::ReduceOperation>(reduce.t);
+ // F'2023 C1132, reduction variables should have suitable intrinsic type
+ bool supported_identifier = true;
+ common::visit(
+ common::visitors{
+ [&](const parser::DefinedOperator &dOpr) {
+ const auto &intrinsicOp{
+ std::get<parser::DefinedOperator::IntrinsicOperator>(dOpr.u)
+ };
+ for (const Fortran::parser::Name &x :
+ std::get<std::list<Fortran::parser::Name>>(reduce.t)) {
+ const auto *type{x.symbol->GetType()};
+ bool suitable_type = false;
+ switch (intrinsicOp) {
+ case parser::DefinedOperator::IntrinsicOperator::Add:
+ case parser::DefinedOperator::IntrinsicOperator::Multiply:
+ if (type->IsNumeric(TypeCategory::Integer) ||
+ type->IsNumeric(TypeCategory::Real) ||
+ type->IsNumeric(TypeCategory::Complex)) {
+ // TODO: check composite type.
+ suitable_type = true;
+ }
+ break;
+ case parser::DefinedOperator::IntrinsicOperator::AND:
+ case parser::DefinedOperator::IntrinsicOperator::OR:
+ case parser::DefinedOperator::IntrinsicOperator::EQV:
+ case parser::DefinedOperator::IntrinsicOperator::NEQV:
+ if (type->category() == DeclTypeSpec::Category::Logical) {
+ suitable_type = true;
+ }
+ break;
+ default:
+ supported_identifier = false;
+ return;
+ }
+ if (!suitable_type) {
+ context_.Say(currentStatementSourcePosition_,
+ "Reduction variable '%s' does not have a "
+ "suitable type."_err_en_US, x.symbol->name());
+ }
+ }
+ },
+ [&](const parser::ProcedureDesignator &procD) {
+ const parser::Name *name{std::get_if<parser::Name>(&procD.u)};
+ if (!(name && name->symbol)) {
+ supported_identifier = false;
+ return;
+ }
+ const SourceName &realName{name->symbol->GetUltimate().name()};
+ for (const Fortran::parser::Name &x : std::get<std::list<
+ Fortran::parser::Name>>(reduce.t)) {
+ const auto *type{x.symbol->GetType()};
+ bool suitable_type = false;
+ if (realName == "max" || realName == "min") {
+ if (type->IsNumeric(TypeCategory::Integer) ||
+ type->IsNumeric(TypeCategory::Real))
+ suitable_type = true;
+ } else if (realName == "iand" || realName == "ior" ||
+ realName == "ieor") {
+ if (type->IsNumeric(TypeCategory::Integer))
+ suitable_type = true;
+ } else {
+ supported_identifier = false;
+ return;
+ }
+ if (!suitable_type) {
+ context_.Say(currentStatementSourcePosition_,
+ "Reduction variable '%s' does not have a "
+ "suitable type."_err_en_US, x.symbol->name());
+ }
+ }
+ }
+ },
+ reduceOperation.u);
+ if (!supported_identifier) {
+ context_.Say(currentStatementSourcePosition_,
+ "Invalid reduction identifier in REDUCE clause."_err_en_US);
+ }
+ }
+
// C1123, concurrent limit or step expressions can't reference index-names
void CheckConcurrentHeader(const parser::ConcurrentHeader &header) const {
if (const auto &mask{
@@ -737,6 +820,12 @@ class DoContext {
std::get<std::optional<parser::ScalarLogicalExpr>>(header.t)}) {
CheckMaskDoesNotReferenceLocal(*mask, localVars);
}
+ for (auto &ls : localitySpecs) {
+ if (const auto *reduce{
+ std::get_if<parser::LocalitySpec::Reduce>(&ls.u)}) {
+ CheckReduce(*reduce);
+ }
+ }
CheckDefaultNoneImpliesExplicitLocality(localitySpecs, block);
}
}
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 40eee89de131a..61f0811982feb 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -537,7 +537,9 @@ class ScopeHandler : public ImplicitRulesVisitor {
void SayAlreadyDeclared(const SourceName &, const SourceName &);
void SayWithReason(
const parser::Name &, Symbol &, MessageFixedText &&, Message &&);
- void SayWithDecl(const parser::Name &, Symbol &, MessageFixedText &&);
+ template <typename... A>
+ void SayWithDecl(const parser::Name &, Symbol &, MessageFixedText &&,
+ A &&...args);
void SayLocalMustBeVariable(const parser::Name &, Symbol &);
void SayDerivedType(const SourceName &, MessageFixedText &&, const Scope &);
void Say2(const SourceName &, MessageFixedText &&, const SourceName &,
@@ -1041,10 +1043,10 @@ class DeclarationVisitor : public ArraySpecVisitor,
Symbol &DeclareObjectEntity(const parser::Name &, Attrs = Attrs{});
// Make sure that there's an entity in an enclosing scope called Name
Symbol &FindOrDeclareEnclosingEntity(const parser::Name &);
- // Declare a LOCAL/LOCAL_INIT entity. If there isn't a type specified
+ // Declare a LOCAL/LOCAL_INIT/REDUCE entity. If there isn't a type specified
// it comes from the entity in the containing scope, or implicit rules.
// Return pointer to the new symbol, or nullptr on error.
- Symbol *DeclareLocalEntity(const parser::Name &);
+ Symbol *DeclareLocalEntity(const parser::Name &, Symbol::Flag);
// Declare a statement entity (i.e., an implied DO loop index for
// a DATA statement or an array constructor). If there isn't an explict
// type specified, implicit rules apply. Return pointer to the new symbol,
@@ -1145,7 +1147,8 @@ class DeclarationVisitor : public ArraySpecVisitor,
const parser::Name *FindComponent(const parser::Name *, const parser::Name &);
void Initialization(const parser::Name &, const parser::Initialization &,
bool inComponentDecl);
- bool PassesLocalityChecks(const parser::Name &name, Symbol &symbol);
+ bool PassesLocalityChecks(const parser::Name &name, Symbol &symbol,
+ Symbol::Flag flag);
bool CheckForHostAssociatedImplicit(const parser::Name &);
// Declare an object or procedure entity.
@@ -1214,6 +1217,7 @@ class ConstructVisitor : public virtual DeclarationVisitor {
bool Pre(const parser::ConcurrentHeader &);
bool Pre(const parser::LocalitySpec::Local &);
bool Pre(const parser::LocalitySpec::LocalInit &);
+ bool Pre(const parser::LocalitySpec::Reduce &);
bool Pre(const parser::LocalitySpec::Shared &);
bool Pre(const parser::AcSpec &);
bool Pre(const parser::AcImpliedDo &);
@@ -1573,6 +1577,7 @@ class ResolveNamesVisitor : public virtual ScopeHandler,
ResolveName(*parser::Unwrap<parser::Name>(x.name));
}
void Post(const parser::ProcComponentRef &);
+ bool Pre(const parser::ReduceOperation &);
bool Pre(const parser::FunctionReference &);
bool Pre(const parser::CallStmt &);
bool Pre(const parser::ImportStmt &);
@@ -2254,9 +2259,11 @@ void ScopeHandler::SayWithReason(const parser::Name &name, Symbol &symbol,
context().SetError(symbol, isFatal);
}
-void ScopeHandler::SayWithDecl(
- const parser::Name &name, Symbol &symbol, MessageFixedText &&msg) {
- auto &message{Say(name, std::move(msg), symbol.name())
+template <typename... A> void ScopeHandler::SayWithDecl(
+ const parser::Name &name, Symbol &symbol, MessageFixedText &&msg,
+ A &&...args) {
+ auto &message{Say(name.source, std::move(msg), symbol.name(),
+ std::forward<A>(args)...)
.Attach(Message{symbol.name(),
symbol.test(Symbol::Flag::Implicit)
? "Implicit declaration of '%s'"_en_US
@@ -6458,44 +6465,60 @@ bool DeclarationVisitor::PassesSharedLocalityChecks(
return true;
}
-// Checks for locality-specs LOCAL and LOCAL_INIT
+// Checks for locality-specs LOCAL, LOCAL_INIT, and REDUCE
bool DeclarationVisitor::PassesLocalityChecks(
- const parser::Name &name, Symbol &symbol) {
- if (IsAllocatable(symbol)) { // C1128
- SayWithDecl(name, symbol,
- "ALLOCATABLE variable '%s' not allowed in a locality-spec"_err_en_US);
+ const parser::Name &name, Symbol &symbol, Symbol::Flag flag) {
+ bool isReduce = flag == Symbol::Flag::LocalityReduce;
+ if (IsAllocatable(symbol) && !isReduce) { // C1128, F'2023 C1130
+ SayWithDecl(name, symbol, "ALLOCATABLE variable '%s' not allowed in a "
+ "LOCAL%s locality-spec"_err_en_US,
+ (flag == Symbol::Flag::LocalityLocalInit) ? "_INIT" : "");
return false;
}
- if (IsOptional(symbol)) { // C1128
+ if (IsOptional(symbol)) { // C1128, F'2023 C1130-C1131
SayWithDecl(name, symbol,
"OPTIONAL argument '%s' not allowed in a locality-spec"_err_en_US);
return false;
}
- if (IsIntentIn(symbol)) { // C1128
+ if (IsIntentIn(symbol)) { // C1128, F'2023 C1130-C1131
SayWithDecl(name, symbol,
"INTENT IN argument '%s' not allowed in a locality-spec"_err_en_US);
return false;
}
- if (IsFinalizable(symbol)) { // C1128
- SayWithDecl(name, symbol,
- "Finalizable variable '%s' not allowed in a locality-spec"_err_en_US);
+ if (IsFinalizable(symbol) && !isReduce) { // C1128, F'2023 C1130
+ SayWithDecl(name, symbol, "Finalizable variable '%s' not allowed in a "
+ "LOCAL%s locality-spec"_err_en_US,
+ (flag == Symbol::Flag::LocalityLocalInit) ? "_INIT" : "");
return false;
}
- if (evaluate::IsCoarray(symbol)) { // C1128
- SayWithDecl(
- name, symbol, "Coarray '%s' not allowed in a locality-spec"_err_en_US);
+ if (evaluate::IsCoarray(symbol) && !isReduce) { // C1128, F'2023 C1130
+ SayWithDecl(name, symbol, "Coarray '%s' not allowed in a "
+ "LOCAL%s locality-spec"_err_en_US,
+ (flag == Symbol::Flag::LocalityLocalInit) ? "_INIT" : "");
return false;
}
if (const DeclTypeSpec * type{symbol.GetType()}) {
if (type->IsPolymorphic() && IsDummy(symbol) &&
- !IsPointer(symbol)) { // C1128
- SayWithDecl(name, symbol,
- "Nonpointer polymorphic argument '%s' not allowed in a "
- "locality-spec"_err_en_US);
+ !IsPointer(symbol) && !isReduce) { // C1128, F'2023 C1130
+ SayWithDecl(name, symbol, "Nonpointer polymorphic argument '%s' not "
+ "allowed in a LOCAL%s locality-spec"_err_en_US,
+ (flag == Symbol::Flag::LocalityLocalInit) ? "_INIT" : "");
return false;
}
}
- if (IsAssumedSizeArray(symbol)) { // C1128
+ if (symbol.attrs().test(Attr::ASYNCHRONOUS) && isReduce) { // F'2023 C1131
+ SayWithDecl(name, symbol,
+ "ASYNCHRONOUS variable '%s' not allowed in a "
+ "REDUCE locality-spec"_err_en_US);
+ return false;
+ }
+ if (symbol.attrs().test(Attr::VOLATILE) && isReduce) { // F'2023 C1131
+ SayWithDecl(name, symbol,
+ "VOLATILE variable '%s' not allowed in a "
+ "REDUCE locality-spec"_err_en_US);
+ return false;
+ }
+ if (IsAssumedSizeArray(symbol)) { // C1128, F'2023 C1130-C1131
SayWithDecl(name, symbol,
"Assumed size array '%s' not allowed in a locality-spec"_err_en_US);
return false;
@@ -6524,9 +6547,10 @@ Symbol &DeclarationVisitor::FindOrDeclareEnclosingEntity(
return *prev;
}
-Symbol *DeclarationVisitor::DeclareLocalEntity(const parser::Name &name) {
+Symbol *DeclarationVisitor::DeclareLocalEntity(
+ const parser::Name &name, Symbol::Flag flag) {
Symbol &prev{FindOrDeclareEnclosingEntity(name)};
- if (!PassesLocalityChecks(name, prev)) {
+ if (!PassesLocalityChecks(name, prev, flag)) {
return nullptr;
}
return &MakeHostAssocSymbol(name, prev);
@@ -6866,7 +6890,7 @@ bool ConstructVisitor::Pre(const parser::ConcurrentHeader &header) {
bool ConstructVisitor::Pre(const parser::LocalitySpec::Local &x) {
for (auto &name : x.v) {
- if (auto *symbol{DeclareLocalEntity(name)}) {
+ if (auto *symbol{DeclareLocalEntity(name, Symbol::Flag::LocalityLocal)}) {
symbol->set(Symbol::Flag::LocalityLocal);
}
}
@@ -6875,13 +6899,25 @@ bool ConstructVisitor::Pre(const parser::LocalitySpec::Local &x) {
bool ConstructVisitor::Pre(const parser::LocalitySpec::LocalInit &x) {
for (auto &name : x.v) {
- if (auto *symbol{DeclareLocalEntity(name)}) {
+ if (auto *symbol{
+ DeclareLocalEntity(name, Symbol::Flag::LocalityLocalInit)}) {
symbol->set(Symbol::Flag::LocalityLocalInit);
}
}
return false;
}
+bool ConstructVisitor::Pre(const parser::LocalitySpec::Reduce &x) {
+ Walk(std::get<parser::ReduceOperation>(x.t));
+ for (auto &name : std::get<std::list<parser::Name>>(x.t)) {
+ if (auto *symbol{
+ DeclareLocalEntity(name, Symbol::Flag::LocalityReduce)}) {
+ symbol->set(Symbol::Flag::LocalityReduce);
+ }
+ }
+ return false;
+}
+
bool ConstructVisitor::Pre(const parser::LocalitySpec::Shared &x) {
for (const auto &name : x.v) {
if (!FindSymbol(name)) {
@@ -8216,6 +8252,15 @@ void ResolveNamesVisitor::HandleProcedureName(
}
}
+bool ResolveNamesVisitor::Pre(const parser:...
[truncated]
|
| // MAX | MIN | IAND | IOR | IEOR | ||
| struct ReduceOperation { | ||
| UNION_CLASS_BOILERPLATE(ReduceOperation); | ||
| std::variant<DefinedOperator, ProcedureDesignator> u; |
There was a problem hiding this comment.
Neither of the types in this variant are appropriate for representing a reduction operator. Use an ENUM_CLASS or (better) reuse the reduction operators already present for OpenACC reductions or !$CUF KERNEL DO REDUCE() reductions.
There was a problem hiding this comment.
I'll repeat a comment sent personally to you before.
I tried an implementation in a similar way to AccReductionOperator::Operator. It cannot tell the reason for parsing failures. The name of AccReductionOperator::Operator is redundant. I tried to add ReduceOperation::Add directly (without ::Operator::), but the OpenACC version has a sourced parser. Defining 'source' in 'ReduceOperation' does not match with other code.
There was no CUF REDUCE support that time. Should I check it?
There was a problem hiding this comment.
Yes. And you can handle the error recovery of an enumerated operator in the parser rather than allowing the parse tree to hold an overly general procedure designator.
| // F'2023 R1131 reduce-operation -> | ||
| // + | * | .AND. | .OR. | .EQV. | .NEQV. | | ||
| // MAX | MIN | IAND | IOR | IEOR | ||
| TYPE_PARSER(construct<ReduceOperation>(Parser<DefinedOperator>{}) || |
There was a problem hiding this comment.
See comment above about the parse tree node.
| // SHARED ( variable-name-list ) | DEFAULT ( NONE ) | ||
| TYPE_PARSER(construct<LocalitySpec>(construct<LocalitySpec::Local>( | ||
| "LOCAL" >> parenthesized(listOfNames))) || | ||
| construct<LocalitySpec>(construct<LocalitySpec::LocalInit>( | ||
| "LOCAL_INIT"_sptok >> parenthesized(listOfNames))) || | ||
| construct<LocalitySpec>(construct<LocalitySpec::Reduce>( | ||
| "REDUCE"_sptok >> "("_tok >> Parser<ReduceOperation>{} / ":", |
There was a problem hiding this comment.
"REDUCE (" >> Parser<ReduceOperation>{} / ":", listOfNames / ")"
There was a problem hiding this comment.
"REDUCE (" allows any whitespace?
There was a problem hiding this comment.
It allows a space between the keyword and the parenthesis.
flang/lib/Parser/unparse.cpp
Outdated
| @@ -1038,6 +1038,10 @@ class UnparseVisitor { | |||
| void Unparse(const LocalitySpec::LocalInit &x) { | |||
| Word("LOCAL_INIT("), Walk(x.v, ", "), Put(')'); | |||
| } | |||
| void Unparse(const LocalitySpec::Reduce &x) { | |||
| Word("REDUCE("), Walk(std::get<parser::ReduceOperation>(x.t)), Put(':'); | |||
| Walk(std::get<std::list<parser::Name>>(x.t), ", "), Put(')'); | |||
There was a problem hiding this comment.
Walk(":", std::get<std::list<ReduceOperation>(x.t), ",", ")");
| SayWithDecl(name, symbol, | ||
| "ALLOCATABLE variable '%s' not allowed in a locality-spec"_err_en_US); | ||
| const parser::Name &name, Symbol &symbol, Symbol::Flag flag) { | ||
| bool isReduce = flag == Symbol::Flag::LocalityReduce; |
There was a problem hiding this comment.
Braced initialization, please.
| "ALLOCATABLE variable '%s' not allowed in a locality-spec"_err_en_US); | ||
| const parser::Name &name, Symbol &symbol, Symbol::Flag flag) { | ||
| bool isReduce = flag == Symbol::Flag::LocalityReduce; | ||
| if (IsAllocatable(symbol) && !isReduce) { // C1128, F'2023 C1130 |
There was a problem hiding this comment.
When updating a comment that is a textual reference into the standard, update any subclause, requirement, or constraint number to the latest standard, and don't bother retaining the F'2018 number: // F'2023 C1130 is fine here.
(References to standards prior to F'2018 should be retained, they can appear in explanations of how the language has changed over time.)
| if (IsAllocatable(symbol) && !isReduce) { // C1128, F'2023 C1130 | ||
| SayWithDecl(name, symbol, "ALLOCATABLE variable '%s' not allowed in a " | ||
| "LOCAL%s locality-spec"_err_en_US, | ||
| (flag == Symbol::Flag::LocalityLocalInit) ? "_INIT" : ""); |
There was a problem hiding this comment.
The parentheses are needless here in the predicate of the conditional expression.
| @@ -6866,7 +6890,7 @@ bool ConstructVisitor::Pre(const parser::ConcurrentHeader &header) { | |||
|
|
|||
| bool ConstructVisitor::Pre(const parser::LocalitySpec::Local &x) { | |||
| for (auto &name : x.v) { | |||
| if (auto *symbol{DeclareLocalEntity(name)}) { | |||
| if (auto *symbol{DeclareLocalEntity(name, Symbol::Flag::LocalityLocal)}) { | |||
There was a problem hiding this comment.
It would be less confusing if your updated DeclareLocalEntity() set the flag now, if that is possible.
flang/test/Semantics/resolve124.f90
Outdated
| subroutine s2() | ||
| ! Cannot apply logical operations to integer variables | ||
| integer :: i1, i2, i3, i4 | ||
| !ERROR: Reduction variable 'i1' does not have a suitable type. |
There was a problem hiding this comment.
Naming the unacceptable type and the "suitable" type(s) would be more useful to the programmer.
There was a problem hiding this comment.
Sure, I will put the list of suitable types.
…e in DO CONCURRENT REDUCE parsing
|
I appreciate your comments. The current version is sharing ReductionOperator among FIR/ACC/CUF. The test was also refreshed. |
| @@ -1870,6 +1870,13 @@ struct ProcComponentRef { | |||
| WRAPPER_CLASS_BOILERPLATE(ProcComponentRef, Scalar<StructureComponent>); | |||
| }; | |||
|
|
|||
| // R1522 procedure-designator -> | |||
There was a problem hiding this comment.
This code was moved up from below, but that no longer seems to be necessary.
| @@ -2236,16 +2243,31 @@ struct ConcurrentHeader { | |||
| t; | |||
| }; | |||
|
|
|||
| // OpenACC 3.2 | |||
There was a problem hiding this comment.
Add references to DO CONCURRENT REDUCE and !$CUF KERNEL DO to the comment.
Are these also the same reduction operators as OpenMP's? If so, the OpenMP code should use them too.
There was a problem hiding this comment.
The OpenMP design stems from ClauseT.h in the LLVM frontend. It allows a unique reduction identifier, so we should keep the current parser.
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Frontend/OpenMP/ClauseT.h#L258
| std::get<parser::ReductionOperator>(reduce.t)}; | ||
| // F'2023 C1132, reduction variables should have suitable intrinsic type | ||
| for (const parser::Name &x : std::get<std::list<parser::Name>>(reduce.t)) { | ||
| bool supported_identifier{false}; |
There was a problem hiding this comment.
supportedIdentifier would be consistent.
| bool supported_identifier{false}; | ||
| if (x.symbol && x.symbol->GetType()) { | ||
| const auto *type{x.symbol->GetType()}; | ||
| auto type_mismatch{[&](const char *suitable_types) { |
| const auto *type{x.symbol->GetType()}; | ||
| auto type_mismatch{[&](const char *suitable_types) { | ||
| context_.Say(currentStatementSourcePosition_, | ||
| "Reduction variable '%s' ('%s') does not have a " |
There was a problem hiding this comment.
Please leave error messages as long contiguous character literals, even if they exceed the 80 character limit. Otherwise they would be much harder to find with grep.
| MessageFixedText &&msg, A &&...args) { | ||
| auto &message{ | ||
| Say(name.source, std::move(msg), symbol.name(), std::forward<A>(args)...) | ||
| .Attach(Message{symbol.name(), |
There was a problem hiding this comment.
You can probably omit Message{ and its corresponding } here.
| SayWithDecl(name, symbol, | ||
| "Finalizable variable '%s' not allowed in a locality-spec"_err_en_US); | ||
| "Finalizable variable '%s' not allowed in a " |
There was a problem hiding this comment.
Please don't break up the messages here and following.
…ve back procedure-designator to the original position
|
Thank you for the reviews. I dealt with all the points. Please check my update again. |
OpenACC changes look good to me. |
| SayWithDecl(name, symbol, | ||
| "ALLOCATABLE variable '%s' not allowed in a locality-spec"_err_en_US); | ||
| "ALLOCATABLE variable '%s' not allowed in a LOCAL%s locality-spec"_err_en_US, | ||
| flag == Symbol::Flag::LocalityLocalInit ? "_INIT" : ""); |
There was a problem hiding this comment.
Declare const char *specName{... ? "LOCAL_INIT" : "LOCAL"}; after line 6470 and then use specName as the argument, it will be easier to understand than this repetitive conditional suffix.
| } | ||
|
|
||
| bool ConstructVisitor::Pre(const parser::LocalitySpec::Reduce &x) { | ||
| for (auto &name : std::get<std::list<parser::Name>>(x.t)) { |
|
Still LGTM. Do you have commit access or do you need someone to merge your change? |
|
It seems that I need another approval:
|
First time that I see this button. I just approve and run. |
You can test this locally with the following command:git-clang-format --diff 8aa80199751b0cd6631d057b0bfb21584acb206f ae30be50865779bd037c0becf6f758fa7b5d5db7 -- flang/examples/FeatureList/FeatureList.cpp flang/include/flang/Parser/dump-parse-tree.h flang/include/flang/Parser/parse-tree.h flang/include/flang/Semantics/symbol.h flang/lib/Lower/OpenACC.cpp flang/lib/Parser/executable-parsers.cpp flang/lib/Parser/openacc-parsers.cpp flang/lib/Parser/unparse.cpp flang/lib/Semantics/check-acc-structure.cpp flang/lib/Semantics/check-cuda.cpp flang/lib/Semantics/check-do-forall.cpp flang/lib/Semantics/resolve-names.cppView the diff from clang-format here.diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index c104eb6230..4f5da8fb70 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -1356,8 +1356,7 @@ genReductions(const Fortran::parser::AccObjectListWithReduction &objectList,
llvm::SmallVector<mlir::Attribute> &reductionRecipes) {
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
const auto &objects = std::get<Fortran::parser::AccObjectList>(objectList.t);
- const auto &op =
- std::get<Fortran::parser::ReductionOperator>(objectList.t);
+ const auto &op = std::get<Fortran::parser::ReductionOperator>(objectList.t);
mlir::acc::ReductionOperator mlirOp = getReductionOperator(op);
Fortran::evaluate::ExpressionAnalyzer ea{semanticsContext};
for (const auto &accObject : objects.v) {
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index f6251455e1..8db7ee6713 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -6514,8 +6514,7 @@ bool DeclarationVisitor::PassesLocalityChecks(
}
if (evaluate::IsCoarray(symbol) && !isReduce) { // F'2023 C1130
SayWithDecl(name, symbol,
- "Coarray '%s' not allowed in a %s locality-spec"_err_en_US,
- specName);
+ "Coarray '%s' not allowed in a %s locality-spec"_err_en_US, specName);
return false;
}
if (const DeclTypeSpec * type{symbol.GetType()}) {
|
|
@khaki3 Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested Please check whether problems have been caused by your change specifically, as How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Derived from #92480. This PR supports parsing of the DO CONCURRENT REDUCE clause in Fortran 2023. Following the style of the OpenMP parser in MLIR, the front end accepts both arbitrary operations and procedures for the REDUCE clause. But later Semantics can notify type errors and resolve procedure names.