Skip to content

[Flang][OpenMP][Sema] Adding parsing and semantic support for scan directive. #102792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Nov 15, 2024
6 changes: 6 additions & 0 deletions flang/include/flang/Semantics/openmp-directive-sets.h
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,12 @@ static const OmpDirectiveSet workShareSet{
} | allDoSet,
};

//===----------------------------------------------------------------------===//
// Directive sets for parent directives that do allow/not allow a construct
//===----------------------------------------------------------------------===//

static const OmpDirectiveSet scanParentAllowedSet{allDoSet | allSimdSet};

//===----------------------------------------------------------------------===//
// Directive sets for allowed/not allowed nested directives
//===----------------------------------------------------------------------===//
Expand Down
3 changes: 2 additions & 1 deletion flang/include/flang/Semantics/symbol.h
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,8 @@ class Symbol {
OmpDeclarativeAllocateDirective, OmpExecutableAllocateDirective,
OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, OmpDeclareReduction,
OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined,
OmpImplicit, OmpDependObject);
OmpImplicit, OmpDependObject, OmpInclusiveScan, OmpExclusiveScan,
OmpInScanReduction);
using Flags = common::EnumSet<Flag, Flag_enumSize>;

const Scope &owner() const { return *owner_; }
Expand Down
3 changes: 3 additions & 0 deletions flang/lib/Lower/OpenMP/OpenMP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2520,6 +2520,9 @@ static void genOMPDispatch(lower::AbstractConverter &converter,
case llvm::omp::Directive::OMPD_parallel:
genStandaloneParallel(converter, symTable, semaCtx, eval, loc, queue, item);
break;
case llvm::omp::Directive::OMPD_scan:
TODO(loc, "Unhandled directive " + llvm::omp::getOpenMPDirectiveName(dir));
break;
case llvm::omp::Directive::OMPD_section:
llvm_unreachable("genOMPDispatch: OMPD_section");
// Lowered in the enclosing genSectionsOp.
Expand Down
5 changes: 5 additions & 0 deletions flang/lib/Parser/openmp-parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,8 @@ TYPE_PARSER(
construct<OmpClause>(construct<OmpClause::DynamicAllocators>()) ||
"ENTER" >> construct<OmpClause>(construct<OmpClause::Enter>(
parenthesized(Parser<OmpObjectList>{}))) ||
"EXCLUSIVE" >> construct<OmpClause>(construct<OmpClause::Exclusive>(
parenthesized(Parser<OmpObjectList>{}))) ||
"FILTER" >> construct<OmpClause>(construct<OmpClause::Filter>(
parenthesized(scalarIntExpr))) ||
"FINAL" >> construct<OmpClause>(construct<OmpClause::Final>(
Expand All @@ -578,6 +580,8 @@ TYPE_PARSER(
"IF" >> construct<OmpClause>(construct<OmpClause::If>(
parenthesized(Parser<OmpIfClause>{}))) ||
"INBRANCH" >> construct<OmpClause>(construct<OmpClause::Inbranch>()) ||
"INCLUSIVE" >> construct<OmpClause>(construct<OmpClause::Inclusive>(
parenthesized(Parser<OmpObjectList>{}))) ||
"IS_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::IsDevicePtr>(
parenthesized(Parser<OmpObjectList>{}))) ||
"LASTPRIVATE" >> construct<OmpClause>(construct<OmpClause::Lastprivate>(
Expand Down Expand Up @@ -790,6 +794,7 @@ TYPE_PARSER(sourced(construct<OpenMPFlushConstruct>(verbatim("FLUSH"_tok),
TYPE_PARSER(sourced(construct<OmpSimpleStandaloneDirective>(first(
"BARRIER" >> pure(llvm::omp::Directive::OMPD_barrier),
"ORDERED" >> pure(llvm::omp::Directive::OMPD_ordered),
"SCAN" >> pure(llvm::omp::Directive::OMPD_scan),
"TARGET ENTER DATA" >> pure(llvm::omp::Directive::OMPD_target_enter_data),
"TARGET EXIT DATA" >> pure(llvm::omp::Directive::OMPD_target_exit_data),
"TARGET UPDATE" >> pure(llvm::omp::Directive::OMPD_target_update),
Expand Down
3 changes: 3 additions & 0 deletions flang/lib/Parser/unparse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2393,6 +2393,9 @@ class UnparseVisitor {
case llvm::omp::Directive::OMPD_barrier:
Word("BARRIER ");
break;
case llvm::omp::Directive::OMPD_scan:
Word("SCAN ");
break;
case llvm::omp::Directive::OMPD_taskwait:
Word("TASKWAIT ");
break;
Expand Down
218 changes: 144 additions & 74 deletions flang/lib/Semantics/check-omp-structure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
#include "flang/Semantics/tools.h"
#include <variant>

namespace Fortran::semantics {

Expand Down Expand Up @@ -747,62 +748,69 @@ void OmpStructureChecker::CheckSIMDNest(const parser::OpenMPConstruct &c) {
// current context yet.
// TODO: Check for declare simd regions.
bool eligibleSIMD{false};
common::visit(Fortran::common::visitors{
// Allow `!$OMP ORDERED SIMD`
[&](const parser::OpenMPBlockConstruct &c) {
const auto &beginBlockDir{
std::get<parser::OmpBeginBlockDirective>(c.t)};
const auto &beginDir{
std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
if (beginDir.v == llvm::omp::Directive::OMPD_ordered) {
const auto &clauses{
std::get<parser::OmpClauseList>(beginBlockDir.t)};
for (const auto &clause : clauses.v) {
if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
eligibleSIMD = true;
break;
}
}
}
},
[&](const parser::OpenMPSimpleStandaloneConstruct &c) {
const auto &dir{
std::get<parser::OmpSimpleStandaloneDirective>(c.t)};
if (dir.v == llvm::omp::Directive::OMPD_ordered) {
const auto &clauses{
std::get<parser::OmpClauseList>(c.t)};
for (const auto &clause : clauses.v) {
if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
eligibleSIMD = true;
break;
}
}
}
},
// Allowing SIMD construct
[&](const parser::OpenMPLoopConstruct &c) {
const auto &beginLoopDir{
std::get<parser::OmpBeginLoopDirective>(c.t)};
const auto &beginDir{
std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
if ((beginDir.v == llvm::omp::Directive::OMPD_simd) ||
(beginDir.v == llvm::omp::Directive::OMPD_do_simd)) {
eligibleSIMD = true;
}
},
[&](const parser::OpenMPAtomicConstruct &c) {
// Allow `!$OMP ATOMIC`
eligibleSIMD = true;
},
[&](const auto &c) {},
},
common::visit(
Fortran::common::visitors{
// Allow `!$OMP ORDERED SIMD`
[&](const parser::OpenMPBlockConstruct &c) {
const auto &beginBlockDir{
std::get<parser::OmpBeginBlockDirective>(c.t)};
const auto &beginDir{
std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
if (beginDir.v == llvm::omp::Directive::OMPD_ordered) {
const auto &clauses{
std::get<parser::OmpClauseList>(beginBlockDir.t)};
for (const auto &clause : clauses.v) {
if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
eligibleSIMD = true;
Comment on lines +751 to +764
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This an the lines below are pure formatting changes. Can you please do those after the review?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Replied here. I am unable to remove them at the moment. Please let me know your suggestion.

break;
}
}
}
},
[&](const parser::OpenMPStandaloneConstruct &c) {
if (const auto &simpleConstruct =
std::get_if<parser::OpenMPSimpleStandaloneConstruct>(
&c.u)) {
const auto &dir{std::get<parser::OmpSimpleStandaloneDirective>(
simpleConstruct->t)};
if (dir.v == llvm::omp::Directive::OMPD_ordered) {
const auto &clauses{
std::get<parser::OmpClauseList>(simpleConstruct->t)};
for (const auto &clause : clauses.v) {
if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
eligibleSIMD = true;
break;
}
}
} else if (dir.v == llvm::omp::Directive::OMPD_scan) {
eligibleSIMD = true;
}
}
},
// Allowing SIMD construct
[&](const parser::OpenMPLoopConstruct &c) {
const auto &beginLoopDir{
std::get<parser::OmpBeginLoopDirective>(c.t)};
const auto &beginDir{
std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
if ((beginDir.v == llvm::omp::Directive::OMPD_simd) ||
(beginDir.v == llvm::omp::Directive::OMPD_do_simd)) {
eligibleSIMD = true;
}
},
[&](const parser::OpenMPAtomicConstruct &c) {
// Allow `!$OMP ATOMIC`
eligibleSIMD = true;
},
[&](const auto &c) {},
},
c.u);
if (!eligibleSIMD) {
context_.Say(parser::FindSourceLocation(c),
"The only OpenMP constructs that can be encountered during execution "
"of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, "
"the `SIMD` construct and the `ORDERED` construct with the `SIMD` "
"clause."_err_en_US);
"the `SIMD` construct, the `SCAN` construct and the `ORDERED` "
"construct with the `SIMD` clause."_err_en_US);
}
}

Expand Down Expand Up @@ -966,6 +974,49 @@ void OmpStructureChecker::CheckDistLinear(
}

void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
const auto &clauseList{std::get<parser::OmpClauseList>(beginLoopDir.t)};

// A few semantic checks for InScan reduction are performed below as SCAN
// constructs inside LOOP may add the relevant information. Scan reduction is
// supported only in loop constructs, so same checks are not applicable to
// other directives.
for (const auto &clause : clauseList.v) {
if (const auto *reductionClause{
std::get_if<parser::OmpClause::Reduction>(&clause.u)}) {
const auto &maybeModifier{
std::get<std::optional<ReductionModifier>>(reductionClause->v.t)};
if (maybeModifier && *maybeModifier == ReductionModifier::Inscan) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Reduce the nesting level to make code easier to follow.

Suggested change
if (maybeModifier && *maybeModifier == ReductionModifier::Inscan) {
if (!maybeModifier || *maybeModifier != ReductionModifier::Inscan)
continue;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally frontend code in flang is more nested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to go with the existing approach as personally I feel it more readable. Please let me know if there are suggestions otherwise.

const auto &objectList{
std::get<parser::OmpObjectList>(reductionClause->v.t)};
auto checkReductionSymbolInScan = [&](const parser::Name *name) {
if (auto &symbol = name->symbol) {
if (!symbol->test(Symbol::Flag::OmpInclusiveScan) &&
!symbol->test(Symbol::Flag::OmpExclusiveScan)) {
context_.Say(name->source,
"List item %s must appear in EXCLUSIVE or "
"INCLUSIVE clause of an "
"enclosed SCAN directive"_err_en_US,
name->ToString());
}
}
};
for (const auto &ompObj : objectList.v) {
common::visit(
common::visitors{
[&](const parser::Designator &designator) {
if (const auto *name{semantics::getDesignatorNameIfDataRef(
designator)}) {
checkReductionSymbolInScan(name);
}
},
[&](const auto &name) { checkReductionSymbolInScan(&name); },
},
ompObj.u);
}
}
}
}
if (llvm::omp::allSimdSet.test(GetContext().directive)) {
ExitDirectiveNest(SIMDNest);
}
Expand Down Expand Up @@ -1638,19 +1689,32 @@ void OmpStructureChecker::Leave(const parser::OpenMPAllocatorsConstruct &x) {
dirContext_.pop_back();
}

void OmpStructureChecker::CheckScan(
const parser::OpenMPSimpleStandaloneConstruct &x) {
if (std::get<parser::OmpClauseList>(x.t).v.size() != 1) {
context_.Say(x.source,
"Exactly one of EXCLUSIVE or INCLUSIVE clause is expected"_err_en_US);
}
if (!CurrentDirectiveIsNested() ||
!llvm::omp::scanParentAllowedSet.test(GetContextParent().directive)) {
context_.Say(x.source,
"Orphaned SCAN directives are prohibited; perhaps you forgot "
"to enclose the directive in to a WORKSHARING LOOP, a WORKSHARING "
"LOOP SIMD or a SIMD directive."_err_en_US);
}
}

void OmpStructureChecker::CheckBarrierNesting(
const parser::OpenMPSimpleStandaloneConstruct &x) {
// A barrier region may not be `closely nested` inside a worksharing, loop,
// task, taskloop, critical, ordered, atomic, or master region.
// TODO: Expand the check to include `LOOP` construct as well when it is
// supported.
if (GetContext().directive == llvm::omp::Directive::OMPD_barrier) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this check for barrier not required?

Copy link
Contributor Author

@anchuraj anchuraj Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the check to the caller (@line 1906) since I needed a function for scan as well.

if (IsCloselyNestedRegion(llvm::omp::nestedBarrierErrSet)) {
context_.Say(parser::FindSourceLocation(x),
"`BARRIER` region may not be closely nested inside of `WORKSHARING`, "
"`LOOP`, `TASK`, `TASKLOOP`,"
"`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region."_err_en_US);
}
if (IsCloselyNestedRegion(llvm::omp::nestedBarrierErrSet)) {
context_.Say(parser::FindSourceLocation(x),
"`BARRIER` region may not be closely nested inside of `WORKSHARING`, "
"`LOOP`, `TASK`, `TASKLOOP`,"
"`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region."_err_en_US);
}
}

Expand Down Expand Up @@ -1834,7 +1898,16 @@ void OmpStructureChecker::Enter(
const parser::OpenMPSimpleStandaloneConstruct &x) {
const auto &dir{std::get<parser::OmpSimpleStandaloneDirective>(x.t)};
PushContextAndClauseSets(dir.source, dir.v);
CheckBarrierNesting(x);
switch (dir.v) {
case llvm::omp::Directive::OMPD_barrier:
CheckBarrierNesting(x);
break;
case llvm::omp::Directive::OMPD_scan:
CheckScan(x);
break;
default:
break;
}
}

void OmpStructureChecker::Leave(
Expand Down Expand Up @@ -2673,8 +2746,8 @@ CHECK_SIMPLE_CLAUSE(Full, OMPC_full)
CHECK_SIMPLE_CLAUSE(Grainsize, OMPC_grainsize)
CHECK_SIMPLE_CLAUSE(Hint, OMPC_hint)
CHECK_SIMPLE_CLAUSE(Holds, OMPC_holds)
CHECK_SIMPLE_CLAUSE(InReduction, OMPC_in_reduction)
CHECK_SIMPLE_CLAUSE(Inclusive, OMPC_inclusive)
CHECK_SIMPLE_CLAUSE(InReduction, OMPC_in_reduction)
CHECK_SIMPLE_CLAUSE(Match, OMPC_match)
CHECK_SIMPLE_CLAUSE(Nontemporal, OMPC_nontemporal)
CHECK_SIMPLE_CLAUSE(NumTasks, OMPC_num_tasks)
Expand Down Expand Up @@ -2767,7 +2840,11 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
if (CheckReductionOperators(x)) {
CheckReductionTypeList(x);
}
CheckReductionModifier(x);
if (const auto &maybeModifier{
std::get<std::optional<ReductionModifier>>(x.v.t)}) {
const ReductionModifier modifier{*maybeModifier};
CheckReductionModifier(modifier);
}
}

bool OmpStructureChecker::CheckReductionOperators(
Expand Down Expand Up @@ -2810,6 +2887,7 @@ bool OmpStructureChecker::CheckReductionOperators(

return ok;
}

bool OmpStructureChecker::CheckIntrinsicOperator(
const parser::DefinedOperator::IntrinsicOperator &op) {

Expand Down Expand Up @@ -2944,14 +3022,11 @@ void OmpStructureChecker::CheckReductionTypeList(
}

void OmpStructureChecker::CheckReductionModifier(
const parser::OmpClause::Reduction &x) {
using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
const auto &maybeModifier{std::get<std::optional<ReductionModifier>>(x.v.t)};
if (!maybeModifier || *maybeModifier == ReductionModifier::Default) {
// No modifier, or the default one is always ok.
const ReductionModifier &modifier) {
if (modifier == ReductionModifier::Default) {
// The default one is always ok.
return;
}
ReductionModifier modifier{*maybeModifier};
const DirectiveContext &dirCtx{GetContext()};
if (dirCtx.directive == llvm::omp::Directive::OMPD_loop) {
// [5.2:257:33-34]
Expand Down Expand Up @@ -2982,15 +3057,10 @@ void OmpStructureChecker::CheckReductionModifier(
// or "simd" directive.
// The worksharing-loop directives are OMPD_do and OMPD_for. Only the
// former is allowed in Fortran.
switch (dirCtx.directive) {
case llvm::omp::Directive::OMPD_do: // worksharing-loop
case llvm::omp::Directive::OMPD_do_simd: // worksharing-loop simd
case llvm::omp::Directive::OMPD_simd: // "simd"
break;
default:
if (!llvm::omp::scanParentAllowedSet.test(dirCtx.directive)) {
context_.Say(GetContext().clauseSource,
"Modifier 'INSCAN' on REDUCTION clause is only allowed with "
"worksharing-loop, worksharing-loop simd, "
"WORKSHARING LOOP, WORKSHARING LOOP SIMD, "
"or SIMD directive"_err_en_US);
}
} else {
Expand Down
4 changes: 3 additions & 1 deletion flang/lib/Semantics/check-omp-structure.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class OmpStructureChecker
) {
}
using llvmOmpClause = const llvm::omp::Clause;
using ReductionModifier = parser::OmpReductionClause::ReductionModifier;

void Enter(const parser::OpenMPConstruct &);
void Leave(const parser::OpenMPConstruct &);
Expand Down Expand Up @@ -227,10 +228,11 @@ class OmpStructureChecker
bool CheckIntrinsicOperator(
const parser::DefinedOperator::IntrinsicOperator &);
void CheckReductionTypeList(const parser::OmpClause::Reduction &);
void CheckReductionModifier(const parser::OmpClause::Reduction &);
void CheckReductionModifier(const ReductionModifier &);
void CheckMasterNesting(const parser::OpenMPBlockConstruct &x);
void ChecksOnOrderedAsBlock();
void CheckBarrierNesting(const parser::OpenMPSimpleStandaloneConstruct &x);
void CheckScan(const parser::OpenMPSimpleStandaloneConstruct &x);
void ChecksOnOrderedAsStandalone();
void CheckOrderedDependClause(std::optional<std::int64_t> orderedValue);
void CheckReductionArraySection(const parser::OmpObjectList &ompObjectList);
Expand Down
Loading