Skip to content

Commit 81dac7d

Browse files
harishch4kiranchandramohan
harishch4
and
kiranchandramohan
authored
[Flang][OpenMP] Add Semantic Checks for Atomic Capture Construct (#108516)
This PR adds semantic checks to ensure the atomic capture construct conforms to one of the valid forms: [capture-stmt, update-stmt], [capture-stmt, write-stmt] or [update-stmt, capture-stmt]. Functions checkForSymbolMatch and checkForSingleVariableOnRHS are moved from flang/lib/Lower/DirectivesCommon.h to flang/Semantics/tools.h for reuse. --------- Co-authored-by: Kiran Chandramohan <[email protected]>
1 parent d50eaac commit 81dac7d

File tree

7 files changed

+161
-43
lines changed

7 files changed

+161
-43
lines changed

‎flang/include/flang/Semantics/tools.h

+29
Original file line numberDiff line numberDiff line change
@@ -736,5 +736,34 @@ std::string GetCommonBlockObjectName(const Symbol &, bool underscoring);
736736
// Check for ambiguous USE associations
737737
bool HadUseError(SemanticsContext &, SourceName at, const Symbol *);
738738

739+
/// Checks if the assignment statement has a single variable on the RHS.
740+
inline bool checkForSingleVariableOnRHS(
741+
const Fortran::parser::AssignmentStmt &assignmentStmt) {
742+
const Fortran::parser::Expr &expr{
743+
std::get<Fortran::parser::Expr>(assignmentStmt.t)};
744+
const Fortran::common::Indirection<Fortran::parser::Designator> *designator =
745+
std::get_if<Fortran::common::Indirection<Fortran::parser::Designator>>(
746+
&expr.u);
747+
return designator != nullptr;
748+
}
749+
750+
/// Checks if the symbol on the LHS of the assignment statement is present in
751+
/// the RHS expression.
752+
inline bool checkForSymbolMatch(
753+
const Fortran::parser::AssignmentStmt &assignmentStmt) {
754+
const auto &var{std::get<Fortran::parser::Variable>(assignmentStmt.t)};
755+
const auto &expr{std::get<Fortran::parser::Expr>(assignmentStmt.t)};
756+
const auto *e{Fortran::semantics::GetExpr(expr)};
757+
const auto *v{Fortran::semantics::GetExpr(var)};
758+
auto varSyms{Fortran::evaluate::GetSymbolVector(*v)};
759+
const Fortran::semantics::Symbol &varSymbol{*varSyms.front()};
760+
for (const Fortran::semantics::Symbol &symbol :
761+
Fortran::evaluate::GetSymbolVector(*e)) {
762+
if (varSymbol == symbol) {
763+
return true;
764+
}
765+
}
766+
return false;
767+
}
739768
} // namespace Fortran::semantics
740769
#endif // FORTRAN_SEMANTICS_TOOLS_H_

‎flang/lib/Lower/DirectivesCommon.h

+3-31
Original file line numberDiff line numberDiff line change
@@ -74,34 +74,6 @@ struct AddrAndBoundsInfo {
7474
}
7575
};
7676

