Skip to content

Commit 09a4bcf

Browse files
kparzyszkiranchandramohan
kparzysz
and
kiranchandramohan
authored
[flang][OpenMP] Update handling of DEPEND clause (#113620)
Parse the locator list in OmpDependClause as an OmpObjectList (instead of a list of Designators). When a common block appears in the locator list, show an informative message. Implement resolving symbols in DependSinkVec in a dedicated visitor instead of having a visitor for OmpDependClause. Resolve unresolved names common blocks in OmpObjectList. Minor changes to the code organization: - rename OmpDependenceType to OmpTaskDependenceType (to follow 5.2 terminology), - rename Depend::WithLocators to Depend::DepType, - add comments with more detailed spec references to parse-tree.h. --------- Co-authored-by: Kiran Chandramohan <[email protected]>
1 parent 5ea6948 commit 09a4bcf

File tree

13 files changed

+93
-62
lines changed

13 files changed

+93
-62
lines changed

‎flang/examples/FeatureList/FeatureList.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,8 +473,8 @@ struct NodeVisitor {
473473
READ_FEATURE(OmpDependClause::InOut)
474474
READ_FEATURE(OmpDependClause::Sink)
475475
READ_FEATURE(OmpDependClause::Source)
476-
READ_FEATURE(OmpDependenceType)
477-
READ_FEATURE(OmpDependenceType::Type)
476+
READ_FEATURE(OmpTaskDependenceType)
477+
READ_FEATURE(OmpTaskDependenceType::Type)
478478
READ_FEATURE(OmpDependSinkVec)
479479
READ_FEATURE(OmpDependSinkVecLength)
480480
READ_FEATURE(OmpEndAllocators)

‎flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,9 @@ void OpenMPCounterVisitor::Post(const OmpLinearModifier::Type &c) {
222222
clauseDetails +=
223223
"modifier=" + std::string{OmpLinearModifier::EnumToString(c)} + ";";
224224
}
225-
void OpenMPCounterVisitor::Post(const OmpDependenceType::Type &c) {
225+
void OpenMPCounterVisitor::Post(const OmpTaskDependenceType::Type &c) {
226226
clauseDetails +=
227-
"type=" + std::string{OmpDependenceType::EnumToString(c)} + ";";
227+
"type=" + std::string{OmpTaskDependenceType::EnumToString(c)} + ";";
228228
}
229229
void OpenMPCounterVisitor::Post(const OmpMapClause::Type &c) {
230230
clauseDetails += "type=" + std::string{OmpMapClause::EnumToString(c)} + ";";

‎flang/examples/FlangOmpReport/FlangOmpReportVisitor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ struct OpenMPCounterVisitor {
7373
void Post(const OmpDeviceTypeClause::Type &c);
7474
void Post(const OmpScheduleModifierType::ModType &c);
7575
void Post(const OmpLinearModifier::Type &c);
76-
void Post(const OmpDependenceType::Type &c);
76+
void Post(const OmpTaskDependenceType::Type &c);
7777
void Post(const OmpMapClause::Type &c);
7878
void Post(const OmpScheduleClause::ScheduleType &c);
7979
void Post(const OmpIfClause::DirectiveNameModifier &c);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,8 +513,8 @@ class ParseTreeDumper {
513513
NODE(OmpDependClause, InOut)
514514
NODE(OmpDependClause, Sink)
515515
NODE(OmpDependClause, Source)
516-
NODE(parser, OmpDependenceType)
517-
NODE_ENUM(OmpDependenceType, Type)
516+
NODE(parser, OmpTaskDependenceType)
517+
NODE_ENUM(OmpTaskDependenceType, Type)
518518
NODE(parser, OmpDependSinkVec)
519519
NODE(parser, OmpDependSinkVecLength)
520520
NODE(parser, OmpEndAllocators)

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3439,6 +3439,18 @@ struct OmpObject {
34393439

34403440
WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
34413441

3442+
// Ref: [4.5:169-170], [5.0:254-256], [5.1:287-289], [5.2:321]
3443+
//
3444+
// task-dependence-type -> // "dependence-type" in 5.1 and before
3445+
// IN | OUT | INOUT | // since 4.5
3446+
// SOURCE | SINK | // since 4.5, until 5.1
3447+
// MUTEXINOUTSET | DEPOBJ | // since 5.0
3448+
// INOUTSET // since 5.2
3449+
struct OmpTaskDependenceType {
3450+
ENUM_CLASS(Type, In, Out, Inout, Source, Sink)
3451+
WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Type);
3452+
};
3453+
34423454
// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple
34433455
// iterator-modifier -> iterator-specifier-list
34443456
struct OmpIteratorSpecifier {
@@ -3534,27 +3546,27 @@ struct OmpDependSinkVecLength {
35343546
std::tuple<DefinedOperator, ScalarIntConstantExpr> t;
35353547
};
35363548

3537-
// 2.13.9 depend-vec -> iterator [+/- depend-vec-length],...,iterator[...]
3549+
// 2.13.9 depend-vec -> induction-variable [depend-vec-length], ...
35383550
struct OmpDependSinkVec {
35393551
TUPLE_CLASS_BOILERPLATE(OmpDependSinkVec);
35403552
std::tuple<Name, std::optional<OmpDependSinkVecLength>> t;
35413553
};
35423554

3543-
// 2.13.9 depend-type -> IN | OUT | INOUT | SOURCE | SINK
3544-
struct OmpDependenceType {
3545-
ENUM_CLASS(Type, In, Out, Inout, Source, Sink)
3546-
WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Type);
3547-
};
3548-
3549-
// 2.13.9 depend-clause -> DEPEND (((IN | OUT | INOUT) : variable-name-list) |
3550-
// SOURCE | SINK : depend-vec)
3555+
// Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289], [5.2:323-324]
3556+
//
3557+
// depend-clause ->
3558+
// DEPEND(SOURCE) | // since 4.5, until 5.1
3559+
// DEPEND(SINK: depend-vec) | // since 4.5, until 5.1
3560+
// DEPEND([depend-modifier,]dependence-type: locator-list) // since 4.5
3561+
//
3562+
// depend-modifier -> iterator-modifier // since 5.0
35513563
struct OmpDependClause {
35523564
UNION_CLASS_BOILERPLATE(OmpDependClause);
35533565
EMPTY_CLASS(Source);
35543566
WRAPPER_CLASS(Sink, std::list<OmpDependSinkVec>);
35553567
struct InOut {
35563568
TUPLE_CLASS_BOILERPLATE(InOut);
3557-
std::tuple<OmpDependenceType, std::list<Designator>> t;
3569+
std::tuple<OmpTaskDependenceType, OmpObjectList> t;
35583570
};
35593571
std::variant<Source, Sink, InOut> u;
35603572
};

‎flang/lib/Lower/OpenMP/ClauseProcessor.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -798,11 +798,11 @@ bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const {
798798
return findRepeatableClause<omp::clause::Depend>(
799799
[&](const omp::clause::Depend &clause, const parser::CharBlock &) {
800800
using Depend = omp::clause::Depend;
801-
assert(std::holds_alternative<Depend::WithLocators>(clause.u) &&
802-
"Only the modern form is handled at the moment");
803-
auto &modern = std::get<Depend::WithLocators>(clause.u);
804-
auto kind = std::get<Depend::TaskDependenceType>(modern.t);
805-
auto &objects = std::get<omp::ObjectList>(modern.t);
801+
assert(std::holds_alternative<Depend::DepType>(clause.u) &&
802+
"Only the form with dependence type is handled at the moment");
803+
auto &depType = std::get<Depend::DepType>(clause.u);
804+
auto kind = std::get<Depend::TaskDependenceType>(depType.t);
805+
auto &objects = std::get<omp::ObjectList>(depType.t);
806806

807807
mlir::omp::ClauseTaskDependAttr dependTypeOperand =
808808
genDependKindAttr(firOpBuilder, kind);

‎flang/lib/Lower/OpenMP/Clauses.cpp

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ Depend make(const parser::OmpClause::Depend &inp,
555555
using Iteration = Doacross::Vector::value_type; // LoopIterationT
556556

557557
CLAUSET_ENUM_CONVERT( //
558-
convert1, parser::OmpDependenceType::Type, Depend::TaskDependenceType,
558+
convert1, parser::OmpTaskDependenceType::Type, Depend::TaskDependenceType,
559559
// clang-format off
560560
MS(In, In)
561561
MS(Out, Out)
@@ -593,17 +593,13 @@ Depend make(const parser::OmpClause::Depend &inp,
593593
return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink,
594594
/*Vector=*/makeList(s.v, convert2)}};
595595
},
596-
// Depend::WithLocators
596+
// Depend::DepType
597597
[&](const wrapped::InOut &s) -> Variant {
598-
auto &t0 = std::get<parser::OmpDependenceType>(s.t);
599-
auto &t1 = std::get<std::list<parser::Designator>>(s.t);
600-
auto convert4 = [&](const parser::Designator &t) {
601-
return makeObject(t, semaCtx);
602-
};
603-
return Depend::WithLocators{
604-
{/*TaskDependenceType=*/convert1(t0.v),
605-
/*Iterator=*/std::nullopt,
606-
/*LocatorList=*/makeList(t1, convert4)}};
598+
auto &t0 = std::get<parser::OmpTaskDependenceType>(s.t);
599+
auto &t1 = std::get<parser::OmpObjectList>(s.t);
600+
return Depend::DepType{{/*TaskDependenceType=*/convert1(t0.v),
601+
/*Iterator=*/std::nullopt,
602+
/*LocatorList=*/makeObjects(t1, semaCtx)}};
607603
},
608604
},
609605
inp.v.u)};

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -365,18 +365,18 @@ TYPE_PARSER(construct<OmpDependSinkVecLength>(
365365
TYPE_PARSER(
366366
construct<OmpDependSinkVec>(name, maybe(Parser<OmpDependSinkVecLength>{})))
367367

368-
TYPE_PARSER(
369-
construct<OmpDependenceType>("IN"_id >> pure(OmpDependenceType::Type::In) ||
370-
"INOUT" >> pure(OmpDependenceType::Type::Inout) ||
371-
"OUT" >> pure(OmpDependenceType::Type::Out)))
368+
TYPE_PARSER(construct<OmpTaskDependenceType>(
369+
"IN"_id >> pure(OmpTaskDependenceType::Type::In) ||
370+
"INOUT" >> pure(OmpTaskDependenceType::Type::Inout) ||
371+
"OUT" >> pure(OmpTaskDependenceType::Type::Out)))
372372

373373
TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
374374
construct<OmpDependClause>(construct<OmpDependClause::Sink>(
375375
"SINK :" >> nonemptyList(Parser<OmpDependSinkVec>{}))) ||
376376
construct<OmpDependClause>(
377377
construct<OmpDependClause::Source>("SOURCE"_tok)) ||
378378
construct<OmpDependClause>(construct<OmpDependClause::InOut>(
379-
Parser<OmpDependenceType>{}, ":" >> nonemptyList(designator))))
379+
Parser<OmpTaskDependenceType>{}, ":" >> Parser<OmpObjectList>{})))
380380

381381
// 2.15.3.7 LINEAR (linear-list: linear-step)
382382
// linear-list -> list | modifier(list)

‎flang/lib/Parser/unparse.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2216,9 +2216,9 @@ class UnparseVisitor {
22162216
}
22172217
void Unparse(const OmpDependClause::InOut &x) {
22182218
Put("(");
2219-
Walk(std::get<OmpDependenceType>(x.t));
2219+
Walk(std::get<OmpTaskDependenceType>(x.t));
22202220
Put(":");
2221-
Walk(std::get<std::list<Designator>>(x.t), ",");
2221+
Walk(std::get<OmpObjectList>(x.t));
22222222
Put(")");
22232223
}
22242224
bool Pre(const OmpDependClause &x) {
@@ -2829,7 +2829,7 @@ class UnparseVisitor {
28292829
OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier
28302830
WALK_NESTED_ENUM(OmpScheduleModifierType, ModType) // OMP schedule-modifier
28312831
WALK_NESTED_ENUM(OmpLinearModifier, Type) // OMP linear-modifier
2832-
WALK_NESTED_ENUM(OmpDependenceType, Type) // OMP dependence-type
2832+
WALK_NESTED_ENUM(OmpTaskDependenceType, Type) // OMP task-dependence-type
28332833
WALK_NESTED_ENUM(OmpScheduleClause, ScheduleType) // OMP schedule-type
28342834
WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier
28352835
WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE

‎flang/lib/Semantics/check-omp-structure.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3288,15 +3288,21 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) {
32883288
parser::ToUpperCaseLetters(getDirectiveName(GetContext().directive)));
32893289
}
32903290
if (const auto *inOut{std::get_if<parser::OmpDependClause::InOut>(&x.v.u)}) {
3291-
const auto &designators{std::get<std::list<parser::Designator>>(inOut->t)};
3292-
for (const auto &ele : designators) {
3293-
if (const auto *dataRef{std::get_if<parser::DataRef>(&ele.u)}) {
3294-
CheckDependList(*dataRef);
3295-
if (const auto *arr{
3296-
std::get_if<common::Indirection<parser::ArrayElement>>(
3297-
&dataRef->u)}) {
3298-
CheckArraySection(arr->value(), GetLastName(*dataRef),
3299-
llvm::omp::Clause::OMPC_depend);
3291+
for (const auto &object : std::get<parser::OmpObjectList>(inOut->t).v) {
3292+
if (const auto *name{std::get_if<parser::Name>(&object.u)}) {
3293+
context_.Say(GetContext().clauseSource,
3294+
"Common block name ('%s') cannot appear in a DEPEND "
3295+
"clause"_err_en_US,
3296+
name->ToString());
3297+
} else if (auto *designator{std::get_if<parser::Designator>(&object.u)}) {
3298+
if (auto *dataRef{std::get_if<parser::DataRef>(&designator->u)}) {
3299+
CheckDependList(*dataRef);
3300+
if (const auto *arr{
3301+
std::get_if<common::Indirection<parser::ArrayElement>>(
3302+
&dataRef->u)}) {
3303+
CheckArraySection(arr->value(), GetLastName(*dataRef),
3304+
llvm::omp::Clause::OMPC_depend);
3305+
}
33003306
}
33013307
}
33023308
}

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

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,20 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
435435
bool Pre(const parser::OpenMPAllocatorsConstruct &);
436436
void Post(const parser::OpenMPAllocatorsConstruct &);
437437

438+
void Post(const parser::OmpObjectList &x) {
439+
// The objects from OMP clauses should have already been resolved,
440+
// except common blocks (the ResolveNamesVisitor does not visit
441+
// parser::Name, those are dealt with as members of other structures).
442+
// Iterate over elements of x, and resolve any common blocks that
443+
// are still unresolved.
444+
for (const parser::OmpObject &obj : x.v) {
445+
auto *name{std::get_if<parser::Name>(&obj.u)};
446+
if (name && !name->symbol) {
447+
Resolve(*name, currScope().MakeCommonBlock(name->source));
448+
}
449+
}
450+
}
451+
438452
// 2.15.3 Data-Sharing Attribute Clauses
439453
void Post(const parser::OmpDefaultClause &);
440454
bool Pre(const parser::OmpClause::Shared &x) {
@@ -531,16 +545,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
531545
return false;
532546
}
533547

534-
bool Pre(const parser::OmpDependClause &x) {
535-
if (const auto *dependSink{
536-
std::get_if<parser::OmpDependClause::Sink>(&x.u)}) {
537-
const auto &dependSinkVec{dependSink->v};
538-
for (const auto &dependSinkElement : dependSinkVec) {
539-
const auto &name{std::get<parser::Name>(dependSinkElement.t)};
540-
ResolveName(&name);
541-
}
542-
}
543-
return false;
548+
void Post(const parser::OmpDependSinkVec &x) {
549+
const auto &name{std::get<parser::Name>(x.t)};
550+
ResolveName(&name);
544551
}
545552

546553
bool Pre(const parser::OmpClause::UseDevicePtr &x) {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50
2+
3+
subroutine f00
4+
integer :: x
5+
common /cc/ x
6+
!ERROR: Common block name ('cc') cannot appear in a DEPEND clause
7+
!$omp task depend(in: /cc/)
8+
x = 0
9+
!$omp end task
10+
end

‎llvm/include/llvm/Frontend/OpenMP/ClauseT.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -503,15 +503,15 @@ struct DependT {
503503
using LocatorList = ObjectListT<I, E>;
504504
using TaskDependenceType = tomp::type::TaskDependenceType;
505505

506-
struct WithLocators { // Modern form
506+
struct DepType { // The form with task dependence type.
507507
using TupleTrait = std::true_type;
508508
// Empty LocatorList means "omp_all_memory".
509509
std::tuple<TaskDependenceType, OPT(Iterator), LocatorList> t;
510510
};
511511

512512
using Doacross = DoacrossT<T, I, E>;
513513
using UnionTrait = std::true_type;
514-
std::variant<Doacross, WithLocators> u; // Doacross form is legacy
514+
std::variant<Doacross, DepType> u; // Doacross form is legacy
515515
};
516516

517517
// V5.2: [3.5] `destroy` clause

0 commit comments

Comments
 (0)