Skip to content

Commit 1c4c9e8

Browse files
NimishMishraNimishMishra
committed
[flang][OpenMP] Added semantic checks for atomic capture, write, and update statements
This patch adds general checks for atomic read, write, and capture statements. -check "capture statement is of the form v = x if atomic construct is read" -check "write statement is of the form x = expr if atomic construct is write" -check "x must not have the ALLOCATABLE attribute." -check for non-scalar variables -check if x (LHS variable) is accessed on the RHS of assignment statement -improve error reporting in atomic update statemen Reviewed By: TIFitis, raghavendhra Differential Revision: https://reviews.llvm.org/D127620
1 parent d57fe1d commit 1c4c9e8

File tree

6 files changed

+270
-49
lines changed

6 files changed

+270
-49
lines changed

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

Lines changed: 133 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,6 +1647,67 @@ void OmpStructureChecker::Leave(const parser::OmpEndBlockDirective &x) {
16471647
}
16481648
}
16491649

1650+
inline void OmpStructureChecker::ErrIfAllocatableVariable(
1651+
const parser::Variable &var) {
1652+
// Err out if the given symbol has
1653+
// ALLOCATABLE attribute
1654+
if (const auto *e{GetExpr(context_, var)})
1655+
for (const Symbol &symbol : evaluate::CollectSymbols(*e))
1656+
if (IsAllocatable(symbol)) {
1657+
const auto &designator =
1658+
std::get<common::Indirection<parser::Designator>>(var.u);
1659+
const auto *dataRef =
1660+
std::get_if<Fortran::parser::DataRef>(&designator.value().u);
1661+
const Fortran::parser::Name *name =
1662+
dataRef ? std::get_if<Fortran::parser::Name>(&dataRef->u) : nullptr;
1663+
if (name)
1664+
context_.Say(name->source,
1665+
"%s must not have ALLOCATABLE "
1666+
"attribute"_err_en_US,
1667+
name->ToString());
1668+
}
1669+
}
1670+
1671+
inline void OmpStructureChecker::ErrIfLHSAndRHSSymbolsMatch(
1672+
const parser::Variable &var, const parser::Expr &expr) {
1673+
// Err out if the symbol on the LHS is also used on the RHS of the assignment
1674+
// statement
1675+
const auto *e{GetExpr(context_, expr)};
1676+
const auto *v{GetExpr(context_, var)};
1677+
if (e && v) {
1678+
const Symbol &varSymbol = evaluate::GetSymbolVector(*v).front();
1679+
for (const Symbol &symbol : evaluate::GetSymbolVector(*e)) {
1680+
if (varSymbol == symbol) {
1681+
context_.Say(expr.source,
1682+
"RHS expression "
1683+
"on atomic assignment statement"
1684+
" cannot access '%s'"_err_en_US,
1685+
var.GetSource().ToString());
1686+
}
1687+
}
1688+
}
1689+
}
1690+
1691+
inline void OmpStructureChecker::ErrIfNonScalarAssignmentStmt(
1692+
const parser::Variable &var, const parser::Expr &expr) {
1693+
// Err out if either the variable on the LHS or the expression on the RHS of
1694+
// the assignment statement are non-scalar (i.e. have rank > 0)
1695+
const auto *e{GetExpr(context_, expr)};
1696+
const auto *v{GetExpr(context_, var)};
1697+
if (e && v) {
1698+
if (e->Rank() != 0)
1699+
context_.Say(expr.source,
1700+
"Expected scalar expression "
1701+
"on the RHS of atomic assignment "
1702+
"statement"_err_en_US);
1703+
if (v->Rank() != 0)
1704+
context_.Say(var.GetSource(),
1705+
"Expected scalar variable "
1706+
"on the LHS of atomic assignment "
1707+
"statement"_err_en_US);
1708+
}
1709+
}
1710+
16501711
template <typename T, typename D>
16511712
bool OmpStructureChecker::IsOperatorValid(const T &node, const D &variable) {
16521713
using AllowedBinaryOperators =
@@ -1667,16 +1728,55 @@ bool OmpStructureChecker::IsOperatorValid(const T &node, const D &variable) {
16671728
if ((exprLeft.value().source.ToString() != variableName) &&
16681729
(exprRight.value().source.ToString() != variableName)) {
16691730
context_.Say(variable.GetSource(),
1670-
"Atomic update variable '%s' not found in the RHS of the "
1671-
"assignment statement in an ATOMIC (UPDATE) construct"_err_en_US,
1672-
variableName);
1731+
"Atomic update statement should be of form "
1732+
"`%s = %s operator expr` OR `%s = expr operator %s`"_err_en_US,
1733+
variableName, variableName, variableName, variableName);
16731734
}
16741735
return common::HasMember<T, AllowedBinaryOperators>;
16751736
}
16761737
return true;
16771738
}
16781739