77-
/// Checks if the assignment statement has a single variable on the RHS.
78-
static inline bool checkForSingleVariableOnRHS(
79-
const Fortran::parser::AssignmentStmt &assignmentStmt) {
80-
const Fortran::parser::Expr &expr{
81-
std::get<Fortran::parser::Expr>(assignmentStmt.t)};
82-
const Fortran::common::Indirection<Fortran::parser::Designator> *designator =
83-
std::get_if<Fortran::common::Indirection<Fortran::parser::Designator>>(
84-
&expr.u);
85-
return designator != nullptr;
86-
}
87-
88-
/// Checks if the symbol on the LHS of the assignment statement is present in
89-
/// the RHS expression.
90-
static inline bool
91-
checkForSymbolMatch(const Fortran::parser::AssignmentStmt &assignmentStmt) {
92-
const auto &var{std::get<Fortran::parser::Variable>(assignmentStmt.t)};
93-
const auto &expr{std::get<Fortran::parser::Expr>(assignmentStmt.t)};
94-
const auto *e{Fortran::semantics::GetExpr(expr)};
95-
const auto *v{Fortran::semantics::GetExpr(var)};
96-
auto varSyms{Fortran::evaluate::GetSymbolVector(*v)};
97-
const Fortran::semantics::Symbol &varSymbol{*varSyms.front()};
98-
for (const Fortran::semantics::Symbol &symbol :
99-
Fortran::evaluate::GetSymbolVector(*e))
100-
if (varSymbol == symbol)
101-
return true;
102-
return false;
103-
}
104-
10577
/// Populates \p hint and \p memoryOrder with appropriate clause information
10678
/// if present on atomic construct.
10779
static inline void genOmpAtomicHintAndMemoryOrderClauses(
@@ -537,7 +509,7 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
537509
stmt2LHSArg = fir::getBase(converter.genExprAddr(assign2.lhs, stmtCtx));
538510

539511
// Operation specific RHS evaluations
540-
if (checkForSingleVariableOnRHS(stmt1)) {
512+
if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
541513
// Atomic capture construct is of the form [capture-stmt, update-stmt] or
542514
// of the form [capture-stmt, write-stmt]
543515
stmt1RHSArg = fir::getBase(converter.genExprAddr(assign1.rhs, stmtCtx));
@@ -573,8 +545,8 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
573545
firOpBuilder.createBlock(&(atomicCaptureOp->getRegion(0)));
574546
mlir::Block &block = atomicCaptureOp->getRegion(0).back();
575547
firOpBuilder.setInsertionPointToStart(&block);
576-
if (checkForSingleVariableOnRHS(stmt1)) {
577-
if (checkForSymbolMatch(stmt2)) {
548+
if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
549+
if (Fortran::semantics::checkForSymbolMatch(stmt2)) {
578550
// Atomic capture construct is of the form [capture-stmt, update-stmt]
579551
const Fortran::semantics::SomeExpr &fromExpr =
580552
*Fortran::semantics::GetExpr(stmt1Expr);

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

+58-6
Original file line numberDiff line numberDiff line change
@@ -1977,6 +1977,58 @@ void OmpStructureChecker::CheckAtomicUpdateStmt(
19771977
ErrIfAllocatableVariable(var);
19781978
}
19791979

1980+
// TODO: Allow cond-update-stmt once compare clause is supported.
1981+
void OmpStructureChecker::CheckAtomicCaptureConstruct(
1982+
const parser::OmpAtomicCapture &atomicCaptureConstruct) {
1983+
const Fortran::parser::AssignmentStmt &stmt1 =
1984+
std::get<Fortran::parser::OmpAtomicCapture::Stmt1>(
1985+
atomicCaptureConstruct.t)
1986+
.v.statement;
1987+
const auto &stmt1Var{std::get<Fortran::parser::Variable>(stmt1.t)};
1988+
const auto &stmt1Expr{std::get<Fortran::parser::Expr>(stmt1.t)};
1989+
1990+
const Fortran::parser::AssignmentStmt &stmt2 =
1991+
std::get<Fortran::parser::OmpAtomicCapture::Stmt2>(
1992+
atomicCaptureConstruct.t)
1993+
.v.statement;
1994+
const auto &stmt2Var{std::get<Fortran::parser::Variable>(stmt2.t)};
1995+
const auto &stmt2Expr{std::get<Fortran::parser::Expr>(stmt2.t)};
1996+
1997+
if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
1998+
CheckAtomicCaptureStmt(stmt1);
1999+
if (Fortran::semantics::checkForSymbolMatch(stmt2)) {
2000+
// ATOMIC CAPTURE construct is of the form [capture-stmt, update-stmt]
2001+
CheckAtomicUpdateStmt(stmt2);
2002+
} else {
2003+
// ATOMIC CAPTURE construct is of the form [capture-stmt, write-stmt]
2004+
CheckAtomicWriteStmt(stmt2);
2005+
}
2006+
auto *v{stmt2Var.typedExpr.get()};
2007+
auto *e{stmt1Expr.typedExpr.get()};
2008+
if (v && e && !(v->v == e->v)) {
2009+
context_.Say(stmt1Expr.source,
2010+
"Captured variable/array element/derived-type component %s expected to be assigned in the second statement of ATOMIC CAPTURE construct"_err_en_US,
2011+
stmt1Expr.source);
2012+
}
2013+
} else if (Fortran::semantics::checkForSymbolMatch(stmt1) &&
2014+
Fortran::semantics::checkForSingleVariableOnRHS(stmt2)) {
2015+
// ATOMIC CAPTURE construct is of the form [update-stmt, capture-stmt]
2016+
CheckAtomicUpdateStmt(stmt1);
2017+
CheckAtomicCaptureStmt(stmt2);
2018+
// Variable updated in stmt1 should be captured in stmt2
2019+
auto *v{stmt1Var.typedExpr.get()};
2020+
auto *e{stmt2Expr.typedExpr.get()};
2021+
if (v && e && !(v->v == e->v)) {
2022+
context_.Say(stmt1Var.GetSource(),
2023+
"Updated variable/array element/derived-type component %s expected to be captured in the second statement of ATOMIC CAPTURE construct"_err_en_US,
2024+
stmt1Var.GetSource());
2025+
}
2026+
} else {
2027+
context_.Say(stmt1Expr.source,
2028+
"Invalid ATOMIC CAPTURE construct statements. Expected one of [update-stmt, capture-stmt], [capture-stmt, update-stmt], or [capture-stmt, write-stmt]"_err_en_US);
2029+
}
2030+
}
2031+
19802032
void OmpStructureChecker::CheckAtomicMemoryOrderClause(
19812033
const parser::OmpAtomicClauseList *leftHandClauseList,
19822034
const parser::OmpAtomicClauseList *rightHandClauseList) {
@@ -2060,15 +2112,15 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
20602112
atomicWrite.t)
20612113
.statement);
20622114
},
2063-
[&](const auto &atomicConstruct) {
2064-
const auto &dir{std::get<parser::Verbatim>(atomicConstruct.t)};
2115+
[&](const parser::OmpAtomicCapture &atomicCapture) {
2116+
const auto &dir{std::get<parser::Verbatim>(atomicCapture.t)};
20652117
PushContextAndClauseSets(
20662118
dir.source, llvm::omp::Directive::OMPD_atomic);
2067-
CheckAtomicMemoryOrderClause(&std::get<0>(atomicConstruct.t),
2068-
&std::get<2>(atomicConstruct.t));
2119+
CheckAtomicMemoryOrderClause(
2120+
&std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t));
20692121
CheckHintClause<const parser::OmpAtomicClauseList>(
2070-
&std::get<0>(atomicConstruct.t),
2071-
&std::get<2>(atomicConstruct.t));
2122+
&std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t));
2123+
CheckAtomicCaptureConstruct(atomicCapture);
20722124
},
20732125
},
20742126
x.u);

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

