Skip to content

Commit

Permalink
[Flang][OpenMP][OpenACC] Fix exit out of a region in OpenMP parallel …
Browse files Browse the repository at this point in the history 
…construct.

From below mentioned standard references
OpenACC 3.0 Standards document
840 • A program may not branch into or out of an OpenACC parallel construct

OpenMP 5.0 Standards document
A program that branches into or out of a parallel region is non-conforming.

This patch
Resolves the issue of exit out of a parallel region, other branching out issues like goto statements are not handled with this patch.
Moves code from D87906 to be reused by other OpenMP/OpenACC to check-directive-structure.h.
Adds support in OpenMP parallel construct and a test case to verify.

Reviewed By: clementval

Differential Revision: https://reviews.llvm.org/D88655
  • Loading branch information
@Sameeranjoshi
Sameeranjoshi committed Oct 30, 2020
1 parent edd6ed3 commit 61f11f8
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 89 deletions.
84 changes: 0 additions & 84 deletions flang/lib/Semantics/check-acc-structure.cpp
View file
Expand Up @@ -44,83 +44,6 @@ static constexpr inline AccClauseSet routineOnlyAllowedAfterDeviceTypeClauses{
llvm::acc::Clause::ACCC_bind, llvm::acc::Clause::ACCC_gang,
llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_worker};

class NoBranchingEnforce {
public:
NoBranchingEnforce(SemanticsContext &context,
parser::CharBlock sourcePosition, llvm::acc::Directive directive)
: context_{context}, sourcePosition_{sourcePosition}, currentDirective_{
directive} {}
template <typename T> bool Pre(const T &) { return true; }
template <typename T> void Post(const T &) {}

template <typename T> bool Pre(const parser::Statement<T> &statement) {
currentStatementSourcePosition_ = statement.source;
return true;
}

void Post(const parser::ReturnStmt &) { EmitBranchOutError("RETURN"); }
void Post(const parser::ExitStmt &exitStmt) {
if (const auto &exitName{exitStmt.v}) {
CheckConstructNameBranching("EXIT", exitName.value());
}
}
void Post(const parser::StopStmt &) { EmitBranchOutError("STOP"); }

private:
parser::MessageFormattedText GetEnclosingMsg() const {
return {"Enclosing %s construct"_en_US,
parser::ToUpperCaseLetters(
llvm::acc::getOpenACCDirectiveName(currentDirective_).str())};
}

void EmitBranchOutError(const char *stmt) const {
context_
.Say(currentStatementSourcePosition_,
"%s statement is not allowed in a %s construct"_err_en_US, stmt,
parser::ToUpperCaseLetters(
llvm::acc::getOpenACCDirectiveName(currentDirective_).str()))
.Attach(sourcePosition_, GetEnclosingMsg());
}

void EmitBranchOutErrorWithName(
const char *stmt, const parser::Name &toName) const {
const std::string branchingToName{toName.ToString()};
const auto upperCaseConstructName{parser::ToUpperCaseLetters(
llvm::acc::getOpenACCDirectiveName(currentDirective_).str())};
context_
.Say(currentStatementSourcePosition_,
"%s to construct '%s' outside of %s construct is not allowed"_err_en_US,
stmt, branchingToName, upperCaseConstructName)
.Attach(sourcePosition_, GetEnclosingMsg());
}

// Current semantic checker is not following OpenACC constructs as they are
// not Fortran constructs. Hence the ConstructStack doesn't capture OpenACC
// constructs. Apply an inverse way to figure out if a construct-name is
// branching out of an OpenACC construct. The control flow goes out of an
// OpenACC construct, if a construct-name from statement is found in
// ConstructStack.
void CheckConstructNameBranching(
const char *stmt, const parser::Name &stmtName) {
const ConstructStack &stack{context_.constructStack()};
for (auto iter{stack.cend()}; iter-- != stack.cbegin();) {
const ConstructNode &construct{*iter};
const auto &constructName{MaybeGetNodeName(construct)};
if (constructName) {
if (stmtName.source == constructName->source) {
EmitBranchOutErrorWithName(stmt, stmtName);
return;
}
}
}
}

SemanticsContext &context_;
parser::CharBlock currentStatementSourcePosition_;
parser::CharBlock sourcePosition_;
llvm::acc::Directive currentDirective_;
};