1679-
void OmpStructureChecker::CheckAtomicUpdateAssignmentStmt(
1740+
void OmpStructureChecker::CheckAtomicCaptureStmt(
1741+
const parser::AssignmentStmt &assignmentStmt) {
1742+
const auto &var{std::get<parser::Variable>(assignmentStmt.t)};
1743+
const auto &expr{std::get<parser::Expr>(assignmentStmt.t)};
1744+
common::visit(
1745+
common::visitors{
1746+
[&](const common::Indirection<parser::Designator> &designator) {
1747+
const auto *dataRef =
1748+
std::get_if<Fortran::parser::DataRef>(&designator.value().u);
1749+
const auto *name = dataRef
1750+
? std::get_if<Fortran::parser::Name>(&dataRef->u)
1751+
: nullptr;
1752+
if (name && IsAllocatable(*name->symbol))
1753+
context_.Say(name->source,
1754+
"%s must not have ALLOCATABLE "
1755+
"attribute"_err_en_US,
1756+
name->ToString());
1757+
},
1758+
[&](const auto &) {
1759+
// Anything other than a `parser::Designator` is not allowed
1760+
context_.Say(expr.source,
1761+
"Expected scalar variable "
1762+
"of intrinsic type on RHS of atomic "
1763+
"assignment statement"_err_en_US);
1764+
}},
1765+
expr.u);
1766+
ErrIfLHSAndRHSSymbolsMatch(var, expr);
1767+
ErrIfNonScalarAssignmentStmt(var, expr);
1768+
}
1769+
1770+
void OmpStructureChecker::CheckAtomicWriteStmt(
1771+
const parser::AssignmentStmt &assignmentStmt) {
1772+
const auto &var{std::get<parser::Variable>(assignmentStmt.t)};
1773+
const auto &expr{std::get<parser::Expr>(assignmentStmt.t)};
1774+
ErrIfAllocatableVariable(var);
1775+
ErrIfLHSAndRHSSymbolsMatch(var, expr);
1776+
ErrIfNonScalarAssignmentStmt(var, expr);
1777+
}
1778+
1779+
void OmpStructureChecker::CheckAtomicUpdateStmt(
16801780
const parser::AssignmentStmt &assignment) {
16811781
const auto &expr{std::get<parser::Expr>(assignment.t)};
16821782
const auto &var{std::get<parser::Variable>(assignment.t)};
@@ -1734,6 +1834,7 @@ void OmpStructureChecker::CheckAtomicUpdateAssignmentStmt(
17341834
},
17351835
},
17361836
expr.u);
1837+
ErrIfAllocatableVariable(var);
17371838
}
17381839