+1
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ class OmpStructureChecker
193193
void CheckAtomicUpdateStmt(const parser::AssignmentStmt &);
194194
void CheckAtomicCaptureStmt(const parser::AssignmentStmt &);
195195
void CheckAtomicWriteStmt(const parser::AssignmentStmt &);
196+
void CheckAtomicCaptureConstruct(const parser::OmpAtomicCapture &);
196197
void CheckAtomicConstructStructure(const parser::OpenMPAtomicConstruct &);
197198
void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
198199
void CheckSIMDNest(const parser::OpenMPConstruct &x);

‎flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90

+64
Original file line numberDiff line numberDiff line change
@@ -84,4 +84,68 @@ program sample
8484
!$omp atomic write
8585
!ERROR: Expected scalar variable on the LHS of atomic assignment statement
8686
a = x
87+
88+
!$omp atomic capture
89+
v = x
90+
x = x + 1
91+
!$omp end atomic
92+
93+
!$omp atomic release capture
94+
v = x
95+
!ERROR: Atomic update statement should be of form `x = x operator expr` OR `x = expr operator x`
96+
x = b + (x*1)
97+
!$omp end atomic
98+
99+
!$omp atomic capture hint(0)
100+
v = x
101+
x = 1
102+
!$omp end atomic
103+
104+
!$omp atomic capture
105+
!ERROR: Captured variable/array element/derived-type component x expected to be assigned in the second statement of ATOMIC CAPTURE construct
106+
v = x
107+
b = b + 1
108+
!$omp end atomic
109+
110+
!$omp atomic capture
111+
!ERROR: Captured variable/array element/derived-type component x expected to be assigned in the second statement of ATOMIC CAPTURE construct
112+
v = x
113+
b = 10
114+
!$omp end atomic
115+
116+
!$omp atomic capture
117+
!ERROR: Updated variable/array element/derived-type component x expected to be captured in the second statement of ATOMIC CAPTURE construct
118+
x = x + 10
119+
v = b
120+
!$omp end atomic
121+
122+
!$omp atomic capture
123+
!ERROR: Invalid ATOMIC CAPTURE construct statements. Expected one of [update-stmt, capture-stmt], [capture-stmt, update-stmt], or [capture-stmt, write-stmt]
124+
v = 1
125+
x = 4
126+
!$omp end atomic
127+
128+
!$omp atomic capture
129+
!ERROR: Captured variable/array element/derived-type component z%y expected to be assigned in the second statement of ATOMIC CAPTURE construct
130+
x = z%y
131+
z%m = z%m + 1.0
132+
!$omp end atomic
133+
134+
!$omp atomic capture
135+
!ERROR: Updated variable/array element/derived-type component z%m expected to be captured in the second statement of ATOMIC CAPTURE construct
136+
z%m = z%m + 1.0
137+
x = z%y
138+
!$omp end atomic
139+
140+
!$omp atomic capture
141+
!ERROR: Captured variable/array element/derived-type component y(2) expected to be assigned in the second statement of ATOMIC CAPTURE construct
142+
x = y(2)
143+
y(1) = y(1) + 1
144+
!$omp end atomic
145+
146+
!$omp atomic capture
147+
!ERROR: Updated variable/array element/derived-type component y(1) expected to be captured in the second statement of ATOMIC CAPTURE construct
148+
y(1) = y(1) + 1
149+
x = y(2)
150+
!$omp end atomic
87151
end program

