[flang][openacc] Add semantic checks for atomic constructs #149579
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
klausler
reviewed
Jul 18, 2025
akuhlens
changed the title
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-semantics Author: Andre Kuhlenschmidt (akuhlens) ChangesAn 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. ! 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 capturePatch is 28.36 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149579.diff 10 Files Affected:
diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h
index 96ed86f468350..43ad8707fa2b8 100644
--- a/flang/include/flang/Evaluate/tools.h
+++ b/flang/include/flang/Evaluate/tools.h
@@ -1512,6 +1512,10 @@ GetTopLevelOperation(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..c34bf8b8ebfca 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -1936,6 +1936,35 @@ 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 {
+ 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;
+};
+
+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/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 9cbea9742a6a2..f18c974ebbe2d 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -7,8 +7,19 @@
//===----------------------------------------------------------------------===//
#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 "llvm/Support/raw_ostream.h"
+#include <optional>
+
+// TODO: Move the parts that I am reusing from openmp-utils.h to
+// evaluate/tools.h
+#include "openmp-utils.h"
#define CHECK_SIMPLE_CLAUSE(X, Y) \
void AccStructureChecker::Enter(const parser::AccClause::X &) { \
@@ -342,21 +353,237 @@ 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, 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 bool IsValidAtomicUpdateOperation(
+ const evaluate::operation::Operator &op) {
+ switch (op) {
+ case evaluate::operation::Operator::Add:
+ case evaluate::operation::Operator::Mul:
+ case evaluate::operation::Operator::Sub:
+ case evaluate::operation::Operator::Div:
+ case evaluate::operation::Operator::And:
+ case evaluate::operation::Operator::Or:
+ case evaluate::operation::Operator::Eqv:
+ case evaluate::operation::Operator::Neqv:
+ // 2909 intrinsic-procedure name is one of max, min, iand, ior, or ieor.
+ case evaluate::operation::Operator::Max:
+ case evaluate::operation::Operator::Min:
+ // Currently all get mapped to And, Or, and Neqv
+ // case evaluate::operation::Operator::Iand:
+ // case evaluate::operation::Operator::Ior:
+ // case evaluate::operation::Operator::Ieor:
+ return true;
+ case evaluate::operation::Operator::Convert:
+ case evaluate::operation::Operator::Identity:
+ default:
+ return false;
+ }
+}
+
+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 || args.size() >= 1) &&
+ "Invalid conversion operation");
+ if (op == evaluate::operation::Operator::Convert && args.size() == 1) {
+ return args[0];
+ }
+ return expr;
+}
+
+void AccStructureChecker::CheckAtomicUpdateStmt(
+ const parser::AssignmentStmt &assign, const SomeExpr &updateVar,
+ const SomeExpr *captureVar) {
+ static std::string construct{"update"};
+ CheckAtomicStmt(assign, construct);
+ 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,
+ construct);
+ } 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) {
+ static std::string construct{"write"};
+ CheckAtomicStmt(assign, construct);
+ 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) {
+ static std::string construct{"capture"};
+ CheckAtomicStmt(assign, construct);
+}
+
+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.
+ }
+ return;
+}
+
+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)};
+ const auto *updateVar{GetExpr(context_, var)};
+ if (!updateVar) {
+ return;
+ }
+ CheckAtomicUpdateStmt(assign, *updateVar, 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)};
+ const auto *updateVar{GetExpr(context_, var)};
+ if (!updateVar) {
+ return;
+ }
+ CheckAtomicWriteStmt(assign, *updateVar, 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)};
+ const auto *captureVar{GetExpr(context_, var)};
+ if (!captureVar) {
+ return;
}
+ CheckAtomicCaptureStmt(assign, nullptr, *captureVar);
}
void AccStructureChecker::Enter(const parser::OpenACCCacheConstruct &x) {
diff --git a/flang/lib/Semantics/check-acc-structure.h b/flang/lib/Semantics/check-acc-structure.h
index 6a9aa01a9bb13..7e00dfca2d746 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, 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..b20de7adf9385 100644
--- a/flang/lib/Semantics/check-omp-atomic.cpp
+++ b/flang/lib/Semantics/check-omp-atomic.cpp
@@ -399,8 +399,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?
@@ -657,7 +657,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);
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index f43d2cc75620e..a50ca8149d3fd 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -245,28 +245,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);
}
@@ -355,10 +333,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 a96c008fb26e7..5f25f9ef28372 100644
--- a/flang/lib/Semantics/openmp-utils.h
+++ b/flang/lib/Semantics/openmp-utils.h
@@ -69,7 +69,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...
[truncated]
|
|
@llvm/pr-subscribers-openacc Author: Andre Kuhlenschmidt (akuhlens) ChangesAn 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. ! 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 capturePatch is 28.36 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149579.diff 10 Files Affected:
diff --git a/flang/include/flang/Evaluate/tools.h b/flang/include/flang/Evaluate/tools.h
index 96ed86f468350..43ad8707fa2b8 100644
--- a/flang/include/flang/Evaluate/tools.h
+++ b/flang/include/flang/Evaluate/tools.h
@@ -1512,6 +1512,10 @@ GetTopLevelOperation(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..c34bf8b8ebfca 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -1936,6 +1936,35 @@ 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 {
+ 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;
+};
+
+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/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 9cbea9742a6a2..f18c974ebbe2d 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -7,8 +7,19 @@
//===----------------------------------------------------------------------===//
#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 "llvm/Support/raw_ostream.h"
+#include <optional>
+
+// TODO: Move the parts that I am reusing from openmp-utils.h to
+// evaluate/tools.h
+#include "openmp-utils.h"
#define CHECK_SIMPLE_CLAUSE(X, Y) \
void AccStructureChecker::Enter(const parser::AccClause::X &) { \
@@ -342,21 +353,237 @@ 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, 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 bool IsValidAtomicUpdateOperation(
+ const evaluate::operation::Operator &op) {
+ switch (op) {
+ case evaluate::operation::Operator::Add:
+ case evaluate::operation::Operator::Mul:
+ case evaluate::operation::Operator::Sub:
+ case evaluate::operation::Operator::Div:
+ case evaluate::operation::Operator::And:
+ case evaluate::operation::Operator::Or:
+ case evaluate::operation::Operator::Eqv:
+ case evaluate::operation::Operator::Neqv:
+ // 2909 intrinsic-procedure name is one of max, min, iand, ior, or ieor.
+ case evaluate::operation::Operator::Max:
+ case evaluate::operation::Operator::Min:
+ // Currently all get mapped to And, Or, and Neqv
+ // case evaluate::operation::Operator::Iand:
+ // case evaluate::operation::Operator::Ior:
+ // case evaluate::operation::Operator::Ieor:
+ return true;
+ case evaluate::operation::Operator::Convert:
+ case evaluate::operation::Operator::Identity:
+ default:
+ return false;
+ }
+}
+
+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 || args.size() >= 1) &&
+ "Invalid conversion operation");
+ if (op == evaluate::operation::Operator::Convert && args.size() == 1) {
+ return args[0];
+ }
+ return expr;
+}
+
+void AccStructureChecker::CheckAtomicUpdateStmt(
+ const parser::AssignmentStmt &assign, const SomeExpr &updateVar,
+ const SomeExpr *captureVar) {
+ static std::string construct{"update"};
+ CheckAtomicStmt(assign, construct);
+ 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,
+ construct);
+ } 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) {
+ static std::string construct{"write"};
+ CheckAtomicStmt(assign, construct);
+ 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) {
+ static std::string construct{"capture"};
+ CheckAtomicStmt(assign, construct);
+}
+
+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.
+ }
+ return;
+}
+
+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)};
+ const auto *updateVar{GetExpr(context_, var)};
+ if (!updateVar) {
+ return;
+ }
+ CheckAtomicUpdateStmt(assign, *updateVar, 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)};
+ const auto *updateVar{GetExpr(context_, var)};
+ if (!updateVar) {
+ return;
+ }
+ CheckAtomicWriteStmt(assign, *updateVar, 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)};
+ const auto *captureVar{GetExpr(context_, var)};
+ if (!captureVar) {
+ return;
}
+ CheckAtomicCaptureStmt(assign, nullptr, *captureVar);
}
void AccStructureChecker::Enter(const parser::OpenACCCacheConstruct &x) {
diff --git a/flang/lib/Semantics/check-acc-structure.h b/flang/lib/Semantics/check-acc-structure.h
index 6a9aa01a9bb13..7e00dfca2d746 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, 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..b20de7adf9385 100644
--- a/flang/lib/Semantics/check-omp-atomic.cpp
+++ b/flang/lib/Semantics/check-omp-atomic.cpp
@@ -399,8 +399,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?
@@ -657,7 +657,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);
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index f43d2cc75620e..a50ca8149d3fd 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -245,28 +245,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);
}
@@ -355,10 +333,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 a96c008fb26e7..5f25f9ef28372 100644
--- a/flang/lib/Semantics/openmp-utils.h
+++ b/flang/lib/Semantics/openmp-utils.h
@@ -69,7 +69,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...
[truncated]
|
klausler
reviewed
Jul 28, 2025
akuhlens
force-pushed
the
andre/atomic-capture
branch
from
d5e8597 to
c4e8970
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.