[flang-commits] [flang] [Flang][OpenMP] Add Semantic Checks for Atomic Capture Construct (PR #108516)
via flang-commits
flang-commits at lists.llvm.org
Wed Sep 18 05:10:31 PDT 2024
https://github.com/harishch4 updated https://github.com/llvm/llvm-project/pull/108516
>From bbd04979f59a24bd03579250d31bf6616fe0a341 Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Fri, 13 Sep 2024 14:07:41 +0530
Subject: [PATCH 1/5] [Flang][OpenMP] Add Semantic Checks for Atomic Capture
Construct
---
flang/include/flang/Semantics/tools.h | 27 +++++++
flang/lib/Lower/DirectivesCommon.h | 34 +--------
flang/lib/Semantics/check-omp-structure.cpp | 76 +++++++++++++++++--
flang/lib/Semantics/check-omp-structure.h | 1 +
.../OpenMP/omp-atomic-assignment-stmt.f90 | 40 ++++++++++
.../Semantics/OpenMP/requires-atomic01.f90 | 6 +-
.../Semantics/OpenMP/requires-atomic02.f90 | 6 +-
7 files changed, 147 insertions(+), 43 deletions(-)
diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index 15c02ecc0058cc..b60514b991b727 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -736,5 +736,32 @@ std::string GetCommonBlockObjectName(const Symbol &, bool underscoring);
// Check for ambiguous USE associations
bool HadUseError(SemanticsContext &, SourceName at, const Symbol *);
+/// Checks if the assignment statement has a single variable on the RHS.
+inline bool checkForSingleVariableOnRHS(
+ const Fortran::parser::AssignmentStmt &assignmentStmt) {
+ const Fortran::parser::Expr &expr{
+ std::get<Fortran::parser::Expr>(assignmentStmt.t)};
+ const Fortran::common::Indirection<Fortran::parser::Designator> *designator =
+ std::get_if<Fortran::common::Indirection<Fortran::parser::Designator>>(
+ &expr.u);
+ return designator != nullptr;
+}
+
+/// Checks if the symbol on the LHS of the assignment statement is present in
+/// the RHS expression.
+inline bool checkForSymbolMatch(
+ const Fortran::parser::AssignmentStmt &assignmentStmt) {
+ const auto &var{std::get<Fortran::parser::Variable>(assignmentStmt.t)};
+ const auto &expr{std::get<Fortran::parser::Expr>(assignmentStmt.t)};
+ const auto *e{Fortran::semantics::GetExpr(expr)};
+ const auto *v{Fortran::semantics::GetExpr(var)};
+ auto varSyms{Fortran::evaluate::GetSymbolVector(*v)};
+ const Fortran::semantics::Symbol &varSymbol{*varSyms.front()};
+ for (const Fortran::semantics::Symbol &symbol :
+ Fortran::evaluate::GetSymbolVector(*e))
+ if (varSymbol == symbol)
+ return true;
+ return false;
+}
} // namespace Fortran::semantics
#endif // FORTRAN_SEMANTICS_TOOLS_H_
diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index d2060e77ce5305..a32f0b287e049a 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -74,34 +74,6 @@ struct AddrAndBoundsInfo {
}
};
-/// Checks if the assignment statement has a single variable on the RHS.
-static inline bool checkForSingleVariableOnRHS(
- const Fortran::parser::AssignmentStmt &assignmentStmt) {
- const Fortran::parser::Expr &expr{
- std::get<Fortran::parser::Expr>(assignmentStmt.t)};
- const Fortran::common::Indirection<Fortran::parser::Designator> *designator =
- std::get_if<Fortran::common::Indirection<Fortran::parser::Designator>>(
- &expr.u);
- return designator != nullptr;
-}
-
-/// Checks if the symbol on the LHS of the assignment statement is present in
-/// the RHS expression.
-static inline bool
-checkForSymbolMatch(const Fortran::parser::AssignmentStmt &assignmentStmt) {
- const auto &var{std::get<Fortran::parser::Variable>(assignmentStmt.t)};
- const auto &expr{std::get<Fortran::parser::Expr>(assignmentStmt.t)};
- const auto *e{Fortran::semantics::GetExpr(expr)};
- const auto *v{Fortran::semantics::GetExpr(var)};
- auto varSyms{Fortran::evaluate::GetSymbolVector(*v)};
- const Fortran::semantics::Symbol &varSymbol{*varSyms.front()};
- for (const Fortran::semantics::Symbol &symbol :
- Fortran::evaluate::GetSymbolVector(*e))
- if (varSymbol == symbol)
- return true;
- return false;
-}
-
/// Populates \p hint and \p memoryOrder with appropriate clause information
/// if present on atomic construct.
static inline void genOmpAtomicHintAndMemoryOrderClauses(
@@ -537,7 +509,7 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
stmt2LHSArg = fir::getBase(converter.genExprAddr(assign2.lhs, stmtCtx));
// Operation specific RHS evaluations
- if (checkForSingleVariableOnRHS(stmt1)) {
+ if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
// Atomic capture construct is of the form [capture-stmt, update-stmt] or
// of the form [capture-stmt, write-stmt]
stmt1RHSArg = fir::getBase(converter.genExprAddr(assign1.rhs, stmtCtx));
@@ -573,8 +545,8 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
firOpBuilder.createBlock(&(atomicCaptureOp->getRegion(0)));
mlir::Block &block = atomicCaptureOp->getRegion(0).back();
firOpBuilder.setInsertionPointToStart(&block);
- if (checkForSingleVariableOnRHS(stmt1)) {
- if (checkForSymbolMatch(stmt2)) {
+ if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
+ if (Fortran::semantics::checkForSymbolMatch(stmt2)) {
// Atomic capture construct is of the form [capture-stmt, update-stmt]
const Fortran::semantics::SomeExpr &fromExpr =
*Fortran::semantics::GetExpr(stmt1Expr);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 643b713b32e29d..662e6c10bd4e03 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1977,6 +1977,70 @@ void OmpStructureChecker::CheckAtomicUpdateStmt(
ErrIfAllocatableVariable(var);
}
+void OmpStructureChecker::CheckAtomicCaptureConstruct(
+ const parser::OmpAtomicCapture &atomicCaptureConstruct) {
+ const Fortran::parser::AssignmentStmt &stmt1 =
+ std::get<Fortran::parser::OmpAtomicCapture::Stmt1>(
+ atomicCaptureConstruct.t)
+ .v.statement;
+ const auto &stmt1Var{std::get<Fortran::parser::Variable>(stmt1.t)};
+ const auto &stmt1Expr{std::get<Fortran::parser::Expr>(stmt1.t)};
+
+ const Fortran::parser::AssignmentStmt &stmt2 =
+ std::get<Fortran::parser::OmpAtomicCapture::Stmt2>(
+ atomicCaptureConstruct.t)
+ .v.statement;
+ const auto &stmt2Var{std::get<Fortran::parser::Variable>(stmt2.t)};
+ const auto &stmt2Expr{std::get<Fortran::parser::Expr>(stmt2.t)};
+
+ if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
+ CheckAtomicCaptureStmt(stmt1);
+ if (Fortran::semantics::checkForSymbolMatch(stmt2)) {
+ // Atomic capture construct is of the form [capture-stmt, update-stmt]
+ CheckAtomicUpdateStmt(stmt2);
+ } else {
+ // Atomic capture construct is of the form [capture-stmt, write-stmt]
+ CheckAtomicWriteStmt(stmt2);
+ }
+ // Variable captured in stmt1 should be assigned in stmt2
+ const auto *e{GetExpr(context_, stmt1Expr)};
+ const auto *v{GetExpr(context_, stmt2Var)};
+ if (e && v) {
+ const Symbol &stmt2VarSymbol = evaluate::GetSymbolVector(*v).front();
+ const Symbol &stmt1ExprSymbol = evaluate::GetSymbolVector(*e).front();
+ if (stmt2VarSymbol != stmt1ExprSymbol)
+ context_.Say(stmt1Expr.source,
+ "Captured variable %s "
+ "expected to be assigned in statement 2 of "
+ "atomic capture construct"_err_en_US,
+ stmt1ExprSymbol.name().ToString());
+ }
+ } else if (Fortran::semantics::checkForSymbolMatch(stmt1) &&
+ Fortran::semantics::checkForSingleVariableOnRHS(stmt2)) {
+ // Atomic capture construct is of the form [update-stmt, capture-stmt]
+ CheckAtomicUpdateStmt(stmt1);
+ CheckAtomicCaptureStmt(stmt2);
+ // Variable updated in stmt1 should be captured in stmt2
+ const auto *e{GetExpr(context_, stmt2Expr)};
+ const auto *v{GetExpr(context_, stmt1Var)};
+ if (e && v) {
+ const Symbol &stmt1VarSymbol = evaluate::GetSymbolVector(*v).front();
+ const Symbol &stmt2ExprSymbol = evaluate::GetSymbolVector(*e).front();
+ if (stmt1VarSymbol != stmt2ExprSymbol)
+ context_.Say(stmt1Var.GetSource(),
+ "Updated variable %s "
+ "expected to be captured in statement 2 of "
+ "atomic capture construct"_err_en_US,
+ stmt1Var.GetSource().ToString());
+ }
+ } else {
+ context_.Say(stmt1Expr.source,
+ "Invalid atomic capture construct statements. "
+ "Expected one of [update-stmt, capture-stmt], "
+ "[capture-stmt, update-stmt], or [capture-stmt, write-stmt]"_err_en_US);
+ }
+}
+
void OmpStructureChecker::CheckAtomicMemoryOrderClause(
const parser::OmpAtomicClauseList *leftHandClauseList,
const parser::OmpAtomicClauseList *rightHandClauseList) {
@@ -2060,15 +2124,15 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
atomicWrite.t)
.statement);
},
- [&](const auto &atomicConstruct) {
- const auto &dir{std::get<parser::Verbatim>(atomicConstruct.t)};
+ [&](const parser::OmpAtomicCapture &atomicCapture) {
+ const auto &dir{std::get<parser::Verbatim>(atomicCapture.t)};
PushContextAndClauseSets(
dir.source, llvm::omp::Directive::OMPD_atomic);
- CheckAtomicMemoryOrderClause(&std::get<0>(atomicConstruct.t),
- &std::get<2>(atomicConstruct.t));
+ CheckAtomicMemoryOrderClause(
+ &std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t));
CheckHintClause<const parser::OmpAtomicClauseList>(
- &std::get<0>(atomicConstruct.t),
- &std::get<2>(atomicConstruct.t));
+ &std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t));
+ CheckAtomicCaptureConstruct(atomicCapture);
},
},
x.u);
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 2cc1a78068f540..8bfd4d594b028e 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -193,6 +193,7 @@ class OmpStructureChecker
void CheckAtomicUpdateStmt(const parser::AssignmentStmt &);
void CheckAtomicCaptureStmt(const parser::AssignmentStmt &);
void CheckAtomicWriteStmt(const parser::AssignmentStmt &);
+ void CheckAtomicCaptureConstruct(const parser::OmpAtomicCapture &);
void CheckAtomicConstructStructure(const parser::OpenMPAtomicConstruct &);
void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
void CheckSIMDNest(const parser::OpenMPConstruct &x);
diff --git a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90 b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
index a346056dee383b..09921944e7fc06 100644
--- a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
+++ b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
@@ -84,4 +84,44 @@ program sample
!$omp atomic write
!ERROR: Expected scalar variable on the LHS of atomic assignment statement
a = x
+
+ !$omp atomic capture
+ v = x
+ x = x + 1
+ !$omp end atomic
+
+ !$omp atomic release capture
+ v = x
+ !ERROR: Atomic update statement should be of form `x = x operator expr` OR `x = expr operator x`
+ x = b + (x*1)
+ !$omp end atomic
+
+ !$omp atomic capture hint(0)
+ v = x
+ x = 1
+ !$omp end atomic
+
+ !$omp atomic capture
+ !ERROR: Captured variable x expected to be assigned in statement 2 of atomic capture construct
+ v = x
+ b = b + 1
+ !$omp end atomic
+
+ !$omp atomic capture
+ !ERROR: Captured variable x expected to be assigned in statement 2 of atomic capture construct
+ v = x
+ b = 10
+ !$omp end atomic
+
+ !$omp atomic capture
+ !ERROR: Updated variable x expected to be captured in statement 2 of atomic capture construct
+ x = x + 10
+ v = b
+ !$omp end atomic
+
+ !$omp atomic capture
+ !ERROR: Invalid atomic capture construct statements. Expected one of [update-stmt, capture-stmt], [capture-stmt, update-stmt], or [capture-stmt, write-stmt]
+ v = 1
+ x = 4
+ !$omp end atomic
end program
diff --git a/flang/test/Semantics/OpenMP/requires-atomic01.f90 b/flang/test/Semantics/OpenMP/requires-atomic01.f90
index b39c9cdcc0bb33..cb7b1bc1ac52ab 100644
--- a/flang/test/Semantics/OpenMP/requires-atomic01.f90
+++ b/flang/test/Semantics/OpenMP/requires-atomic01.f90
@@ -88,7 +88,7 @@ program requires
! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst
!$omp atomic capture
i = j
- i = j
+ j = j + 1
!$omp end atomic
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
@@ -96,7 +96,7 @@ program requires
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
!$omp atomic relaxed capture
i = j
- i = j
+ j = j + 1
!$omp end atomic
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
@@ -104,6 +104,6 @@ program requires
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
!$omp atomic capture relaxed
i = j
- i = j
+ j = j + 1
!$omp end atomic
end program requires
diff --git a/flang/test/Semantics/OpenMP/requires-atomic02.f90 b/flang/test/Semantics/OpenMP/requires-atomic02.f90
index 3af83970e7927a..5a4249794f7b50 100644
--- a/flang/test/Semantics/OpenMP/requires-atomic02.f90
+++ b/flang/test/Semantics/OpenMP/requires-atomic02.f90
@@ -88,7 +88,7 @@ program requires
! CHECK: OmpMemoryOrderClause -> OmpClause -> AcqRel
!$omp atomic capture
i = j
- i = j
+ j = j + 1
!$omp end atomic
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
@@ -96,7 +96,7 @@ program requires
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
!$omp atomic relaxed capture
i = j
- i = j
+ j = j + 1
!$omp end atomic
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
@@ -104,6 +104,6 @@ program requires
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
!$omp atomic capture relaxed
i = j
- i = j
+ j = j + 1
!$omp end atomic
end program requires
>From 2b4292126927ffcbe86fd462257e70a79ecd9feb Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Tue, 17 Sep 2024 15:29:23 +0530
Subject: [PATCH 2/5] Add TODO
---
flang/lib/Semantics/check-omp-structure.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 662e6c10bd4e03..8365793d473374 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1977,6 +1977,7 @@ void OmpStructureChecker::CheckAtomicUpdateStmt(
ErrIfAllocatableVariable(var);
}
+// TODO: Allow cond-update-stmt once compare clause is supported.
void OmpStructureChecker::CheckAtomicCaptureConstruct(
const parser::OmpAtomicCapture &atomicCaptureConstruct) {
const Fortran::parser::AssignmentStmt &stmt1 =
>From 3e3eb13b267427b07ef57007c2e230dd6ed0a449 Mon Sep 17 00:00:00 2001
From: harishch4 <harishcse44 at gmail.com>
Date: Wed, 18 Sep 2024 17:33:34 +0530
Subject: [PATCH 3/5] Update flang/lib/Semantics/check-omp-structure.cpp
Co-authored-by: Kiran Chandramohan <kiranchandramohan at gmail.com>
---
flang/lib/Semantics/check-omp-structure.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 8365793d473374..f8fc3c9727a304 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2030,7 +2030,7 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
if (stmt1VarSymbol != stmt2ExprSymbol)
context_.Say(stmt1Var.GetSource(),
"Updated variable %s "
- "expected to be captured in statement 2 of "
+ "expected to be captured in the second statement of "
"atomic capture construct"_err_en_US,
stmt1Var.GetSource().ToString());
}
>From a7e38da538b313939ec52030a4585e35889d7350 Mon Sep 17 00:00:00 2001
From: harishch4 <harishcse44 at gmail.com>
Date: Wed, 18 Sep 2024 17:33:45 +0530
Subject: [PATCH 4/5] Update flang/lib/Semantics/check-omp-structure.cpp
Co-authored-by: Kiran Chandramohan <kiranchandramohan at gmail.com>
---
flang/lib/Semantics/check-omp-structure.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index f8fc3c9727a304..2893859ae99152 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2012,7 +2012,7 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
if (stmt2VarSymbol != stmt1ExprSymbol)
context_.Say(stmt1Expr.source,
"Captured variable %s "
- "expected to be assigned in statement 2 of "
+ "expected to be assigned in the second statement of "
"atomic capture construct"_err_en_US,
stmt1ExprSymbol.name().ToString());
}
>From 1a76168b7cded0ef7cbef1d572fabaaa1eaf89be Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Wed, 18 Sep 2024 17:40:12 +0530
Subject: [PATCH 5/5] Add braces
---
flang/include/flang/Semantics/tools.h | 6 ++++--
flang/lib/Semantics/check-omp-structure.cpp | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index b60514b991b727..96d4dbb2acaa11 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -758,9 +758,11 @@ inline bool checkForSymbolMatch(
auto varSyms{Fortran::evaluate::GetSymbolVector(*v)};
const Fortran::semantics::Symbol &varSymbol{*varSyms.front()};
for (const Fortran::semantics::Symbol &symbol :
- Fortran::evaluate::GetSymbolVector(*e))
- if (varSymbol == symbol)
+ Fortran::evaluate::GetSymbolVector(*e)) {
+ if (varSymbol == symbol) {
return true;
+ }
+ }
return false;
}
} // namespace Fortran::semantics
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 2893859ae99152..18b8ca5f511771 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2009,12 +2009,13 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
if (e && v) {
const Symbol &stmt2VarSymbol = evaluate::GetSymbolVector(*v).front();
const Symbol &stmt1ExprSymbol = evaluate::GetSymbolVector(*e).front();
- if (stmt2VarSymbol != stmt1ExprSymbol)
+ if (stmt2VarSymbol != stmt1ExprSymbol) {
context_.Say(stmt1Expr.source,
"Captured variable %s "
"expected to be assigned in the second statement of "
"atomic capture construct"_err_en_US,
stmt1ExprSymbol.name().ToString());
+ }
}
} else if (Fortran::semantics::checkForSymbolMatch(stmt1) &&
Fortran::semantics::checkForSingleVariableOnRHS(stmt2)) {
More information about the flang-commits
mailing list