‎flang/test/Semantics/OpenMP/requires-atomic01.f90

+3-3
Original file line numberDiff line numberDiff line change
@@ -88,22 +88,22 @@ program requires
8888
! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst
8989
!$omp atomic capture
9090
i = j
91-
i = j
91+
j = j + 1
9292
!$omp end atomic
9393

9494
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
9595
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
9696
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
9797
!$omp atomic relaxed capture
9898
i = j
99-
i = j
99+
j = j + 1
100100
!$omp end atomic
101101

102102
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
103103
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
104104
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
105105
!$omp atomic capture relaxed
106106
i = j
107-
i = j
107+
j = j + 1
108108
!$omp end atomic
109109
end program requires

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -88,22 +88,22 @@ program requires
8888
! CHECK: OmpMemoryOrderClause -> OmpClause -> AcqRel
8989
!$omp atomic capture
9090
i = j
91-
i = j
91+
j = j + 1
9292
!$omp end atomic
9393

9494
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
9595
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> AcqRel
9696
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
9797
!$omp atomic relaxed capture
9898
i = j
99-
i = j
99+
j = j + 1
100100
!$omp end atomic
101101

102102
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
103103
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> AcqRel
104104
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
105105
!$omp atomic capture relaxed
106106
i = j
107-
i = j
107+
j = j + 1
108108
!$omp end atomic
109109
end program requires

0 commit comments

Comments
 (0)