17391840
void OmpStructureChecker::CheckAtomicMemoryOrderClause(
@@ -1772,7 +1873,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
17721873
const auto &dir{std::get<parser::Verbatim>(atomicConstruct.t)};
17731874
PushContextAndClauseSets(
17741875
dir.source, llvm::omp::Directive::OMPD_atomic);
1775-
CheckAtomicUpdateAssignmentStmt(
1876+
CheckAtomicUpdateStmt(
17761877
std::get<parser::Statement<parser::AssignmentStmt>>(
17771878
atomicConstruct.t)
17781879
.statement);
@@ -1787,7 +1888,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
17871888
const auto &dir{std::get<parser::Verbatim>(atomicUpdate.t)};
17881889
PushContextAndClauseSets(
17891890
dir.source, llvm::omp::Directive::OMPD_atomic);
1790-
CheckAtomicUpdateAssignmentStmt(
1891+
CheckAtomicUpdateStmt(
17911892
std::get<parser::Statement<parser::AssignmentStmt>>(
17921893
atomicUpdate.t)
17931894
.statement);
@@ -1796,6 +1897,32 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
17961897
CheckHintClause<const parser::OmpAtomicClauseList>(
17971898
&std::get<0>(atomicUpdate.t), &std::get<2>(atomicUpdate.t));
17981899
},
1900+
[&](const parser::OmpAtomicRead &atomicRead) {
1901+
const auto &dir{std::get<parser::Verbatim>(atomicRead.t)};
1902+
PushContextAndClauseSets(
1903+
dir.source, llvm::omp::Directive::OMPD_atomic);
1904+
CheckAtomicMemoryOrderClause(
1905+
&std::get<0>(atomicRead.t), &std::get<2>(atomicRead.t));
1906+
CheckHintClause<const parser::OmpAtomicClauseList>(
1907+
&std::get<0>(atomicRead.t), &std::get<2>(atomicRead.t));
1908+
CheckAtomicCaptureStmt(
1909+
std::get<parser::Statement<parser::AssignmentStmt>>(
1910+
atomicRead.t)
1911+
.statement);
1912+
},
1913+
[&](const parser::OmpAtomicWrite &atomicWrite) {
1914+
const auto &dir{std::get<parser::Verbatim>(atomicWrite.t)};
1915+
PushContextAndClauseSets(
1916+
dir.source, llvm::omp::Directive::OMPD_atomic);
1917+
CheckAtomicMemoryOrderClause(
1918+
&std::get<0>(atomicWrite.t), &std::get<2>(atomicWrite.t));
1919+
CheckHintClause<const parser::OmpAtomicClauseList>(
1920+
&std::get<0>(atomicWrite.t), &std::get<2>(atomicWrite.t));
1921+
CheckAtomicWriteStmt(
1922+
std::get<parser::Statement<parser::AssignmentStmt>>(
1923+
atomicWrite.t)
1924+
.statement);
1925+
},
17991926
[&](const auto &atomicConstruct) {
18001927
const auto &dir{std::get<parser::Verbatim>(atomicConstruct.t)};
18011928
PushContextAndClauseSets(

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,9 @@ class OmpStructureChecker
175175
template <typename T, typename D> bool IsOperatorValid(const T &, const D &);
176176
void CheckAtomicMemoryOrderClause(
177177
const parser::OmpAtomicClauseList *, const parser::OmpAtomicClauseList *);
178-
void CheckAtomicUpdateAssignmentStmt(const parser::AssignmentStmt &);
178+
void CheckAtomicUpdateStmt(const parser::AssignmentStmt &);
179+
void CheckAtomicCaptureStmt(const parser::AssignmentStmt &);
180+
void CheckAtomicWriteStmt(const parser::AssignmentStmt &);
179181
void CheckAtomicConstructStructure(const parser::OpenMPAtomicConstruct &);
180182
void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
181183
void CheckSIMDNest(const parser::OpenMPConstruct &x);
@@ -214,7 +216,11 @@ class OmpStructureChecker
214216
void ExitDirectiveNest(const int index) { directiveNest_[index]--; }
215217
int GetDirectiveNest(const int index) { return directiveNest_[index]; }
216218
template <typename D> void CheckHintClause(D *, D *);
217-
219+
inline void ErrIfAllocatableVariable(const parser::Variable &);
220+
inline void ErrIfLHSAndRHSSymbolsMatch(
221+
const parser::Variable &, const parser::Expr &);
222+
inline void ErrIfNonScalarAssignmentStmt(
223+
const parser::Variable &, const parser::Expr &);
218224
enum directiveNestType {
219225
SIMDNest,
220226
TargetBlockOnlyTeams,

‎flang/test/Semantics/OpenMP/atomic-hint-clause.f90‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ program sample
9595
x = 10 * y
9696

9797
!$omp atomic write hint(a)
98+
!ERROR: RHS expression on atomic assignment statement cannot access 'x'
9899
x = y + x
99100

100101
!$omp atomic hint(abs(-1)) write

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,27 +32,27 @@ program OmpAtomic
3232
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
3333
c = c//d
3434
!$omp atomic
35-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
35+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
3636
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
3737
l = a .LT. b
3838
!$omp atomic
39-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
39+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
4040
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
4141
l = a .LE. b
4242
!$omp atomic
43-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
43+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
4444
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
4545
l = a .EQ. b
4646
!$omp atomic
47-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
47+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
4848
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
4949
l = a .NE. b
5050
!$omp atomic
51-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
51+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
5252
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
5353
l = a .GE. b
5454
!$omp atomic
55-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
55+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
5656
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
5757
l = a .GT. b
5858
!$omp atomic
@@ -78,23 +78,23 @@ program OmpAtomic
7878
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
7979
c = c//d
8080
!$omp atomic update
81-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
81+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
8282
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
8383
l = a .LT. b
8484
!$omp atomic update
85-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
85+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
8686
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
8787
l = a .LE. b
8888
!$omp atomic update
89-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
89+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
9090
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
9191
l = a .EQ. b
9292
!$omp atomic update
93-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
93+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
9494
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
9595
l = a .GE. b
9696
!$omp atomic update
97-
!ERROR: Atomic update variable 'l' not found in the RHS of the assignment statement in an ATOMIC (UPDATE) construct
97+
!ERROR: Atomic update statement should be of form `l = l operator expr` OR `l = expr operator l`
9898
!ERROR: Invalid operator in OpenMP ATOMIC (UPDATE) statement
9999
l = a .GT. b
100100
!$omp atomic update

0 commit comments

Comments
 (0)