[flang-commits] [flang] [llvm] [flang][OpenMP] Overhaul implementation of ATOMIC construct (PR #137852)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Wed Jun 4 06:10:13 PDT 2025
================
@@ -2666,422 +2673,1391 @@ void OmpStructureChecker::Leave(const parser::OmpEndBlockDirective &x) {
}
}
-inline void OmpStructureChecker::ErrIfAllocatableVariable(
- const parser::Variable &var) {
- // Err out if the given symbol has
- // ALLOCATABLE attribute
- if (const auto *e{GetExpr(context_, var)})
- for (const Symbol &symbol : evaluate::CollectSymbols(*e))
- if (IsAllocatable(symbol)) {
- const auto &designator =
- std::get<common::Indirection<parser::Designator>>(var.u);
- const auto *dataRef =
- std::get_if<parser::DataRef>(&designator.value().u);
- const parser::Name *name =
- dataRef ? std::get_if<parser::Name>(&dataRef->u) : nullptr;
- if (name)
- context_.Say(name->source,
- "%s must not have ALLOCATABLE "
- "attribute"_err_en_US,
- name->ToString());
+/// parser::Block is a list of executable constructs, parser::BlockConstruct
+/// is Fortran's BLOCK/ENDBLOCK construct.
+/// Strip the outermost BlockConstructs, return the reference to the Block
+/// in the executable part of the innermost of the stripped constructs.
+/// Specifically, if the given `block` has a single entry (it's a list), and
+/// the entry is a BlockConstruct, get the Block contained within. Repeat
+/// this step as many times as possible.
+static const parser::Block &GetInnermostExecPart(const parser::Block &block) {
+ const parser::Block *iter{&block};
+ while (iter->size() == 1) {
+ const parser::ExecutionPartConstruct &ep{iter->front()};
+ if (auto *exec{std::get_if<parser::ExecutableConstruct>(&ep.u)}) {
+ using BlockConstruct = common::Indirection<parser::BlockConstruct>;
+ if (auto *bc{std::get_if<BlockConstruct>(&exec->u)}) {
+ iter = &std::get<parser::Block>(bc->value().t);
+ continue;
}
+ }
+ break;
+ }
+ return *iter;
}
-inline void OmpStructureChecker::ErrIfLHSAndRHSSymbolsMatch(
- const parser::Variable &var, const parser::Expr &expr) {
- // Err out if the symbol on the LHS is also used on the RHS of the assignment
- // statement
- const auto *e{GetExpr(context_, expr)};
- const auto *v{GetExpr(context_, var)};
- if (e && v) {
- auto vSyms{evaluate::GetSymbolVector(*v)};
- const Symbol &varSymbol = vSyms.front();
- for (const Symbol &symbol : evaluate::GetSymbolVector(*e)) {
- if (varSymbol == symbol) {
- const common::Indirection<parser::Designator> *designator =
- std::get_if<common::Indirection<parser::Designator>>(&expr.u);
- if (designator) {
- auto *z{var.typedExpr.get()};
- auto *c{expr.typedExpr.get()};
- if (z->v == c->v) {
- context_.Say(expr.source,
- "RHS expression on atomic assignment statement cannot access '%s'"_err_en_US,
- var.GetSource());
- }
+// There is no consistent way to get the source of a given ActionStmt, so
+// extract the source information from Statement<ActionStmt> when we can,
+// and keep it around for error reporting in further analyses.
+struct SourcedActionStmt {
+ const parser::ActionStmt *stmt{nullptr};
+ parser::CharBlock source;
+
+ operator bool() const { return stmt != nullptr; }
+};
+
+struct AnalyzedCondStmt {
+ SomeExpr cond{evaluate::NullPointer{}}; // Default ctor is deleted
+ parser::CharBlock source;
+ SourcedActionStmt ift, iff;
+};
+
+static SourcedActionStmt GetActionStmt(
+ const parser::ExecutionPartConstruct *x) {
+ if (x == nullptr) {
+ return SourcedActionStmt{};
+ }
+ if (auto *exec{std::get_if<parser::ExecutableConstruct>(&x->u)}) {
+ using ActionStmt = parser::Statement<parser::ActionStmt>;
+ if (auto *stmt{std::get_if<ActionStmt>(&exec->u)}) {
+ return SourcedActionStmt{&stmt->statement, stmt->source};
+ }
+ }
+ return SourcedActionStmt{};
+}
+
+static SourcedActionStmt GetActionStmt(const parser::Block &block) {
+ if (block.size() == 1) {
+ return GetActionStmt(&block.front());
+ }
+ return SourcedActionStmt{};
+}
+
+// Compute the `evaluate::Assignment` from parser::ActionStmt. The assumption
+// is that the ActionStmt will be either an assignment or a pointer-assignment,
+// otherwise return std::nullopt.
+// Note: This function can return std::nullopt on [Pointer]AssignmentStmt where
+// the "typedAssignment" is unset. This can happen if there are semantic errors
+// in the purported assignment.
+static std::optional<evaluate::Assignment> GetEvaluateAssignment(
+ const parser::ActionStmt *x) {
+ if (x == nullptr) {
+ return std::nullopt;
+ }
+
+ using AssignmentStmt = common::Indirection<parser::AssignmentStmt>;
+ using PointerAssignmentStmt =
+ common::Indirection<parser::PointerAssignmentStmt>;
+ using TypedAssignment = parser::AssignmentStmt::TypedAssignment;
+
+ return common::visit(
+ [](auto &&s) -> std::optional<evaluate::Assignment> {
+ using BareS = llvm::remove_cvref_t<decltype(s)>;
+ if constexpr (std::is_same_v<BareS, AssignmentStmt> ||
+ std::is_same_v<BareS, PointerAssignmentStmt>) {
+ const TypedAssignment &typed{s.value().typedAssignment};
+ // ForwardOwningPointer typedAssignment
+ // `- GenericAssignmentWrapper ^.get()
+ // `- std::optional<Assignment> ^->v
+ return typed.get()->v;
} else {
- context_.Say(expr.source,
- "RHS expression on atomic assignment statement cannot access '%s'"_err_en_US,
- var.GetSource());
+ return std::nullopt;
}
+ },
+ x->u);
+}
+
+// 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
+// checks), and where the source has some other statement (which we want
+// to report as "should be an assignment").
+static bool IsAssignment(const parser::ActionStmt *x) {
+ if (x == nullptr) {
+ return false;
+ }
+
+ using AssignmentStmt = common::Indirection<parser::AssignmentStmt>;
+ using PointerAssignmentStmt =
+ common::Indirection<parser::PointerAssignmentStmt>;
+
+ return common::visit(
+ [](auto &&s) -> bool {
+ using BareS = llvm::remove_cvref_t<decltype(s)>;
+ return std::is_same_v<BareS, AssignmentStmt> ||
+ std::is_same_v<BareS, PointerAssignmentStmt>;
+ },
+ x->u);
+}
+
+static std::optional<AnalyzedCondStmt> AnalyzeConditionalStmt(
+ const parser::ExecutionPartConstruct *x) {
+ if (x == nullptr) {
+ return std::nullopt;
+ }
+
+ // Extract the evaluate::Expr from ScalarLogicalExpr.
+ auto getFromLogical{[](const parser::ScalarLogicalExpr &logical) {
+ // ScalarLogicalExpr is Scalar<Logical<common::Indirection<Expr>>>
+ const parser::Expr &expr{logical.thing.thing.value()};
+ return GetEvaluateExpr(expr);
+ }};
+
+ // Recognize either
+ // ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> IfStmt, or
+ // ExecutionPartConstruct -> ExecutableConstruct -> IfConstruct.
+
+ if (auto &&action{GetActionStmt(x)}) {
+ if (auto *ifs{std::get_if<common::Indirection<parser::IfStmt>>(
+ &action.stmt->u)}) {
+ const parser::IfStmt &s{ifs->value()};
+ auto &&maybeCond{
+ getFromLogical(std::get<parser::ScalarLogicalExpr>(s.t))};
+ auto &thenStmt{
+ std::get<parser::UnlabeledStatement<parser::ActionStmt>>(s.t)};
+ if (maybeCond) {
+ return AnalyzedCondStmt{std::move(*maybeCond), action.source,
+ SourcedActionStmt{&thenStmt.statement, thenStmt.source},
+ SourcedActionStmt{}};
}
}
+ return std::nullopt;
}
+
+ if (auto *exec{std::get_if<parser::ExecutableConstruct>(&x->u)}) {
+ if (auto *ifc{
+ std::get_if<common::Indirection<parser::IfConstruct>>(&exec->u)}) {
+ using ElseBlock = parser::IfConstruct::ElseBlock;
+ using ElseIfBlock = parser::IfConstruct::ElseIfBlock;
+ const parser::IfConstruct &s{ifc->value()};
+
+ if (!std::get<std::list<ElseIfBlock>>(s.t).empty()) {
+ // Not expecting any else-if statements.
+ return std::nullopt;
+ }
+ auto &stmt{std::get<parser::Statement<parser::IfThenStmt>>(s.t)};
+ auto &&maybeCond{getFromLogical(
+ std::get<parser::ScalarLogicalExpr>(stmt.statement.t))};
+ if (!maybeCond) {
+ return std::nullopt;
+ }
+
+ if (auto &maybeElse{std::get<std::optional<ElseBlock>>(s.t)}) {
+ AnalyzedCondStmt result{std::move(*maybeCond), stmt.source,
+ GetActionStmt(std::get<parser::Block>(s.t)),
+ GetActionStmt(std::get<parser::Block>(maybeElse->t))};
+ if (result.ift.stmt && result.iff.stmt) {
+ return result;
+ }
+ } else {
+ AnalyzedCondStmt result{std::move(*maybeCond), stmt.source,
+ GetActionStmt(std::get<parser::Block>(s.t)), SourcedActionStmt{}};
+ if (result.ift.stmt) {
+ return result;
+ }
+ }
+ }
+ return std::nullopt;
+ }
+
+ return std::nullopt;
}
-inline void OmpStructureChecker::ErrIfNonScalarAssignmentStmt(
- const parser::Variable &var, const parser::Expr &expr) {
- // Err out if either the variable on the LHS or the expression on the RHS of
- // the assignment statement are non-scalar (i.e. have rank > 0 or is of
- // CHARACTER type)
- const auto *e{GetExpr(context_, expr)};
- const auto *v{GetExpr(context_, var)};
- if (e && v) {
- if (e->Rank() != 0 ||
- (e->GetType().has_value() &&
- e->GetType().value().category() == common::TypeCategory::Character))
- context_.Say(expr.source,
- "Expected scalar expression "
- "on the RHS of atomic assignment "
- "statement"_err_en_US);
- if (v->Rank() != 0 ||
- (v->GetType().has_value() &&
- v->GetType()->category() == common::TypeCategory::Character))
- context_.Say(var.GetSource(),
- "Expected scalar variable "
- "on the LHS of atomic assignment "
- "statement"_err_en_US);
- }
-}
-
-template <typename T, typename D>
-bool OmpStructureChecker::IsOperatorValid(const T &node, const D &variable) {
- using AllowedBinaryOperators =
- std::variant<parser::Expr::Add, parser::Expr::Multiply,
- parser::Expr::Subtract, parser::Expr::Divide, parser::Expr::AND,
- parser::Expr::OR, parser::Expr::EQV, parser::Expr::NEQV>;
- using BinaryOperators = std::variant<parser::Expr::Add,
- parser::Expr::Multiply, parser::Expr::Subtract, parser::Expr::Divide,
- parser::Expr::AND, parser::Expr::OR, parser::Expr::EQV,
- parser::Expr::NEQV, parser::Expr::Power, parser::Expr::Concat,
- parser::Expr::LT, parser::Expr::LE, parser::Expr::EQ, parser::Expr::NE,
- parser::Expr::GE, parser::Expr::GT>;
-
- if constexpr (common::HasMember<T, BinaryOperators>) {
- const auto &variableName{variable.GetSource().ToString()};
- const auto &exprLeft{std::get<0>(node.t)};
- const auto &exprRight{std::get<1>(node.t)};
- if ((exprLeft.value().source.ToString() != variableName) &&
- (exprRight.value().source.ToString() != variableName)) {
- context_.Say(variable.GetSource(),
- "Atomic update statement should be of form "
- "`%s = %s operator expr` OR `%s = expr operator %s`"_err_en_US,
- variableName, variableName, variableName, variableName);
- }
- return common::HasMember<T, AllowedBinaryOperators>;
+static std::pair<parser::CharBlock, parser::CharBlock> SplitAssignmentSource(
+ parser::CharBlock source) {
+ // Find => in the range, if not found, find = that is not a part of
+ // <=, >=, ==, or /=.
+ auto trim{[](std::string_view v) {
+ const char *begin{v.data()};
+ const char *end{begin + v.size()};
+ while (*begin == ' ' && begin != end) {
+ ++begin;
+ }
+ while (begin != end && end[-1] == ' ') {
+ --end;
+ }
+ assert(begin != end && "Source should not be empty");
+ return parser::CharBlock(begin, end - begin);
+ }};
+
+ std::string_view sv(source.begin(), source.size());
+
+ if (auto where{sv.find("=>")}; where != sv.npos) {
+ std::string_view lhs(sv.data(), where);
+ std::string_view rhs(sv.data() + where + 2, sv.size() - where - 2);
+ return std::make_pair(trim(lhs), trim(rhs));
}
- return false;
+
+ // Go backwards, since all the exclusions above end with a '='.
+ for (size_t next{source.size()}; next > 1; --next) {
+ if (sv[next - 1] == '=' && !llvm::is_contained("<>=/", sv[next - 2])) {
+ std::string_view lhs(sv.data(), next - 1);
+ std::string_view rhs(sv.data() + next, sv.size() - next);
+ return std::make_pair(trim(lhs), trim(rhs));
+ }
+ }
+ llvm_unreachable("Could not find assignment operator");
}
-void OmpStructureChecker::CheckAtomicCaptureStmt(
- const parser::AssignmentStmt &assignmentStmt) {
- const auto &var{std::get<parser::Variable>(assignmentStmt.t)};
- const auto &expr{std::get<parser::Expr>(assignmentStmt.t)};
- common::visit(
- common::visitors{
- [&](const common::Indirection<parser::Designator> &designator) {
- const auto *dataRef =
- std::get_if<parser::DataRef>(&designator.value().u);
- const auto *name =
- dataRef ? std::get_if<parser::Name>(&dataRef->u) : nullptr;
- if (name && IsAllocatable(*name->symbol))
- context_.Say(name->source,
- "%s must not have ALLOCATABLE "
- "attribute"_err_en_US,
- name->ToString());
- },
- [&](const auto &) {
- // Anything other than a `parser::Designator` is not allowed
- context_.Say(expr.source,
- "Expected scalar variable "
- "of intrinsic type on RHS of atomic "
- "assignment statement"_err_en_US);
- }},
- expr.u);
- ErrIfLHSAndRHSSymbolsMatch(var, expr);
- ErrIfNonScalarAssignmentStmt(var, expr);
-}
-
-void OmpStructureChecker::CheckAtomicWriteStmt(
- const parser::AssignmentStmt &assignmentStmt) {
- const auto &var{std::get<parser::Variable>(assignmentStmt.t)};
- const auto &expr{std::get<parser::Expr>(assignmentStmt.t)};
- ErrIfAllocatableVariable(var);
- ErrIfLHSAndRHSSymbolsMatch(var, expr);
- ErrIfNonScalarAssignmentStmt(var, expr);
-}
-
-void OmpStructureChecker::CheckAtomicUpdateStmt(
- const parser::AssignmentStmt &assignment) {
- const auto &expr{std::get<parser::Expr>(assignment.t)};
- const auto &var{std::get<parser::Variable>(assignment.t)};
- bool isIntrinsicProcedure{false};
- bool isValidOperator{false};
- common::visit(
- common::visitors{
- [&](const common::Indirection<parser::FunctionReference> &x) {
- isIntrinsicProcedure = true;
- const auto &procedureDesignator{
- std::get<parser::ProcedureDesignator>(x.value().v.t)};
- const parser::Name *name{
- std::get_if<parser::Name>(&procedureDesignator.u)};
- if (name &&
- !(name->source == "max" || name->source == "min" ||
- name->source == "iand" || name->source == "ior" ||
- name->source == "ieor")) {
- context_.Say(expr.source,
- "Invalid intrinsic procedure name in "
- "OpenMP ATOMIC (UPDATE) statement"_err_en_US);
- }
- },
- [&](const auto &x) {
- if (!IsOperatorValid(x, var)) {
- context_.Say(expr.source,
- "Invalid or missing operator in atomic update "
- "statement"_err_en_US);
- } else
- isValidOperator = true;
- },
- },
- expr.u);
- if (const auto *e{GetExpr(context_, expr)}) {
- const auto *v{GetExpr(context_, var)};
- if (e->Rank() != 0 ||
- (e->GetType().has_value() &&
- e->GetType().value().category() == common::TypeCategory::Character))
- context_.Say(expr.source,
- "Expected scalar expression "
- "on the RHS of atomic update assignment "
- "statement"_err_en_US);
- if (v->Rank() != 0 ||
- (v->GetType().has_value() &&
- v->GetType()->category() == common::TypeCategory::Character))
- context_.Say(var.GetSource(),
- "Expected scalar variable "
- "on the LHS of atomic update assignment "
- "statement"_err_en_US);
- auto vSyms{evaluate::GetSymbolVector(*v)};
- const Symbol &varSymbol = vSyms.front();
- int numOfSymbolMatches{0};
- SymbolVector exprSymbols{evaluate::GetSymbolVector(*e)};
- for (const Symbol &symbol : exprSymbols) {
- if (varSymbol == symbol) {
- numOfSymbolMatches++;
+namespace atomic {
+
+struct DesignatorCollector : public evaluate::Traverse<DesignatorCollector,
+ std::vector<SomeExpr>, false> {
+ using Result = std::vector<SomeExpr>;
+ using Base = evaluate::Traverse<DesignatorCollector, Result, false>;
+ DesignatorCollector() : Base(*this) {}
+
+ Result Default() const { return {}; }
+
+ using Base::operator();
+
+ template <typename T> //
+ Result operator()(const evaluate::Designator<T> &x) const {
+ // Once in a designator, don't traverse it any further (i.e. only
+ // collect top-level designators).
+ auto copy{x};
+ return Result{AsGenericExpr(std::move(copy))};
+ }
+
+ template <typename... Rs> //
+ Result Combine(Result &&result, Rs &&...results) const {
+ Result v(std::move(result));
+ auto moveAppend{[](auto &accum, auto &&other) {
+ for (auto &&s : other) {
+ accum.push_back(std::move(s));
}
+ }};
+ (moveAppend(v, std::move(results)), ...);
+ return v;
+ }
+};
+
+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;
+};
+} // namespace atomic
+
+static bool IsAllocatable(const SomeExpr &expr) {
+ std::vector<SomeExpr> dsgs{atomic::DesignatorCollector{}(expr)};
+ assert(dsgs.size() == 1 && "Should have a single top-level designator");
+ evaluate::SymbolVector syms{evaluate::GetSymbolVector(dsgs.front())};
+ return !syms.empty() && IsAllocatable(syms.back()) && !IsArrayElement(expr);
+}
+
+static bool IsPointerAssignment(const evaluate::Assignment &x) {
+ return std::holds_alternative<evaluate::Assignment::BoundsSpec>(x.u) ||
+ std::holds_alternative<evaluate::Assignment::BoundsRemapping>(x.u);
+}
+
+static bool IsCheckForAssociated(const SomeExpr &cond) {
+ return GetTopLevelOperation(cond).first == operation::Operator::Associated;
+}
+
+static bool HasCommonDesignatorSymbols(
+ const evaluate::SymbolVector &baseSyms, const SomeExpr &other) {
+ // Compare the designators used in "other" with the designators whose
+ // symbols are given in baseSyms.
+ // This is a part of the check if these two expressions can access the same
+ // storage: if the designators used in them are different enough, then they
+ // will be assumed not to access the same memory.
+ //
+ // Consider an (array element) expression x%y(w%z), the corresponding symbol
+ // vector will be {x, y, w, z} (i.e. the symbols for these names).
+ // Check whether this exact sequence appears anywhere in any the symbol
+ // vector for "other". This will be true for x(y) and x(y+1), so this is
+ // not a sufficient condition, but can be used to eliminate candidates
+ // before doing more exhaustive checks.
+ //
+ // If any of the symbols in this sequence are function names, assume that
+ // there is no storage overlap, mostly because it would be impossible in
+ // general to determine what storage the function will access.
+ // Note: if f is pure, then two calls to f will access the same storage
+ // when called with the same arguments. This check is not done yet.
+
+ if (llvm::any_of(
+ baseSyms, [](const SymbolRef &s) { return s->IsSubprogram(); })) {
+ // If there is a function symbol in the chain then we can't infer much
+ // about the accessed storage.
+ return false;
+ }
+
+ auto isSubsequence{// Is u a subsequence of v.
+ [](const evaluate::SymbolVector &u, const evaluate::SymbolVector &v) {
+ size_t us{u.size()}, vs{v.size()};
+ if (us > vs) {
+ return false;
+ }
+ for (size_t off{0}; off != vs - us + 1; ++off) {
+ bool same{true};
+ for (size_t i{0}; i != us; ++i) {
+ if (u[i] != v[off + i]) {
+ same = false;
+ break;
+ }
+ }
+ if (same) {
+ return true;
+ }
+ }
+ return false;
+ }};
+
+ evaluate::SymbolVector otherSyms{evaluate::GetSymbolVector(other)};
+ return isSubsequence(baseSyms, otherSyms);
+}
+
+static bool HasCommonTopLevelDesignators(
+ const std::vector<SomeExpr> &baseDsgs, const SomeExpr &other) {
+ // Compare designators directly as expressions. This will ensure
+ // that x(y) and x(y+1) are not flagged as overlapping, whereas
+ // the symbol vectors for both of these would be identical.
+ std::vector<SomeExpr> otherDsgs{atomic::DesignatorCollector{}(other)};
+
+ for (auto &s : baseDsgs) {
+ if (llvm::any_of(otherDsgs, [&](auto &&t) { return s == t; })) {
+ return true;
}
- if (isIntrinsicProcedure) {
- std::string varName = var.GetSource().ToString();
- if (numOfSymbolMatches != 1)
- context_.Say(expr.source,
- "Intrinsic procedure"
- " arguments in atomic update statement"
- " must have exactly one occurence of '%s'"_err_en_US,
- varName);
- else if (varSymbol != exprSymbols.front() &&
- varSymbol != exprSymbols.back())
- context_.Say(expr.source,
- "Atomic update statement "
- "should be of the form `%s = intrinsic_procedure(%s, expr_list)` "
- "OR `%s = intrinsic_procedure(expr_list, %s)`"_err_en_US,
- varName, varName, varName, varName);
- } else if (isValidOperator) {
- if (numOfSymbolMatches != 1)
- context_.Say(expr.source,
- "Exactly one occurence of '%s' "
- "expected on the RHS of atomic update assignment statement"_err_en_US,
- var.GetSource().ToString());
+ }
+ return false;
+}
+
+static const SomeExpr *HasStorageOverlap(
+ const SomeExpr &base, llvm::ArrayRef<SomeExpr> exprs) {
+ evaluate::SymbolVector baseSyms{evaluate::GetSymbolVector(base)};
+ std::vector<SomeExpr> baseDsgs{atomic::DesignatorCollector{}(base)};
+
+ for (const SomeExpr &expr : exprs) {
+ if (!HasCommonDesignatorSymbols(baseSyms, expr)) {
+ continue;
+ }
+ if (HasCommonTopLevelDesignators(baseDsgs, expr)) {
+ return &expr;
}
}
+ return nullptr;
+}
- ErrIfAllocatableVariable(var);
+static bool IsMaybeAtomicWrite(const evaluate::Assignment &assign) {
+ // This ignores function calls, so it will accept "f(x) = f(x) + 1"
+ // for example.
+ return HasStorageOverlap(assign.lhs, assign.rhs) == nullptr;
}
-void OmpStructureChecker::CheckAtomicCompareConstruct(
- const parser::OmpAtomicCompare &atomicCompareConstruct) {
+static bool IsSubexpressionOf(const SomeExpr &sub, const SomeExpr &super) {
+ return atomic::VariableFinder{sub}(super);
+}
- // TODO: Check that the if-stmt is `if (var == expr) var = new`
- // [with or without then/end-do]
+static void SetExpr(parser::TypedExpr &expr, MaybeExpr value) {
+ if (value) {
+ expr.Reset(new evaluate::GenericExprWrapper(std::move(value)),
+ evaluate::GenericExprWrapper::Deleter);
+ }
+}
- unsigned version{context_.langOptions().OpenMPVersion};
- if (version < 51) {
- context_.Say(atomicCompareConstruct.source,
- "%s construct not allowed in %s, %s"_err_en_US,
- atomicCompareConstruct.source, ThisVersion(version), TryVersion(51));
- }
-
- // TODO: More work needed here. Some of the Update restrictions need to
- // be added, but Update isn't the same either.
-}
-
-// TODO: Allow cond-update-stmt once compare clause is supported.
-void OmpStructureChecker::CheckAtomicCaptureConstruct(
- const parser::OmpAtomicCapture &atomicCaptureConstruct) {
- const parser::AssignmentStmt &stmt1 =
- std::get<parser::OmpAtomicCapture::Stmt1>(atomicCaptureConstruct.t)
- .v.statement;
- const auto &stmt1Var{std::get<parser::Variable>(stmt1.t)};
- const auto &stmt1Expr{std::get<parser::Expr>(stmt1.t)};
- const auto *v1 = GetExpr(context_, stmt1Var);
- const auto *e1 = GetExpr(context_, stmt1Expr);
-
- const parser::AssignmentStmt &stmt2 =
- std::get<parser::OmpAtomicCapture::Stmt2>(atomicCaptureConstruct.t)
- .v.statement;
- const auto &stmt2Var{std::get<parser::Variable>(stmt2.t)};
- const auto &stmt2Expr{std::get<parser::Expr>(stmt2.t)};
- const auto *v2 = GetExpr(context_, stmt2Var);
- const auto *e2 = GetExpr(context_, stmt2Expr);
-
- if (e1 && v1 && e2 && v2) {
- if (semantics::checkForSingleVariableOnRHS(stmt1)) {
- CheckAtomicCaptureStmt(stmt1);
- if (semantics::checkForSymbolMatch(v2, e2)) {
- // ATOMIC CAPTURE construct is of the form [capture-stmt, update-stmt]
- CheckAtomicUpdateStmt(stmt2);
+static void SetAssignment(parser::AssignmentStmt::TypedAssignment &assign,
+ std::optional<evaluate::Assignment> value) {
+ if (value) {
+ assign.Reset(new evaluate::GenericAssignmentWrapper(std::move(value)),
+ evaluate::GenericAssignmentWrapper::Deleter);
+ }
+}
+
+static parser::OpenMPAtomicConstruct::Analysis::Op MakeAtomicAnalysisOp(
+ int what,
+ const std::optional<evaluate::Assignment> &maybeAssign = std::nullopt) {
+ parser::OpenMPAtomicConstruct::Analysis::Op operation;
+ operation.what = what;
+ SetAssignment(operation.assign, maybeAssign);
+ return operation;
+}
+
+static parser::OpenMPAtomicConstruct::Analysis MakeAtomicAnalysis(
+ const SomeExpr &atom, const MaybeExpr &cond,
+ parser::OpenMPAtomicConstruct::Analysis::Op &&op0,
+ parser::OpenMPAtomicConstruct::Analysis::Op &&op1) {
+ // Defined in flang/include/flang/Parser/parse-tree.h
+ //
+ // struct Analysis {
+ // struct Kind {
+ // static constexpr int None = 0;
+ // static constexpr int Read = 1;
+ // static constexpr int Write = 2;
+ // static constexpr int Update = Read | Write;
+ // static constexpr int Action = 3; // Bits containing N, R, W, U
+ // static constexpr int IfTrue = 4;
+ // static constexpr int IfFalse = 8;
+ // static constexpr int Condition = 12; // Bits containing IfTrue, IfFalse
+ // };
+ // struct Op {
+ // int what;
+ // TypedAssignment assign;
+ // };
+ // TypedExpr atom, cond;
+ // Op op0, op1;
+ // };
+
+ parser::OpenMPAtomicConstruct::Analysis an;
+ SetExpr(an.atom, atom);
+ SetExpr(an.cond, cond);
+ an.op0 = std::move(op0);
+ an.op1 = std::move(op1);
+ return an;
+}
+
+void OmpStructureChecker::CheckStorageOverlap(const SomeExpr &base,
+ llvm::ArrayRef<evaluate::Expr<evaluate::SomeType>> exprs,
+ parser::CharBlock source) {
+ if (auto *expr{HasStorageOverlap(base, exprs)}) {
+ context_.Say(source,
+ "Within atomic operation %s and %s access the same storage"_warn_en_US,
+ base.AsFortran(), expr->AsFortran());
+ }
+}
+
+void OmpStructureChecker::ErrorShouldBeVariable(
+ const MaybeExpr &expr, parser::CharBlock source) {
+ if (expr) {
+ context_.Say(source, "Atomic expression %s should be a variable"_err_en_US,
+ expr->AsFortran());
+ } else {
+ context_.Say(source, "Atomic expression should be a variable"_err_en_US);
+ }
+}
+
+/// Check if `expr` satisfies the following conditions for x and v:
+///
+/// [6.0:189:10-12]
+/// - x and v (as applicable) are either scalar variables or
+/// function references with scalar data pointer result of non-character
+/// intrinsic type or variables that are non-polymorphic scalar pointers
+/// and any length type parameter must be constant.
+void OmpStructureChecker::CheckAtomicVariable(
+ const SomeExpr &atom, parser::CharBlock source) {
+ if (atom.Rank() != 0) {
+ context_.Say(source, "Atomic variable %s should be a scalar"_err_en_US,
+ atom.AsFortran());
+ }
+
+ if (std::optional<evaluate::DynamicType> dtype{atom.GetType()}) {
+ if (dtype->category() == TypeCategory::Character) {
+ context_.Say(source,
+ "Atomic variable %s cannot have CHARACTER type"_err_en_US,
+ atom.AsFortran());
+ } else if (dtype->IsPolymorphic()) {
+ context_.Say(source,
+ "Atomic variable %s cannot have a polymorphic type"_err_en_US,
+ atom.AsFortran());
+ }
+ // TODO: Check non-constant type parameters for non-character types.
+ // At the moment there don't seem to be any.
+ }
+
+ if (IsAllocatable(atom)) {
+ context_.Say(source, "Atomic variable %s cannot be ALLOCATABLE"_err_en_US,
+ atom.AsFortran());
+ }
+}
+
+std::pair<const parser::ExecutionPartConstruct *,
+ const parser::ExecutionPartConstruct *>
+OmpStructureChecker::CheckUpdateCapture(
+ const parser::ExecutionPartConstruct *ec1,
+ const parser::ExecutionPartConstruct *ec2, parser::CharBlock source) {
+ // Decide which statement is the atomic update and which is the capture.
+ //
+ // The two allowed cases are:
+ // x = ... atomic-var = ...
+ // ... = x capture-var = atomic-var (with optional converts)
+ // or
+ // ... = x capture-var = atomic-var (with optional converts)
+ // x = ... atomic-var = ...
+ //
+ // The case of 'a = b; b = a' is ambiguous, so pick the first one as capture
+ // (which makes more sense, as it captures the original value of the atomic
+ // variable).
+ //
+ // If the two statements don't fit these criteria, return a pair of default-
+ // constructed values.
+ using ReturnTy = std::pair<const parser::ExecutionPartConstruct *,
+ const parser::ExecutionPartConstruct *>;
+
+ SourcedActionStmt act1{GetActionStmt(ec1)};
+ SourcedActionStmt act2{GetActionStmt(ec2)};
+ auto maybeAssign1{GetEvaluateAssignment(act1.stmt)};
+ auto maybeAssign2{GetEvaluateAssignment(act2.stmt)};
+ if (!maybeAssign1 || !maybeAssign2) {
+ if (!IsAssignment(act1.stmt) || !IsAssignment(act2.stmt)) {
+ context_.Say(source,
+ "ATOMIC UPDATE operation with CAPTURE should contain two assignments"_err_en_US);
+ }
+ return std::make_pair(nullptr, nullptr);
+ }
+
+ auto as1{*maybeAssign1}, as2{*maybeAssign2};
+
+ auto isUpdateCapture{
+ [](const evaluate::Assignment &u, const evaluate::Assignment &c) {
+ return IsSameOrConvertOf(c.rhs, u.lhs);
+ }};
+
+ // Do some checks that narrow down the possible choices for the update
+ // and the capture statements. This will help to emit better diagnostics.
+ // 1. An assignment could be an update (cbu) if the left-hand side is a
+ // 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 cbc1{IsVarOrFunctionRef(GetConvertInput(as1.rhs))}; // Can 1 be capture?
+ bool cbc2{IsVarOrFunctionRef(GetConvertInput(as2.rhs))}; // Can 2 be capture?
+
+ // We want to diagnose cases where both assignments cannot be an update,
+ // or both cannot be a capture, as well as cases where either assignment
+ // cannot be any of these two.
+ //
+ // If we organize these boolean values into a matrix
+ // |cbu1 cbu2|
+ // |cbc1 cbc2|
+ // then we want to diagnose cases where the matrix has a zero (i.e. "false")
+ // row or column, including the case where everything is zero. All these
+ // cases correspond to the determinant of the matrix being 0, which suggests
+ // that checking the det may be a convenient diagnostic check. There is only
+ // one additional case where the det is 0, which is when the matrix is all 1
+ // ("true"). The "all true" case represents the situation where both
+ // assignments could be an update as well as a capture. On the other hand,
+ // whenever det != 0, the roles of the update and the capture can be
+ // unambiguously assigned to as1 and as2 [1].
+ //
+ // [1] This can be easily verified by hand: there are 10 2x2 matrices with
+ // det = 0, leaving 6 cases where det != 0:
+ // 0 1 0 1 1 0 1 0 1 1 1 1
+ // 1 0 1 1 0 1 1 1 0 1 1 0
+ // In each case the classification is unambiguous.
+
+ // |cbu1 cbu2|
+ // det |cbc1 cbc2| = cbu1*cbc2 - cbu2*cbc1
+ int det{int(cbu1) * int(cbc2) - int(cbu2) * int(cbc1)};
+
+ auto errorCaptureShouldRead{[&](const parser::CharBlock &source,
+ const std::string &expr) {
+ context_.Say(source,
+ "In ATOMIC UPDATE operation with CAPTURE the right-hand side of the capture assignment should read %s"_err_en_US,
+ expr);
+ }};
+
+ auto errorNeitherWorks{[&]() {
+ context_.Say(source,
+ "In ATOMIC UPDATE operation with CAPTURE neither statement could be the update or the capture"_err_en_US);
+ }};
+
+ auto makeSelectionFromDet{[&](int det) -> ReturnTy {
+ // If det != 0, then the checks unambiguously suggest a specific
+ // categorization.
+ // If det == 0, then this function should be called only if the
+ // checks haven't ruled out any possibility, i.e. when both assigments
+ // could still be either updates or captures.
+ if (det > 0) {
+ // as1 is update, as2 is capture
+ if (isUpdateCapture(as1, as2)) {
+ return std::make_pair(/*Update=*/ec1, /*Capture=*/ec2);
+ } else {
+ errorCaptureShouldRead(act2.source, as1.lhs.AsFortran());
+ return std::make_pair(nullptr, nullptr);
+ }
+ } else if (det < 0) {
+ // as2 is update, as1 is capture
+ if (isUpdateCapture(as2, as1)) {
+ return std::make_pair(/*Update=*/ec2, /*Capture=*/ec1);
} else {
- // ATOMIC CAPTURE construct is of the form [capture-stmt, write-stmt]
- CheckAtomicWriteStmt(stmt2);
+ errorCaptureShouldRead(act1.source, as2.lhs.AsFortran());
+ return std::make_pair(nullptr, nullptr);
}
- if (!(*e1 == *v2)) {
- context_.Say(stmt1Expr.source,
- "Captured variable/array element/derived-type component %s expected to be assigned in the second statement of ATOMIC CAPTURE construct"_err_en_US,
- stmt1Expr.source);
+ } else {
+ bool updateFirst{isUpdateCapture(as1, as2)};
+ bool captureFirst{isUpdateCapture(as2, as1)};
+ if (updateFirst && captureFirst) {
+ // If both assignment could be the update and both could be the
+ // capture, emit a warning about the ambiguity.
+ context_.Say(act1.source,
+ "In ATOMIC UPDATE operation with CAPTURE either statement could be the update and the capture, assuming the first one is the capture statement"_warn_en_US);
+ return std::make_pair(/*Update=*/ec2, /*Capture=*/ec1);
}
- } else if (semantics::checkForSymbolMatch(v1, e1) &&
- semantics::checkForSingleVariableOnRHS(stmt2)) {
- // ATOMIC CAPTURE construct is of the form [update-stmt, capture-stmt]
- CheckAtomicUpdateStmt(stmt1);
- CheckAtomicCaptureStmt(stmt2);
- // Variable updated in stmt1 should be captured in stmt2
- if (!(*v1 == *e2)) {
- context_.Say(stmt1Var.GetSource(),
- "Updated variable/array element/derived-type component %s expected to be captured in the second statement of ATOMIC CAPTURE construct"_err_en_US,
- stmt1Var.GetSource());
+ if (updateFirst != captureFirst) {
+ const parser::ExecutionPartConstruct *upd{updateFirst ? ec1 : ec2};
+ const parser::ExecutionPartConstruct *cap{captureFirst ? ec1 : ec2};
+ return std::make_pair(upd, cap);
}
+ assert(!updateFirst && !captureFirst);
+ errorNeitherWorks();
+ return std::make_pair(nullptr, nullptr);
+ }
+ }};
+
+ if (det != 0 || (cbu1 && cbu2 && cbc1 && cbc2)) {
+ return makeSelectionFromDet(det);
+ }
+ assert(det == 0 && "Prior checks should have covered det != 0");
+
+ // If neither of the statements is an RMW update, it could still be a
+ // "write" update. Pretty much any assignment can be a write update, so
+ // recompute det with cbu1 = cbu2 = true.
+ if (int writeDet{int(cbc2) - int(cbc1)}; writeDet || (cbc1 && cbc2)) {
+ return makeSelectionFromDet(writeDet);
+ }
+
+ // It's only errors from here on.
+
+ if (!cbu1 && !cbu2 && !cbc1 && !cbc2) {
+ errorNeitherWorks();
+ return std::make_pair(nullptr, nullptr);
+ }
+
+ // The remaining cases are that
+ // - no candidate for update, or for capture,
+ // - one of the assigments cannot be anything.
+
+ if (!cbu1 && !cbu2) {
+ context_.Say(source,
+ "In ATOMIC UPDATE operation with CAPTURE neither statement could be the update"_err_en_US);
+ return std::make_pair(nullptr, nullptr);
+ } else if (!cbc1 && !cbc2) {
+ context_.Say(source,
+ "In ATOMIC UPDATE operation with CAPTURE neither statement could be the capture"_err_en_US);
+ return std::make_pair(nullptr, nullptr);
+ }
+
+ if ((!cbu1 && !cbc1) || (!cbu2 && !cbc2)) {
+ auto &src = (!cbu1 && !cbc1) ? act1.source : act2.source;
+ context_.Say(src,
+ "In ATOMIC UPDATE operation with CAPTURE the statement could be neither the update nor the capture"_err_en_US);
+ return std::make_pair(nullptr, nullptr);
+ }
+
+ // All cases should have been covered.
+ llvm_unreachable("Unchecked condition");
+}
+
+void OmpStructureChecker::CheckAtomicCaptureAssignment(
+ const evaluate::Assignment &capture, const SomeExpr &atom,
+ parser::CharBlock source) {
+ auto [lsrc, rsrc]{SplitAssignmentSource(source)};
+ const SomeExpr &cap{capture.lhs};
+
+ if (!IsVarOrFunctionRef(atom)) {
+ ErrorShouldBeVariable(atom, rsrc);
+ } else {
+ CheckAtomicVariable(atom, rsrc);
+ // This part should have been checked prior to calling this function.
+ assert(*GetConvertInput(capture.rhs) == atom &&
+ "This canont be a capture assignment");
+ CheckStorageOverlap(atom, {cap}, source);
+ }
+}
+
+void OmpStructureChecker::CheckAtomicReadAssignment(
+ const evaluate::Assignment &read, parser::CharBlock source) {
+ auto [lsrc, rsrc]{SplitAssignmentSource(source)};
+
+ if (auto maybe{GetConvertInput(read.rhs)}) {
+ const SomeExpr &atom{*maybe};
+
+ if (!IsVarOrFunctionRef(atom)) {
+ ErrorShouldBeVariable(atom, rsrc);
} else {
- context_.Say(stmt1Expr.source,
- "Invalid ATOMIC CAPTURE construct statements. Expected one of [update-stmt, capture-stmt], [capture-stmt, update-stmt], or [capture-stmt, write-stmt]"_err_en_US);
+ CheckAtomicVariable(atom, rsrc);
+ CheckStorageOverlap(atom, {read.lhs}, source);
}
+ } else {
+ ErrorShouldBeVariable(read.rhs, rsrc);
}
}
-void OmpStructureChecker::CheckAtomicMemoryOrderClause(
- const parser::OmpAtomicClauseList *leftHandClauseList,
- const parser::OmpAtomicClauseList *rightHandClauseList) {
- int numMemoryOrderClause{0};
- int numFailClause{0};
- auto checkForValidMemoryOrderClause = [&](const parser::OmpAtomicClauseList
- *clauseList) {
- for (const auto &clause : clauseList->v) {
- if (std::get_if<parser::OmpFailClause>(&clause.u)) {
- numFailClause++;
- if (numFailClause > 1) {
- context_.Say(clause.source,
- "More than one FAIL clause not allowed on OpenMP ATOMIC construct"_err_en_US);
- return;
+void OmpStructureChecker::CheckAtomicWriteAssignment(
+ const evaluate::Assignment &write, parser::CharBlock source) {
+ // [6.0:190:13-15]
+ // A write structured block is write-statement, a write statement that has
+ // one of the following forms:
+ // x = expr
+ // x => expr
+ auto [lsrc, rsrc]{SplitAssignmentSource(source)};
+ const SomeExpr &atom{write.lhs};
+
+ if (!IsVarOrFunctionRef(atom)) {
+ ErrorShouldBeVariable(atom, rsrc);
+ } else {
+ CheckAtomicVariable(atom, lsrc);
+ CheckStorageOverlap(atom, {write.rhs}, source);
+ }
+}
+
+void OmpStructureChecker::CheckAtomicUpdateAssignment(
+ const evaluate::Assignment &update, parser::CharBlock source) {
+ // [6.0:191:1-7]
+ // An update structured block is update-statement, an update statement
+ // that has one of the following forms:
+ // x = x operator expr
+ // x = expr operator x
+ // x = intrinsic-procedure-name (x)
+ // x = intrinsic-procedure-name (x, expr-list)
+ // x = intrinsic-procedure-name (expr-list, x)
+ auto [lsrc, rsrc]{SplitAssignmentSource(source)};
+ const SomeExpr &atom{update.lhs};
+
+ if (!IsVarOrFunctionRef(atom)) {
+ ErrorShouldBeVariable(atom, rsrc);
+ // Skip other checks.
+ return;
+ }
+
+ CheckAtomicVariable(atom, lsrc);
+
+ std::pair<operation::Operator, std::vector<SomeExpr>> top{
+ operation::Operator::Unknown, {}};
+ if (auto &&maybeInput{GetConvertInput(update.rhs)}) {
+ top = GetTopLevelOperation(*maybeInput);
+ }
+ switch (top.first) {
+ case operation::Operator::Add:
+ case operation::Operator::Sub:
+ case operation::Operator::Mul:
+ case operation::Operator::Div:
+ case operation::Operator::And:
+ case operation::Operator::Or:
+ case operation::Operator::Eqv:
+ case operation::Operator::Neqv:
+ case operation::Operator::Min:
+ case operation::Operator::Max:
+ case operation::Operator::Identity:
+ break;
+ case operation::Operator::Call:
+ context_.Say(source,
+ "A call to this function is not a valid ATOMIC UPDATE operation"_err_en_US);
+ return;
+ case operation::Operator::Convert:
+ context_.Say(source,
+ "An implicit or explicit type conversion is not a valid ATOMIC UPDATE operation"_err_en_US);
+ return;
+ case operation::Operator::Intrinsic:
+ context_.Say(source,
+ "This intrinsic function is not a valid ATOMIC UPDATE operation"_err_en_US);
+ return;
+ case operation::Operator::Constant:
+ case operation::Operator::Unknown:
+ context_.Say(
+ source, "This is not a valid ATOMIC UPDATE operation"_err_en_US);
+ return;
+ default:
+ assert(
+ top.first != operation::Operator::Identity && "Handle this separately");
+ context_.Say(source,
+ "The %s operator is not a valid ATOMIC UPDATE operation"_err_en_US,
+ operation::ToString(top.first));
+ return;
+ }
+ // Check if `atom` occurs exactly once in the argument list.
+ std::vector<SomeExpr> nonAtom;
+ auto unique{[&]() { // -> iterator
+ auto found{top.second.end()};
+ for (auto i{top.second.begin()}, e{top.second.end()}; i != e; ++i) {
+ if (IsSameOrConvertOf(*i, atom)) {
+ if (found != top.second.end()) {
+ return top.second.end();
}
+ found = i;
} else {
- if (std::get_if<parser::OmpMemoryOrderClause>(&clause.u)) {
- numMemoryOrderClause++;
- if (numMemoryOrderClause > 1) {
- context_.Say(clause.source,
- "More than one memory order clause not allowed on OpenMP ATOMIC construct"_err_en_US);
- return;
- }
+ nonAtom.push_back(*i);
+ }
+ }
+ return found;
+ }()};
+
+ if (unique == top.second.end()) {
+ if (top.first == operation::Operator::Identity) {
+ // This is "x = y".
+ context_.Say(rsrc,
+ "The atomic variable %s should appear as an argument in the update operation"_err_en_US,
+ atom.AsFortran());
+ } else {
+ assert(top.first != operation::Operator::Identity &&
+ "Handle this separately");
+ context_.Say(rsrc,
+ "The atomic variable %s should occur exactly once among the arguments of the top-level %s operator"_err_en_US,
+ atom.AsFortran(), operation::ToString(top.first));
+ }
+ } else {
+ CheckStorageOverlap(atom, nonAtom, source);
+ }
+}
+
+void OmpStructureChecker::CheckAtomicConditionalUpdateAssignment(
+ const SomeExpr &cond, parser::CharBlock condSource,
+ const evaluate::Assignment &assign, parser::CharBlock assignSource) {
+ auto [alsrc, arsrc]{SplitAssignmentSource(assignSource)};
+ const SomeExpr &atom{assign.lhs};
+
+ if (!IsVarOrFunctionRef(atom)) {
+ ErrorShouldBeVariable(atom, arsrc);
+ // Skip other checks.
+ return;
+ }
+
+ CheckAtomicVariable(atom, alsrc);
+
+ auto top{GetTopLevelOperation(cond)};
+ // Missing arguments to operations would have been diagnosed by now.
+
+ switch (top.first) {
+ case operation::Operator::Associated:
+ if (atom != top.second.front()) {
+ context_.Say(assignSource,
+ "The pointer argument to ASSOCIATED must be same as the target of the assignment"_err_en_US);
+ }
+ break;
+ // x equalop e | e equalop x (allowing "e equalop x" is an extension)
+ case operation::Operator::Eq:
+ case operation::Operator::Eqv:
+ // x ordop expr | expr ordop x
+ case operation::Operator::Lt:
+ case operation::Operator::Gt: {
+ const SomeExpr &arg0{top.second[0]};
+ const SomeExpr &arg1{top.second[1]};
+ if (IsSameOrConvertOf(arg0, atom)) {
+ CheckStorageOverlap(atom, {arg1}, condSource);
+ } else if (IsSameOrConvertOf(arg1, atom)) {
+ CheckStorageOverlap(atom, {arg0}, condSource);
+ } else {
+ assert(top.first != operation::Operator::Identity &&
+ "Handle this separately");
+ context_.Say(assignSource,
+ "An argument of the %s operator should be the target of the assignment"_err_en_US,
+ operation::ToString(top.first));
+ }
+ break;
+ }
+ case operation::Operator::Identity:
+ case operation::Operator::True:
+ case operation::Operator::False:
+ break;
+ default:
+ assert(
+ top.first != operation::Operator::Identity && "Handle this separately");
+ context_.Say(condSource,
+ "The %s operator is not a valid condition for ATOMIC operation"_err_en_US,
+ operation::ToString(top.first));
+ break;
+ }
+}
+
+void OmpStructureChecker::CheckAtomicConditionalUpdateStmt(
+ const AnalyzedCondStmt &update, parser::CharBlock source) {
+ // The condition/statements must be:
+ // - cond: x equalop e ift: x = d iff: -
+ // - cond: x ordop expr ift: x = expr iff: - (+ commute ordop)
+ // - cond: associated(x) ift: x => expr iff: -
+ // - cond: associated(x, e) ift: x => expr iff: -
+
+ // The if-true statement must be present, and must be an assignment.
+ auto maybeAssign{GetEvaluateAssignment(update.ift.stmt)};
+ if (!maybeAssign) {
+ if (update.ift.stmt && !IsAssignment(update.ift.stmt)) {
+ context_.Say(update.ift.source,
+ "In ATOMIC UPDATE COMPARE the update statement should be an assignment"_err_en_US);
+ } else {
+ context_.Say(
+ source, "Invalid body of ATOMIC UPDATE COMPARE operation"_err_en_US);
+ }
+ return;
+ }
+ const evaluate::Assignment assign{*maybeAssign};
+ const SomeExpr &atom{assign.lhs};
+
+ CheckAtomicConditionalUpdateAssignment(
+ update.cond, update.source, assign, update.ift.source);
+
+ CheckStorageOverlap(atom, {assign.rhs}, update.ift.source);
+
+ if (update.iff) {
+ context_.Say(update.iff.source,
+ "In ATOMIC UPDATE COMPARE the update statement should not have an ELSE branch"_err_en_US);
+ }
+}
+
+void OmpStructureChecker::CheckAtomicUpdateOnly(
+ const parser::OpenMPAtomicConstruct &x, const parser::Block &body,
+ parser::CharBlock source) {
+ if (body.size() == 1) {
+ SourcedActionStmt action{GetActionStmt(&body.front())};
+ if (auto maybeUpdate{GetEvaluateAssignment(action.stmt)}) {
+ const SomeExpr &atom{maybeUpdate->lhs};
+ CheckAtomicUpdateAssignment(*maybeUpdate, action.source);
+
+ using Analysis = parser::OpenMPAtomicConstruct::Analysis;
+ x.analysis = MakeAtomicAnalysis(atom, std::nullopt,
+ MakeAtomicAnalysisOp(Analysis::Update, maybeUpdate),
+ MakeAtomicAnalysisOp(Analysis::None));
+ } else if (!IsAssignment(action.stmt)) {
+ context_.Say(
+ source, "ATOMIC UPDATE operation should be an assignment"_err_en_US);
+ }
+ } else {
+ context_.Say(x.source,
+ "ATOMIC UPDATE operation should have a single statement"_err_en_US);
+ }
+}
+
+void OmpStructureChecker::CheckAtomicConditionalUpdate(
+ const parser::OpenMPAtomicConstruct &x, const parser::Block &body,
+ parser::CharBlock source) {
+ // Allowable forms are (single-statement):
+ // - if ...
+ // - x = (... ? ... : x)
+ // and two-statement:
+ // - r = cond ; if (r) ...
+
+ const parser::ExecutionPartConstruct *ust{nullptr}; // update
+ const parser::ExecutionPartConstruct *cst{nullptr}; // condition
+
+ if (body.size() == 1) {
+ ust = &body.front();
+ } else if (body.size() == 2) {
+ cst = &body.front();
+ ust = &body.back();
+ } else {
+ context_.Say(source,
+ "ATOMIC UPDATE COMPARE operation should contain one or two statements"_err_en_US);
+ return;
+ }
+
+ // Flang doesn't support conditional-expr yet, so all update statements
+ // are if-statements.
+
+ // IfStmt: if (...) ...
+ // IfConstruct: if (...) then ... endif
+ auto maybeUpdate{AnalyzeConditionalStmt(ust)};
+ if (!maybeUpdate) {
+ context_.Say(source,
+ "In ATOMIC UPDATE COMPARE the update statement should be a conditional statement"_err_en_US);
+ return;
+ }
+
+ AnalyzedCondStmt &update{*maybeUpdate};
+
+ if (SourcedActionStmt action{GetActionStmt(cst)}) {
+ // The "condition" statement must be `r = cond`.
+ if (auto maybeCond{GetEvaluateAssignment(action.stmt)}) {
+ if (maybeCond->lhs != update.cond) {
+ context_.Say(update.source,
+ "In ATOMIC UPDATE COMPARE the conditional statement must use %s as the condition"_err_en_US,
+ maybeCond->lhs.AsFortran());
+ } else {
+ // If it's "r = ...; if (r) ..." then put the original condition
+ // in `update`.
+ update.cond = maybeCond->rhs;
+ }
+ } else {
+ context_.Say(action.source,
+ "In ATOMIC UPDATE COMPARE with two statements the first statement should compute the condition"_err_en_US);
+ }
+ }
+
+ evaluate::Assignment assign{*GetEvaluateAssignment(update.ift.stmt)};
+
+ CheckAtomicConditionalUpdateStmt(update, source);
+ if (IsCheckForAssociated(update.cond)) {
+ if (!IsPointerAssignment(assign)) {
+ context_.Say(source,
+ "The assignment should be a pointer-assignment when the condition is ASSOCIATED"_err_en_US);
+ }
+ } else {
+ if (IsPointerAssignment(assign)) {
+ context_.Say(source,
+ "The assignment cannot be a pointer-assignment except when the condition is ASSOCIATED"_err_en_US);
+ }
+ }
+
+ using Analysis = parser::OpenMPAtomicConstruct::Analysis;
+ x.analysis = MakeAtomicAnalysis(assign.lhs, update.cond,
+ MakeAtomicAnalysisOp(Analysis::Update | Analysis::IfTrue, assign),
+ MakeAtomicAnalysisOp(Analysis::None));
+}
+
+void OmpStructureChecker::CheckAtomicUpdateCapture(
+ const parser::OpenMPAtomicConstruct &x, const parser::Block &body,
+ parser::CharBlock source) {
+ if (body.size() != 2) {
+ context_.Say(source,
+ "ATOMIC UPDATE operation with CAPTURE should contain two statements"_err_en_US);
+ return;
+ }
+
+ auto [uec, cec]{CheckUpdateCapture(&body.front(), &body.back(), source)};
+ if (!uec || !cec) {
+ // Diagnostics already emitted.
+ return;
+ }
+ SourcedActionStmt uact{GetActionStmt(uec)};
+ SourcedActionStmt cact{GetActionStmt(cec)};
+ // The "dereferences" of std::optional are guaranteed to be valid after
+ // CheckUpdateCapture.
+ evaluate::Assignment update{*GetEvaluateAssignment(uact.stmt)};
+ evaluate::Assignment capture{*GetEvaluateAssignment(cact.stmt)};
+
+ const SomeExpr &atom{update.lhs};
+
+ using Analysis = parser::OpenMPAtomicConstruct::Analysis;
+ int action;
+
+ if (IsMaybeAtomicWrite(update)) {
+ action = Analysis::Write;
+ CheckAtomicWriteAssignment(update, uact.source);
+ } else {
+ action = Analysis::Update;
+ CheckAtomicUpdateAssignment(update, uact.source);
+ }
+ CheckAtomicCaptureAssignment(capture, atom, cact.source);
+
+ if (IsPointerAssignment(update) != IsPointerAssignment(capture)) {
+ context_.Say(cact.source,
+ "The update and capture assignments should both be pointer-assignments or both be non-pointer-assignments"_err_en_US);
+ return;
+ }
+
+ if (GetActionStmt(&body.front()).stmt == uact.stmt) {
+ x.analysis = MakeAtomicAnalysis(atom, std::nullopt,
+ MakeAtomicAnalysisOp(action, update),
+ MakeAtomicAnalysisOp(Analysis::Read, capture));
+ } else {
+ x.analysis = MakeAtomicAnalysis(atom, std::nullopt,
+ MakeAtomicAnalysisOp(Analysis::Read, capture),
+ MakeAtomicAnalysisOp(action, update));
+ }
+}
+
+void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
+ const parser::OpenMPAtomicConstruct &x, const parser::Block &body,
+ parser::CharBlock source) {
+ // There are two different variants of this:
+ // (1) conditional-update and capture separately:
+ // This form only allows single-statement updates, i.e. the update
+ // form "r = cond; if (r) ...)" is not allowed.
----------------
kparzysz wrote:
Done
https://github.com/llvm/llvm-project/pull/137852
More information about the flang-commits
mailing list