bool AccStructureChecker::CheckAllowedModifier(llvm::acc::Clause clause) {
if (GetContext().directive == llvm::acc::ACCD_enter_data ||
GetContext().directive == llvm::acc::ACCD_exit_data) {
Expand Down Expand Up @@ -186,13 +109,6 @@ void AccStructureChecker::Leave(const parser::OpenACCBlockConstruct &x) {
dirContext_.pop_back();
}

void AccStructureChecker::CheckNoBranching(const parser::Block &block,
const llvm::acc::Directive directive,
const parser::CharBlock &directiveSource) const {
NoBranchingEnforce noBranchingEnforce{context_, directiveSource, directive};
parser::Walk(block, noBranchingEnforce);
}

void AccStructureChecker::Enter(
const parser::OpenACCStandaloneDeclarativeConstruct &x) {
const auto &declarativeDir{std::get<parser::AccDeclarativeDirective>(x.t)};
Expand Down
5 changes: 0 additions & 5 deletions flang/lib/Semantics/check-acc-structure.h
View file
Expand Up @@ -111,12 +111,7 @@ class AccStructureChecker

private:

void CheckNoBranching(const parser::Block &block,
const llvm::acc::Directive directive,
const parser::CharBlock &directiveSource) const;

bool CheckAllowedModifier(llvm::acc::Clause clause);

llvm::StringRef getClauseName(llvm::acc::Clause clause) override;
llvm::StringRef getDirectiveName(llvm::acc::Directive directive) override;
};
Expand Down
89 changes: 89 additions & 0 deletions flang/lib/Semantics/check-directive-structure.h
View file
Expand Up @@ -27,6 +27,84 @@ template <typename C, std::size_t ClauseEnumSize> struct DirectiveClauses {
const common::EnumSet<C, ClauseEnumSize> requiredOneOf;
};

// Generic branching checker for invalid branching out of OpenMP/OpenACC
// directive.
// typename D is the directive enumeration.
template <typename D> class NoBranchingEnforce {
public:
NoBranchingEnforce(SemanticsContext &context,
parser::CharBlock sourcePosition, D directive,
std::string &&upperCaseDirName)
: context_{context}, sourcePosition_{sourcePosition},
currentDirective_{directive}, upperCaseDirName_{
std::move(upperCaseDirName)} {}
template <typename T> bool Pre(const T &) { return true; }
template <typename T> void Post(const T &) {}

template <typename T> bool Pre(const parser::Statement<T> &statement) {
currentStatementSourcePosition_ = statement.source;
return true;
}

void Post(const parser::ReturnStmt &) { EmitBranchOutError("RETURN"); }
void Post(const parser::ExitStmt &exitStmt) {
if (const auto &exitName{exitStmt.v}) {
CheckConstructNameBranching("EXIT", exitName.value());
}
}
void Post(const parser::StopStmt &) { EmitBranchOutError("STOP"); }

private:
parser::MessageFormattedText GetEnclosingMsg() const {
return {"Enclosing %s construct"_en_US, upperCaseDirName_};
}

void EmitBranchOutError(const char *stmt) const {
context_
.Say(currentStatementSourcePosition_,
"%s statement is not allowed in a %s construct"_err_en_US, stmt,
upperCaseDirName_)
.Attach(sourcePosition_, GetEnclosingMsg());
}

void EmitBranchOutErrorWithName(
const char *stmt, const parser::Name &toName) const {
const std::string branchingToName{toName.ToString()};
context_
.Say(currentStatementSourcePosition_,
"%s to construct '%s' outside of %s construct is not allowed"_err_en_US,
stmt, branchingToName, upperCaseDirName_)
.Attach(sourcePosition_, GetEnclosingMsg());
}

// Current semantic checker is not following OpenACC/OpenMP constructs as they
// are not Fortran constructs. Hence the ConstructStack doesn't capture
// OpenACC/OpenMP constructs. Apply an inverse way to figure out if a
// construct-name is branching out of an OpenACC/OpenMP construct. The control
// flow goes out of an OpenACC/OpenMP construct, if a construct-name from
// statement is found in ConstructStack.
void CheckConstructNameBranching(
const char *stmt, const parser::Name &stmtName) {
const ConstructStack &stack{context_.constructStack()};
for (auto iter{stack.cend()}; iter-- != stack.cbegin();) {
const ConstructNode &construct{*iter};
const auto &constructName{MaybeGetNodeName(construct)};
if (constructName) {
if (stmtName.source == constructName->source) {
EmitBranchOutErrorWithName(stmt, stmtName);
return;
}
}
}
}

SemanticsContext &context_;
parser::CharBlock currentStatementSourcePosition_;
parser::CharBlock sourcePosition_;
std::string upperCaseDirName_;
D currentDirective_;
};

// Generic structure checker for directives/clauses language such as OpenMP
// and OpenACC.
// typename D is the directive enumeration.
Expand Down Expand Up @@ -148,6 +226,8 @@ class DirectiveStructureChecker : public virtual BaseChecker {
SayNotMatching(beginDir.source, endDir.source);
}
}
void CheckNoBranching(const parser::Block &block, D directive,
const parser::CharBlock &directiveSource);

// Check that only clauses in set are after the specific clauses.
void CheckOnlyAllowedAfter(C clause, common::EnumSet<C, ClauseEnumSize> set);
Expand Down Expand Up @@ -186,6 +266,15 @@ class DirectiveStructureChecker : public virtual BaseChecker {
std::string ClauseSetToString(const common::EnumSet<C, ClauseEnumSize> set);
};

template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
void DirectiveStructureChecker<D, C, PC, ClauseEnumSize>::CheckNoBranching(
const parser::Block &block, D directive,
const parser::CharBlock &directiveSource) {
NoBranchingEnforce<D> noBranchingEnforce{
context_, directiveSource, directive, ContextDirectiveAsFortran()};
parser::Walk(block, noBranchingEnforce);
}

// Check that only clauses included in the given set are present after the given
// clause.
template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
Expand Down
9 changes: 9 additions & 0 deletions flang/lib/Semantics/check-omp-structure.cpp
View file
Expand Up @@ -95,9 +95,18 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
const auto &endBlockDir{std::get<parser::OmpEndBlockDirective>(x.t)};
const auto &beginDir{std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
const auto &endDir{std::get<parser::OmpBlockDirective>(endBlockDir.t)};
const parser::Block &block{std::get<parser::Block>(x.t)};

CheckMatching<parser::OmpBlockDirective>(beginDir, endDir);

PushContextAndClauseSets(beginDir.source, beginDir.v);

switch (beginDir.v) {
case llvm::omp::OMPD_parallel:
CheckNoBranching(block, llvm::omp::OMPD_parallel, beginDir.source);
default:
break;
}
}

void OmpStructureChecker::Leave(const parser::OpenMPBlockConstruct &) {
Expand Down
16 changes: 16 additions & 0 deletions flang/test/Semantics/omp-clause-validity01.f90
View file
Expand Up @@ -163,6 +163,22 @@
!ERROR: Unmatched END TARGET directive
!$omp end target

! OMP 5.0 - 2.6 Restriction point 1
outofparallel: do k =1, 10
!$omp parallel
!$omp do
outer: do i=0, 10
inner: do j=1, 10
exit
exit outer
!ERROR: EXIT to construct 'outofparallel' outside of PARALLEL construct is not allowed
exit outofparallel
end do inner
end do outer
!$end omp do
!$omp end parallel
end do outofparallel

! 2.7.1 do-clause -> private-clause |
! firstprivate-clause |
! lastprivate-clause |
Expand Down

0 comments on commit 61f11f8

Please sign in to comment.