[flang-commits] [flang] 062b22e - [flang][openacc] Add semantic checks for atomic constructs (#149579)
via flang-commits
flang-commits at lists.llvm.org
Wed Jul 30 08:13:11 PDT 2025
Author: Andre Kuhlenschmidt
Date: 2025-07-30T08:13:07-07:00
New Revision: 062b22e46238be9a542755a9021486b651731908
URL: https://github.com/llvm/llvm-project/commit/062b22e46238be9a542755a9021486b651731908
DIFF: https://github.com/llvm/llvm-project/commit/062b22e46238be9a542755a9021486b651731908.diff
LOG: [flang][openacc] Add semantic checks for atomic constructs (#149579)
An error report of the following code generating non-atomic code led us
to realize there are missing checks in the OpenACC atomics code. Add
some of those checks for atomic and sketch how the rest of the code
should proceed in checking the rest of the properties. The following
cases are all reported as errors.
```fortran
! Originally reported error!
!$acc atomic capture
a = b
c = b
!$acc end atomic capture
! Other ambiguous, but related errors!
!$acc atomic capture
x = i
i = x
!$acc end atomic capture
!$acc atomic capture
a = b
b = b
!$acc end atomic capture
!$acc atomic capture
a = b
a = c
!$acc end atomic capture
```
Added:
Modified:
flang/include/flang/Evaluate/tools.h
flang/lib/Evaluate/tools.cpp
flang/lib/Lower/OpenMP/Atomic.cpp
flang/lib/Semantics/check-acc-structure.cpp
flang/lib/Semantics/check-acc-structure.h
flang/lib/Semantics/check-omp-atomic.cpp
flang/lib/Semantics/openmp-utils.cpp
flang/lib/Semantics/openmp-utils.h
flang/test/Lower/OpenACC/acc-atomic-capture.f90
flang/test/Semantics/OpenACC/acc-atomic-validity.f90
Removed:
################################################################################
diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h
index 96ed86f468350..cef57f1851bcc 100644
--- a/flang/include/flang/Evaluate/tools.h
+++ b/flang/include/flang/Evaluate/tools.h
@@ -10,6 +10,7 @@
#define FORTRAN_EVALUATE_TOOLS_H_
#include "traverse.h"
+#include "flang/Common/enum-set.h"
#include "flang/Common/idioms.h"
#include "flang/Common/template.h"
#include "flang/Common/unwrap.h"
@@ -1397,6 +1398,8 @@ enum class Operator {
True,
};
+using OperatorSet = common::EnumSet<Operator, 32>;
+
std::string ToString(Operator op);
template <typename... Ts, int Kind>
@@ -1509,9 +1512,18 @@ Operator OperationCode(const evaluate::ProcedureDesignator &proc);
std::pair<operation::Operator, std::vector<Expr<SomeType>>>
GetTopLevelOperation(const Expr<SomeType> &expr);
+// Return information about the top-level operation (ignoring parentheses, and
+// resizing converts)
+std::pair<operation::Operator, std::vector<Expr<SomeType>>>
+GetTopLevelOperationIgnoreResizing(const Expr<SomeType> &expr);
+
// Check if expr is same as x, or a sequence of Convert operations on x.
bool IsSameOrConvertOf(const Expr<SomeType> &expr, const Expr<SomeType> &x);
+// Check if the Variable appears as a subexpression of the expression.
+bool IsVarSubexpressionOf(
+ const Expr<SomeType> &var, const Expr<SomeType> &super);
+
// Strip away any top-level Convert operations (if any exist) and return
// the input value. A ComplexConstructor(x, 0) is also considered as a
// convert operation.
diff --git a/flang/lib/Evaluate/tools.cpp b/flang/lib/Evaluate/tools.cpp
index 21e6b3c3dd50d..171dd91fa9fd1 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -1809,10 +1809,15 @@ operation::Operator operation::OperationCode(const ProcedureDesignator &proc) {
}
std::pair<operation::Operator, std::vector<Expr<SomeType>>>
-GetTopLevelOperation(const Expr<SomeType> &expr) {
+GetTopLevelOperationIgnoreResizing(const Expr<SomeType> &expr) {
return operation::ArgumentExtractor<true>{}(expr);
}
+std::pair<operation::Operator, std::vector<Expr<SomeType>>>
+GetTopLevelOperation(const Expr<SomeType> &expr) {
+ return operation::ArgumentExtractor<false>{}(expr);
+}
+
namespace operation {
struct ConvertCollector
: public Traverse<ConvertCollector,
@@ -1936,6 +1941,33 @@ bool IsSameOrConvertOf(const Expr<SomeType> &expr, const Expr<SomeType> &x) {
return false;
}
}
+
+struct VariableFinder : public evaluate::AnyTraverse<VariableFinder> {
+ using Base = evaluate::AnyTraverse<VariableFinder>;
+ using SomeExpr = Expr<SomeType>;
+ VariableFinder(const SomeExpr &v) : Base(*this), var(v) {}
+
+ using Base::operator();
+
+ template <typename T>
+ bool operator()(const evaluate::Designator<T> &x) const {
+ return evaluate::AsGenericExpr(common::Clone(x)) == var;
+ }
+
+ template <typename T>
+ bool operator()(const evaluate::FunctionRef<T> &x) const {
+ return evaluate::AsGenericExpr(common::Clone(x)) == var;
+ }
+
+private:
+ const SomeExpr &var;
+};
+
+bool IsVarSubexpressionOf(
+ const Expr<SomeType> &sub, const Expr<SomeType> &super) {
+ return VariableFinder{sub}(super);
+}
+
} // namespace Fortran::evaluate
namespace Fortran::semantics {
diff --git a/flang/lib/Lower/OpenMP/Atomic.cpp b/flang/lib/Lower/OpenMP/Atomic.cpp
index d4f83f57bd5d4..c9a6dbabd4182 100644
--- a/flang/lib/Lower/OpenMP/Atomic.cpp
+++ b/flang/lib/Lower/OpenMP/Atomic.cpp
@@ -607,7 +607,7 @@ genAtomicUpdate(lower::AbstractConverter &converter,
// This must exist by now.
semantics::SomeExpr rhs = assign.rhs;
semantics::SomeExpr input = *evaluate::GetConvertInput(rhs);
- auto [opcode, args] = evaluate::GetTopLevelOperation(input);
+ auto [opcode, args] = evaluate::GetTopLevelOperationIgnoreResizing(input);
assert(!args.empty() && "Update operation without arguments");
// Pass args as an argument to avoid capturing a structured binding.
@@ -625,7 +625,8 @@ genAtomicUpdate(lower::AbstractConverter &converter,
// operations with exactly two (non-optional) arguments.
rhs = genReducedMinMax(rhs, atomArg, args);
input = *evaluate::GetConvertInput(rhs);
- std::tie(opcode, args) = evaluate::GetTopLevelOperation(input);
+ std::tie(opcode, args) =
+ evaluate::GetTopLevelOperationIgnoreResizing(input);
atomArg = nullptr; // No longer valid.
}
for (auto &arg : args) {
diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 9cbea9742a6a2..77e2b01578641 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -7,8 +7,15 @@
//===----------------------------------------------------------------------===//
#include "check-acc-structure.h"
#include "flang/Common/enum-set.h"
+#include "flang/Evaluate/tools.h"
#include "flang/Parser/parse-tree.h"
+#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
+#include "flang/Semantics/type.h"
+#include "flang/Support/Fortran.h"
+#include "llvm/Support/AtomicOrdering.h"
+
+#include <optional>
#define CHECK_SIMPLE_CLAUSE(X, Y) \
void AccStructureChecker::Enter(const parser::AccClause::X &) { \
@@ -342,20 +349,219 @@ void AccStructureChecker::Leave(const parser::OpenACCAtomicConstruct &x) {
dirContext_.pop_back();
}
-void AccStructureChecker::Enter(const parser::AccAtomicUpdate &x) {
- const parser::AssignmentStmt &assignment{
- std::get<parser::Statement<parser::AssignmentStmt>>(x.t).statement};
- const auto &var{std::get<parser::Variable>(assignment.t)};
- const auto &expr{std::get<parser::Expr>(assignment.t)};
+void AccStructureChecker::CheckAtomicStmt(
+ const parser::AssignmentStmt &assign, const std::string &construct) {
+ const auto &var{std::get<parser::Variable>(assign.t)};
+ const auto &expr{std::get<parser::Expr>(assign.t)};
const auto *rhs{GetExpr(context_, expr)};
const auto *lhs{GetExpr(context_, var)};
- if (lhs && rhs) {
- if (lhs->Rank() != 0)
+
+ if (lhs) {
+ if (lhs->Rank() != 0) {
context_.Say(expr.source,
- "LHS of atomic update statement must be scalar"_err_en_US);
- if (rhs->Rank() != 0)
+ "LHS of atomic %s statement must be scalar"_err_en_US, construct);
+ }
+ // TODO: Check if lhs is intrinsic type.
+ }
+ if (rhs) {
+ if (rhs->Rank() != 0) {
context_.Say(var.GetSource(),
- "RHS of atomic update statement must be scalar"_err_en_US);
+ "RHS of atomic %s statement must be scalar"_err_en_US, construct);
+ }
+ // TODO: Check if rhs is intrinsic type.
+ }
+}
+
+static constexpr evaluate::operation::OperatorSet validAccAtomicUpdateOperators{
+ evaluate::operation::Operator::Add, evaluate::operation::Operator::Mul,
+ evaluate::operation::Operator::Sub, evaluate::operation::Operator::Div,
+ evaluate::operation::Operator::And, evaluate::operation::Operator::Or,
+ evaluate::operation::Operator::Eqv, evaluate::operation::Operator::Neqv,
+ evaluate::operation::Operator::Max, evaluate::operation::Operator::Min};
+
+static bool IsValidAtomicUpdateOperation(
+ const evaluate::operation::Operator &op) {
+ return validAccAtomicUpdateOperators.test(op);
+}
+
+// Couldn't reproduce this behavior with evaluate::UnwrapConvertedExpr which
+// is similar but only works within a single type category.
+static SomeExpr GetExprModuloConversion(const SomeExpr &expr) {
+ const auto [op, args]{evaluate::GetTopLevelOperation(expr)};
+ // Check: if it is a conversion then it must have at least one argument.
+ CHECK(((op != evaluate::operation::Operator::Convert &&
+ op != evaluate::operation::Operator::Resize) ||
+ args.size() >= 1) &&
+ "Invalid conversion operation");
+ if ((op == evaluate::operation::Operator::Convert ||
+ op == evaluate::operation::Operator::Resize) &&
+ args.size() >= 1) {
+ return args[0];
+ }
+ return expr;
+}
+
+void AccStructureChecker::CheckAtomicUpdateStmt(
+ const parser::AssignmentStmt &assign, const SomeExpr &updateVar,
+ const SomeExpr *captureVar) {
+ CheckAtomicStmt(assign, "update");
+ const auto &expr{std::get<parser::Expr>(assign.t)};
+ const auto *rhs{GetExpr(context_, expr)};
+ if (rhs) {
+ const auto [op, args]{
+ evaluate::GetTopLevelOperation(GetExprModuloConversion(*rhs))};
+ if (!IsValidAtomicUpdateOperation(op)) {
+ context_.Say(expr.source,
+ "Invalid atomic update operation, can only use: *, +, -, *, /, and, or, eqv, neqv, max, min, iand, ior, ieor"_err_en_US);
+ } else {
+ bool foundUpdateVar{false};
+ for (const auto &arg : args) {
+ if (updateVar == GetExprModuloConversion(arg)) {
+ if (foundUpdateVar) {
+ context_.Say(expr.source,
+ "The updated variable, %s, cannot appear more than once in the atomic update operation"_err_en_US,
+ updateVar.AsFortran());
+ } else {
+ foundUpdateVar = true;
+ }
+ } else if (evaluate::IsVarSubexpressionOf(updateVar, arg)) {
+ // TODO: Get the source location of arg and point to the individual
+ // argument.
+ context_.Say(expr.source,
+ "Arguments to the atomic update operation cannot reference the updated variable, %s, as a subexpression"_err_en_US,
+ updateVar.AsFortran());
+ }
+ }
+ if (!foundUpdateVar) {
+ context_.Say(expr.source,
+ "The RHS of this atomic update statement must reference the updated variable: %s"_err_en_US,
+ updateVar.AsFortran());
+ }
+ }
+ }
+}
+
+void AccStructureChecker::CheckAtomicWriteStmt(
+ const parser::AssignmentStmt &assign, const SomeExpr &updateVar,
+ const SomeExpr *captureVar) {
+ CheckAtomicStmt(assign, "write");
+ const auto &expr{std::get<parser::Expr>(assign.t)};
+ const auto *rhs{GetExpr(context_, expr)};
+ if (rhs) {
+ if (evaluate::IsVarSubexpressionOf(updateVar, *rhs)) {
+ context_.Say(expr.source,
+ "The RHS of this atomic write statement cannot reference the atomic variable: %s"_err_en_US,
+ updateVar.AsFortran());
+ }
+ }
+}
+
+void AccStructureChecker::CheckAtomicCaptureStmt(
+ const parser::AssignmentStmt &assign, const SomeExpr *updateVar,
+ const SomeExpr &captureVar) {
+ CheckAtomicStmt(assign, "capture");
+}
+
+void AccStructureChecker::Enter(const parser::AccAtomicCapture &capture) {
+ const Fortran::parser::AssignmentStmt &stmt1{
+ std::get<Fortran::parser::AccAtomicCapture::Stmt1>(capture.t)
+ .v.statement};
+ const Fortran::parser::AssignmentStmt &stmt2{
+ std::get<Fortran::parser::AccAtomicCapture::Stmt2>(capture.t)
+ .v.statement};
+ const auto &var1{std::get<parser::Variable>(stmt1.t)};
+ const auto &var2{std::get<parser::Variable>(stmt2.t)};
+ const auto *lhs1{GetExpr(context_, var1)};
+ const auto *lhs2{GetExpr(context_, var2)};
+ if (!lhs1 || !lhs2) {
+ // Not enough information to check.
+ return;
+ }
+ if (*lhs1 == *lhs2) {
+ context_.Say(std::get<parser::Verbatim>(capture.t).source,
+ "The variables assigned in this atomic capture construct must be distinct"_err_en_US);
+ return;
+ }
+ const auto &expr1{std::get<parser::Expr>(stmt1.t)};
+ const auto &expr2{std::get<parser::Expr>(stmt2.t)};
+ const auto *rhs1{GetExpr(context_, expr1)};
+ const auto *rhs2{GetExpr(context_, expr2)};
+ if (!rhs1 || !rhs2) {
+ return;
+ }
+ bool stmt1CapturesLhs2{*lhs2 == GetExprModuloConversion(*rhs1)};
+ bool stmt2CapturesLhs1{*lhs1 == GetExprModuloConversion(*rhs2)};
+ if (stmt1CapturesLhs2 && !stmt2CapturesLhs1) {
+ if (*lhs2 == GetExprModuloConversion(*rhs2)) {
+ // a = b; b = b: Doesn't fit the spec.
+ context_.Say(std::get<parser::Verbatim>(capture.t).source,
+ "The assignments in this atomic capture construct do not update a variable and capture either its initial or final value"_err_en_US);
+ // TODO: Add attatchment that a = b seems to be a capture,
+ // but b = b is not a valid update or write.
+ } else if (evaluate::IsVarSubexpressionOf(*lhs2, *rhs2)) {
+ // Take v = x; x = <expr w/ x> as capture; update
+ const auto &updateVar{*lhs2};
+ const auto &captureVar{*lhs1};
+ CheckAtomicCaptureStmt(stmt1, &updateVar, captureVar);
+ CheckAtomicUpdateStmt(stmt2, updateVar, &captureVar);
+ } else {
+ // Take v = x; x = <expr w/o x> as capture; write
+ const auto &updateVar{*lhs2};
+ const auto &captureVar{*lhs1};
+ CheckAtomicCaptureStmt(stmt1, &updateVar, captureVar);
+ CheckAtomicWriteStmt(stmt2, updateVar, &captureVar);
+ }
+ } else if (stmt2CapturesLhs1 && !stmt1CapturesLhs2) {
+ if (*lhs1 == GetExprModuloConversion(*rhs1)) {
+ // Error a = a; b = a;
+ context_.Say(var1.GetSource(),
+ "The first assignment in this atomic capture construct doesn't perform a valid update"_err_en_US);
+ // Add attatchment that a = a is not considered an update,
+ // but b = a seems to be a capture.
+ } else {
+ // Take x = <expr>; v = x: as update; capture
+ const auto &updateVar{*lhs1};
+ const auto &captureVar{*lhs2};
+ CheckAtomicUpdateStmt(stmt1, updateVar, &captureVar);
+ CheckAtomicCaptureStmt(stmt2, &updateVar, captureVar);
+ }
+ } else if (stmt1CapturesLhs2 && stmt2CapturesLhs1) {
+ // x1 = x2; x2 = x1; Doesn't fit the spec.
+ context_.Say(std::get<parser::Verbatim>(capture.t).source,
+ "The assignments in this atomic capture construct do not update a variable and capture either its initial or final value"_err_en_US);
+ // TODO: Add attatchment that both assignments seem to be captures.
+ } else { // !stmt1CapturesLhs2 && !stmt2CapturesLhs1
+ // a = <expr != b>; b = <expr != a>; Doesn't fit the spec
+ context_.Say(std::get<parser::Verbatim>(capture.t).source,
+ "The assignments in this atomic capture construct do not update a variable and capture either its initial or final value"_err_en_US);
+ // TODO: Add attatchment that neither assignment seems to be a capture.
+ }
+}
+
+void AccStructureChecker::Enter(const parser::AccAtomicUpdate &x) {
+ const auto &assign{
+ std::get<parser::Statement<parser::AssignmentStmt>>(x.t).statement};
+ const auto &var{std::get<parser::Variable>(assign.t)};
+ if (const auto *updateVar{GetExpr(context_, var)}) {
+ CheckAtomicUpdateStmt(assign, *updateVar, /*captureVar=*/nullptr);
+ }
+}
+
+void AccStructureChecker::Enter(const parser::AccAtomicWrite &x) {
+ const auto &assign{
+ std::get<parser::Statement<parser::AssignmentStmt>>(x.t).statement};
+ const auto &var{std::get<parser::Variable>(assign.t)};
+ if (const auto *updateVar{GetExpr(context_, var)}) {
+ CheckAtomicWriteStmt(assign, *updateVar, /*captureVar=*/nullptr);
+ }
+}
+
+void AccStructureChecker::Enter(const parser::AccAtomicRead &x) {
+ const auto &assign{
+ std::get<parser::Statement<parser::AssignmentStmt>>(x.t).statement};
+ const auto &var{std::get<parser::Variable>(assign.t)};
+ if (const auto *captureVar{GetExpr(context_, var)}) {
+ CheckAtomicCaptureStmt(assign, /*updateVar=*/nullptr, *captureVar);
}
}
diff --git a/flang/lib/Semantics/check-acc-structure.h b/flang/lib/Semantics/check-acc-structure.h
index 6a9aa01a9bb13..359f1557b62cb 100644
--- a/flang/lib/Semantics/check-acc-structure.h
+++ b/flang/lib/Semantics/check-acc-structure.h
@@ -63,6 +63,9 @@ class AccStructureChecker
void Enter(const parser::OpenACCCacheConstruct &);
void Leave(const parser::OpenACCCacheConstruct &);
void Enter(const parser::AccAtomicUpdate &);
+ void Enter(const parser::AccAtomicCapture &);
+ void Enter(const parser::AccAtomicWrite &);
+ void Enter(const parser::AccAtomicRead &);
void Enter(const parser::OpenACCEndConstruct &);
// Clauses
@@ -80,6 +83,19 @@ class AccStructureChecker
#include "llvm/Frontend/OpenACC/ACC.inc"
private:
+ void CheckAtomicStmt(
+ const parser::AssignmentStmt &assign, const std::string &construct);
+ void CheckAtomicUpdateStmt(const parser::AssignmentStmt &assign,
+ const SomeExpr &updateVar, const SomeExpr *captureVar);
+ void CheckAtomicCaptureStmt(const parser::AssignmentStmt &assign,
+ const SomeExpr *updateVar, const SomeExpr &captureVar);
+ void CheckAtomicWriteStmt(const parser::AssignmentStmt &assign,
+ const SomeExpr &updateVar, const SomeExpr *captureVar);
+ void CheckAtomicUpdateVariable(
+ const parser::Variable &updateVar, const parser::Variable &captureVar);
+ void CheckAtomicCaptureVariable(
+ const parser::Variable &captureVar, const parser::Variable &updateVar);
+
bool CheckAllowedModifier(llvm::acc::Clause clause);
bool IsComputeConstruct(llvm::acc::Directive directive) const;
bool IsInsideComputeConstruct() const;
diff --git a/flang/lib/Semantics/check-omp-atomic.cpp b/flang/lib/Semantics/check-omp-atomic.cpp
index c5ed8796f0c34..333fad0ca8678 100644
--- a/flang/lib/Semantics/check-omp-atomic.cpp
+++ b/flang/lib/Semantics/check-omp-atomic.cpp
@@ -197,7 +197,8 @@ static std::pair<parser::CharBlock, parser::CharBlock> SplitAssignmentSource(
}
static bool IsCheckForAssociated(const SomeExpr &cond) {
- return GetTopLevelOperation(cond).first == operation::Operator::Associated;
+ return GetTopLevelOperationIgnoreResizing(cond).first ==
+ operation::Operator::Associated;
}
static bool IsMaybeAtomicWrite(const evaluate::Assignment &assign) {
@@ -399,8 +400,8 @@ OmpStructureChecker::CheckUpdateCapture(
// subexpression of the right-hand side.
// 2. An assignment could be a capture (cbc) if the right-hand side is
// a variable (or a function ref), with potential type conversions.
- bool cbu1{IsSubexpressionOf(as1.lhs, as1.rhs)}; // Can as1 be an update?
- bool cbu2{IsSubexpressionOf(as2.lhs, as2.rhs)}; // Can as2 be an update?
+ bool cbu1{IsVarSubexpressionOf(as1.lhs, as1.rhs)}; // Can as1 be an update?
+ bool cbu2{IsVarSubexpressionOf(as2.lhs, as2.rhs)}; // Can as2 be an update?
bool cbc1{IsVarOrFunctionRef(GetConvertInput(as1.rhs))}; // Can 1 be capture?
bool cbc2{IsVarOrFunctionRef(GetConvertInput(as2.rhs))}; // Can 2 be capture?
@@ -607,7 +608,7 @@ void OmpStructureChecker::CheckAtomicUpdateAssignment(
std::pair<operation::Operator, std::vector<SomeExpr>> top{
operation::Operator::Unknown, {}};
if (auto &&maybeInput{GetConvertInput(update.rhs)}) {
- top = GetTopLevelOperation(*maybeInput);
+ top = GetTopLevelOperationIgnoreResizing(*maybeInput);
}
switch (top.first) {
case operation::Operator::Add:
@@ -657,7 +658,7 @@ void OmpStructureChecker::CheckAtomicUpdateAssignment(
if (IsSameOrConvertOf(arg, atom)) {
++count;
} else {
- if (!subExpr && IsSubexpressionOf(atom, arg)) {
+ if (!subExpr && evaluate::IsVarSubexpressionOf(atom, arg)) {
subExpr = arg;
}
nonAtom.push_back(arg);
@@ -715,7 +716,7 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateAssignment(
CheckAtomicVariable(atom, alsrc);
- auto top{GetTopLevelOperation(cond)};
+ auto top{GetTopLevelOperationIgnoreResizing(cond)};
// Missing arguments to operations would have been diagnosed by now.
switch (top.first) {
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index da14507aa9fe6..7a492a4378907 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -270,28 +270,6 @@ struct DesignatorCollector : public evaluate::Traverse<DesignatorCollector,
}
};
-struct VariableFinder : public evaluate::AnyTraverse<VariableFinder> {
- using Base = evaluate::AnyTraverse<VariableFinder>;
- VariableFinder(const SomeExpr &v) : Base(*this), var(v) {}
-
- using Base::operator();
-
- template <typename T>
- bool operator()(const evaluate::Designator<T> &x) const {
- auto copy{x};
- return evaluate::AsGenericExpr(std::move(copy)) == var;
- }
-
- template <typename T>
- bool operator()(const evaluate::FunctionRef<T> &x) const {
- auto copy{x};
- return evaluate::AsGenericExpr(std::move(copy)) == var;
- }
-
-private:
- const SomeExpr &var;
-};
-
std::vector<SomeExpr> GetAllDesignators(const SomeExpr &expr) {
return DesignatorCollector{}(expr);
}
@@ -380,10 +358,6 @@ const SomeExpr *HasStorageOverlap(
return nullptr;
}
-bool IsSubexpressionOf(const SomeExpr &sub, const SomeExpr &super) {
- return VariableFinder{sub}(super);
-}
-
// Check if the ActionStmt is actually a [Pointer]AssignmentStmt. This is
// to separate cases where the source has something that looks like an
// assignment, but is semantically wrong (diagnosed by general semantic
diff --git a/flang/lib/Semantics/openmp-utils.h b/flang/lib/Semantics/openmp-utils.h
index 001fbeb45ceec..b8ad9ed17c720 100644
--- a/flang/lib/Semantics/openmp-utils.h
+++ b/flang/lib/Semantics/openmp-utils.h
@@ -72,7 +72,6 @@ std::optional<bool> IsContiguous(
std::vector<SomeExpr> GetAllDesignators(const SomeExpr &expr);
const SomeExpr *HasStorageOverlap(
const SomeExpr &base, llvm::ArrayRef<SomeExpr> exprs);
-bool IsSubexpressionOf(const SomeExpr &sub, const SomeExpr &super);
bool IsAssignment(const parser::ActionStmt *x);
bool IsPointerAssignment(const evaluate::Assignment &x);
const parser::Block &GetInnermostExecPart(const parser::Block &block);
diff --git a/flang/test/Lower/OpenACC/acc-atomic-capture.f90 b/flang/test/Lower/OpenACC/acc-atomic-capture.f90
index ee38ab6ce826a..30e60e34b13a2 100644
--- a/flang/test/Lower/OpenACC/acc-atomic-capture.f90
+++ b/flang/test/Lower/OpenACC/acc-atomic-capture.f90
@@ -123,17 +123,20 @@ subroutine capture_with_convert_f32_to_i32()
! CHECK: }
subroutine capture_with_convert_i32_to_f64()
- real(8) :: x
- integer :: v
+ real(8) :: x
+ integer :: v, u
x = 1.0
v = 0
+ u = 1
!$acc atomic capture
v = x
- x = v
+ x = u
!$acc end atomic
end subroutine capture_with_convert_i32_to_f64
! CHECK-LABEL: func.func @_QPcapture_with_convert_i32_to_f64()
+! CHECK: %[[U:.*]] = fir.alloca i32 {bindc_name = "u", uniq_name = "_QFcapture_with_convert_i32_to_f64Eu"}
+! CHECK: %[[U_DECL:.*]]:2 = hlfir.declare %[[U]] {uniq_name = "_QFcapture_with_convert_i32_to_f64Eu"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
! CHECK: %[[V:.*]] = fir.alloca i32 {bindc_name = "v", uniq_name = "_QFcapture_with_convert_i32_to_f64Ev"}
! CHECK: %[[V_DECL:.*]]:2 = hlfir.declare %[[V]] {uniq_name = "_QFcapture_with_convert_i32_to_f64Ev"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
! CHECK: %[[X:.*]] = fir.alloca f64 {bindc_name = "x", uniq_name = "_QFcapture_with_convert_i32_to_f64Ex"}
@@ -142,7 +145,9 @@ end subroutine capture_with_convert_i32_to_f64
! CHECK: hlfir.assign %[[CST]] to %[[X_DECL]]#0 : f64, !fir.ref<f64>
! CHECK: %c0_i32 = arith.constant 0 : i32
! CHECK: hlfir.assign %c0_i32 to %[[V_DECL]]#0 : i32, !fir.ref<i32>
-! CHECK: %[[LOAD:.*]] = fir.load %[[V_DECL]]#0 : !fir.ref<i32>
+! CHECK: %c1_i32 = arith.constant 1 : i32
+! CHECK: hlfir.assign %c1_i32 to %[[U_DECL]]#0 : i32, !fir.ref<i32>
+! CHECK: %[[LOAD:.*]] = fir.load %[[U_DECL]]#0 : !fir.ref<i32>
! CHECK: %[[CONV:.*]] = fir.convert %[[LOAD]] : (i32) -> f64
! CHECK: acc.atomic.capture {
! CHECK: acc.atomic.read %[[V_DECL]]#0 = %[[X_DECL]]#0 : !fir.ref<i32>, !fir.ref<f64>, f64
@@ -155,7 +160,7 @@ subroutine capture_with_convert_f64_to_i32()
x = 1
v = 0
!$acc atomic capture
- x = v * v
+ x = x * 2.0_8
v = x
!$acc end atomic
end subroutine capture_with_convert_f64_to_i32
@@ -167,15 +172,14 @@ end subroutine capture_with_convert_f64_to_i32
! CHECK: %[[X_DECL:.*]]:2 = hlfir.declare %[[X]] {uniq_name = "_QFcapture_with_convert_f64_to_i32Ex"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
! CHECK: %c1_i32 = arith.constant 1 : i32
! CHECK: hlfir.assign %c1_i32 to %[[X_DECL]]#0 : i32, !fir.ref<i32>
-! CHECK: %[[CST:.*]] = arith.constant 0.000000e+00 : f64
-! CHECK: hlfir.assign %[[CST]] to %[[V_DECL]]#0 : f64, !fir.ref<f64>
-! CHECK: %[[LOAD:.*]] = fir.load %[[V_DECL]]#0 : !fir.ref<f64>
+! CHECK: %[[CST:.*]] = arith.constant 2.000000e+00 : f64
! CHECK: acc.atomic.capture {
! CHECK: acc.atomic.update %[[X_DECL]]#0 : !fir.ref<i32> {
-! CHECK: ^bb0(%arg0: i32):
-! CHECK: %[[MUL:.*]] = arith.mulf %[[LOAD]], %[[LOAD]] fastmath<contract> : f64
-! CHECK: %[[CONV:.*]] = fir.convert %[[MUL]] : (f64) -> i32
-! CHECK: acc.yield %[[CONV]] : i32
+! CHECK: ^bb0(%[[ARG:.*]]: i32):
+! CHECK: %[[CONV_ARG:.*]] = fir.convert %[[ARG]] : (i32) -> f64
+! CHECK: %[[MUL:.*]] = arith.mulf %[[CONV_ARG]], %[[CST]] fastmath<contract> : f64
+! CHECK: %[[CONV_MUL:.*]] = fir.convert %[[MUL]] : (f64) -> i32
+! CHECK: acc.yield %[[CONV_MUL]] : i32
! CHECK: }
! CHECK: acc.atomic.read %[[V_DECL]]#0 = %[[X_DECL]]#0 : !fir.ref<f64>, !fir.ref<i32>, i32
! CHECK: }
diff --git a/flang/test/Semantics/OpenACC/acc-atomic-validity.f90 b/flang/test/Semantics/OpenACC/acc-atomic-validity.f90
index 07fb864695737..5c8d33fbbadc7 100644
--- a/flang/test/Semantics/OpenACC/acc-atomic-validity.f90
+++ b/flang/test/Semantics/OpenACC/acc-atomic-validity.f90
@@ -54,11 +54,38 @@ program openacc_atomic_validity
i = c(i)
!$acc end atomic
+ !TODO: Should error because c(i) references i which is the atomic update variable.
!$acc atomic capture
c(i) = i
i = i + 1
!$acc end atomic
+ !ERROR: The variables assigned in this atomic capture construct must be distinct
+ !$acc atomic capture
+ c(1) = c(2)
+ c(1) = c(3)
+ !$acc end atomic
+
+ !ERROR: The assignments in this atomic capture construct do not update a variable and capture either its initial or final value
+ !$acc atomic capture
+ c(1) = c(2)
+ c(2) = c(2)
+ !$acc end atomic
+
+ !ERROR: The assignments in this atomic capture construct do not update a variable and capture either its initial or final value
+ !$acc atomic capture
+ c(1) = c(2)
+ c(2) = c(1)
+ !$acc end atomic
+
+ !ERROR: The assignments in this atomic capture construct do not update a variable and capture either its initial or final value
+ !$acc atomic capture
+ c(1) = c(2)
+ c(3) = c(2)
+ !$acc end atomic
+
+
+
!$acc atomic capture if(l .EQV. .false.)
c(i) = i
i = i + 1
@@ -79,3 +106,45 @@ program openacc_atomic_validity
!$acc end parallel
end program openacc_atomic_validity
+
+subroutine capture_with_convert_f64_to_i32()
+ integer :: x
+ real(8) :: v, w
+ x = 1
+ v = 0
+ w = 2
+
+ !$acc atomic capture
+ x = x * 2.5_8
+ v = x
+ !$acc end atomic
+
+ !$acc atomic capture
+ !TODO: The rhs side of this update statement cannot reference v.
+ x = x * v
+ v = x
+ !$acc end atomic
+
+ !$acc atomic capture
+ !TODO: The rhs side of this update statement cannot reference v.
+ x = v * x
+ v = x
+ !$acc end atomic
+
+ !$acc atomic capture
+ !ERROR: The RHS of this atomic update statement must reference the updated variable: x
+ x = v * v
+ v = x
+ !$acc end atomic
+
+ !$acc atomic capture
+ x = v
+ !ERROR: The updated variable, v, cannot appear more than once in the atomic update operation
+ v = v * v
+ !$acc end atomic
+
+ !$acc atomic capture
+ v = x
+ x = w * w
+ !$acc end atomic
+end subroutine capture_with_convert_f64_to_i32
\ No newline at end of file
More information about the flang-commits
mailing list