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; + std::string ToString(Operator op); template @@ -1509,9 +1512,18 @@ Operator OperationCode(const evaluate::ProcedureDesignator &proc); std::pair>> GetTopLevelOperation(const Expr &expr); +// Return information about the top-level operation (ignoring parentheses, and +// resizing converts) +std::pair>> +GetTopLevelOperationIgnoreResizing(const Expr &expr); + // Check if expr is same as x, or a sequence of Convert operations on x. bool IsSameOrConvertOf(const Expr &expr, const Expr &x); +// Check if the Variable appears as a subexpression of the expression. +bool IsVarSubexpressionOf( + const Expr &var, const Expr &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>> -GetTopLevelOperation(const Expr &expr) { +GetTopLevelOperationIgnoreResizing(const Expr &expr) { return operation::ArgumentExtractor{}(expr); } +std::pair>> +GetTopLevelOperation(const Expr &expr) { + return operation::ArgumentExtractor{}(expr); +} + namespace operation { struct ConvertCollector : public Traverse &expr, const Expr &x) { return false; } } + +struct VariableFinder : public evaluate::AnyTraverse { + using Base = evaluate::AnyTraverse; + using SomeExpr = Expr; + VariableFinder(const SomeExpr &v) : Base(*this), var(v) {} + + using Base::operator(); + + template + bool operator()(const evaluate::Designator &x) const { + return evaluate::AsGenericExpr(common::Clone(x)) == var; + } + + template + bool operator()(const evaluate::FunctionRef &x) const { + return evaluate::AsGenericExpr(common::Clone(x)) == var; + } + +private: + const SomeExpr &var; +}; + +bool IsVarSubexpressionOf( + const Expr &sub, const Expr &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 #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>(x.t).statement}; - const auto &var{std::get(assignment.t)}; - const auto &expr{std::get(assignment.t)}; +void AccStructureChecker::CheckAtomicStmt( + const parser::AssignmentStmt &assign, const std::string &construct) { + const auto &var{std::get(assign.t)}; + const auto &expr{std::get(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(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(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(capture.t) + .v.statement}; + const Fortran::parser::AssignmentStmt &stmt2{ + std::get(capture.t) + .v.statement}; + const auto &var1{std::get(stmt1.t)}; + const auto &var2{std::get(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(capture.t).source, + "The variables assigned in this atomic capture construct must be distinct"_err_en_US); + return; + } + const auto &expr1{std::get(stmt1.t)}; + const auto &expr2{std::get(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(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 = as capture; update + const auto &updateVar{*lhs2}; + const auto &captureVar{*lhs1}; + CheckAtomicCaptureStmt(stmt1, &updateVar, captureVar); + CheckAtomicUpdateStmt(stmt2, updateVar, &captureVar); + } else { + // Take v = x; 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 = ; 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(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 = ; b = ; Doesn't fit the spec + context_.Say(std::get(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>(x.t).statement}; + const auto &var{std::get(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>(x.t).statement}; + const auto &var{std::get(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>(x.t).statement}; + const auto &var{std::get(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 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> 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 { - using Base = evaluate::AnyTraverse; - VariableFinder(const SomeExpr &v) : Base(*this), var(v) {} - - using Base::operator(); - - template - bool operator()(const evaluate::Designator &x) const { - auto copy{x}; - return evaluate::AsGenericExpr(std::move(copy)) == var; - } - - template - bool operator()(const evaluate::FunctionRef &x) const { - auto copy{x}; - return evaluate::AsGenericExpr(std::move(copy)) == var; - } - -private: - const SomeExpr &var; -}; - std::vector 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 IsContiguous( std::vector GetAllDesignators(const SomeExpr &expr); const SomeExpr *HasStorageOverlap( const SomeExpr &base, llvm::ArrayRef 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) -> (!fir.ref, !fir.ref) ! 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) -> (!fir.ref, !fir.ref) ! 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 ! CHECK: %c0_i32 = arith.constant 0 : i32 ! CHECK: hlfir.assign %c0_i32 to %[[V_DECL]]#0 : i32, !fir.ref -! CHECK: %[[LOAD:.*]] = fir.load %[[V_DECL]]#0 : !fir.ref +! CHECK: %c1_i32 = arith.constant 1 : i32 +! CHECK: hlfir.assign %c1_i32 to %[[U_DECL]]#0 : i32, !fir.ref +! CHECK: %[[LOAD:.*]] = fir.load %[[U_DECL]]#0 : !fir.ref ! CHECK: %[[CONV:.*]] = fir.convert %[[LOAD]] : (i32) -> f64 ! CHECK: acc.atomic.capture { ! CHECK: acc.atomic.read %[[V_DECL]]#0 = %[[X_DECL]]#0 : !fir.ref, !fir.ref, 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) -> (!fir.ref, !fir.ref) ! CHECK: %c1_i32 = arith.constant 1 : i32 ! CHECK: hlfir.assign %c1_i32 to %[[X_DECL]]#0 : i32, !fir.ref -! CHECK: %[[CST:.*]] = arith.constant 0.000000e+00 : f64 -! CHECK: hlfir.assign %[[CST]] to %[[V_DECL]]#0 : f64, !fir.ref -! CHECK: %[[LOAD:.*]] = fir.load %[[V_DECL]]#0 : !fir.ref +! CHECK: %[[CST:.*]] = arith.constant 2.000000e+00 : f64 ! CHECK: acc.atomic.capture { ! CHECK: acc.atomic.update %[[X_DECL]]#0 : !fir.ref { -! CHECK: ^bb0(%arg0: i32): -! CHECK: %[[MUL:.*]] = arith.mulf %[[LOAD]], %[[LOAD]] fastmath : 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 : 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, !fir.ref, 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