Skip to content

[flang][Evaluate] OperationCode cleanup, fix for Constant<T> #151566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 1, 2025
Merged

[flang][Evaluate] OperationCode cleanup, fix for Constant<T> #151566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d032d4
[flang][Evaluate] OperationCode cleanup, fix for Constant<T>
kparzysz Jul 31, 2025
aab7380
Add overload for Relational<SomeType>
kparzysz Jul 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 22 additions & 34 deletions flang/include/flang/Evaluate/tools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1402,10 +1402,8 @@ using OperatorSet = common::EnumSet<Operator, 32>;

std::string ToString(Operator op);

template <typename... Ts, int Kind>
Operator OperationCode(
const evaluate::Operation<evaluate::LogicalOperation<Kind>, Ts...> &op) {
switch (op.derived().logicalOperator) {
template <int Kind> Operator OperationCode(const LogicalOperation<Kind> &op) {
switch (op.logicalOperator) {
case common::LogicalOperator::And:
return Operator::And;
case common::LogicalOperator::Or:
Expand All @@ -1420,10 +1418,10 @@ Operator OperationCode(
return Operator::Unknown;
}

template <typename T, typename... Ts>
Operator OperationCode(
const evaluate::Operation<evaluate::Relational<T>, Ts...> &op) {
switch (op.derived().opr) {
Operator OperationCode(const Relational<SomeType> &op);

template <typename T> Operator OperationCode(const Relational<T> &op) {
switch (op.opr) {
case common::RelationalOperator::LT:
return Operator::Lt;
case common::RelationalOperator::LE:
Expand All @@ -1440,70 +1438,60 @@ Operator OperationCode(
return Operator::Unknown;
}

template <typename T, typename... Ts>
Operator OperationCode(const evaluate::Operation<evaluate::Add<T>, Ts...> &op) {
template <typename T> Operator OperationCode(const Add<T> &op) {
return Operator::Add;
}

template <typename T, typename... Ts>
Operator OperationCode(
const evaluate::Operation<evaluate::Subtract<T>, Ts...> &op) {
template <typename T> Operator OperationCode(const Subtract<T> &op) {
return Operator::Sub;
}

template <typename T, typename... Ts>
Operator OperationCode(
const evaluate::Operation<evaluate::Multiply<T>, Ts...> &op) {
template <typename T> Operator OperationCode(const Multiply<T> &op) {
return Operator::Mul;
}

template <typename T, typename... Ts>
Operator OperationCode(
const evaluate::Operation<evaluate::Divide<T>, Ts...> &op) {
template <typename T> Operator OperationCode(const Divide<T> &op) {
return Operator::Div;
}

template <typename T, typename... Ts>
Operator OperationCode(
const evaluate::Operation<evaluate::Power<T>, Ts...> &op) {
template <typename T> Operator OperationCode(const Power<T> &op) {
return Operator::Pow;
}

template <typename T, typename... Ts>
Operator OperationCode(
const evaluate::Operation<evaluate::RealToIntPower<T>, Ts...> &op) {
template <typename T> Operator OperationCode(const RealToIntPower<T> &op) {
return Operator::Pow;
}

template <typename T, common::TypeCategory C, typename... Ts>
Operator OperationCode(
const evaluate::Operation<evaluate::Convert<T, C>, Ts...> &op) {
template <typename T, common::TypeCategory C>
Operator OperationCode(const Convert<T, C> &op) {
if constexpr (C == T::category) {
return Operator::Resize;
} else {
return Operator::Convert;
}
}

template <typename T, typename... Ts>
Operator OperationCode(
const evaluate::Operation<evaluate::Extremum<T>, Ts...> &op) {
if (op.derived().ordering == evaluate::Ordering::Greater) {
template <typename T> Operator OperationCode(const Extremum<T> &op) {
if (op.ordering == Ordering::Greater) {
return Operator::Max;
} else {
return Operator::Min;
}
}

template <typename T> Operator OperationCode(const evaluate::Constant<T> &x) {
template <typename T> Operator OperationCode(const Constant<T> &x) {
return Operator::Constant;
}

template <typename T> Operator OperationCode(const Designator<T> &x) {
return Operator::Identity;
}

template <typename T> Operator OperationCode(const T &) {
return Operator::Unknown;
}

Operator OperationCode(const evaluate::ProcedureDesignator &proc);
Operator OperationCode(const ProcedureDesignator &proc);

} // namespace operation

Expand Down
10 changes: 7 additions & 3 deletions flang/lib/Evaluate/tools.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,17 +1693,17 @@ struct ArgumentExtractor
// to int(kind=4) for example.
return (*this)(x.template operand<0>());
} else {
return std::make_pair(operation::OperationCode(x),
return std::make_pair(operation::OperationCode(x.derived()),
OperationArgs(x, std::index_sequence_for<Os...>{}));
}
}

template <typename T> Result operator()(const Designator<T> &x) const {
return {operation::Operator::Identity, {AsSomeExpr(x)}};
return {operation::OperationCode(x), {AsSomeExpr(x)}};
}

template <typename T> Result operator()(const Constant<T> &x) const {
return {operation::Operator::Identity, {AsSomeExpr(x)}};
return {operation::OperationCode(x), {AsSomeExpr(x)}};
}

template <typename... Rs>
Expand Down Expand Up @@ -1793,6 +1793,10 @@ std::string operation::ToString(operation::Operator op) {
llvm_unreachable("Unhandler operator");
}

operation::Operator operation::OperationCode(const Relational<SomeType> &op) {
return common::visit([](auto &&s) { return OperationCode(s); }, op.u);
}

operation::Operator operation::OperationCode(const ProcedureDesignator &proc) {
Operator code{llvm::StringSwitch<Operator>(proc.GetName())
.Case("associated", Operator::Associated)
Expand Down
2 changes: 1 addition & 1 deletion flang/test/Semantics/OpenMP/atomic04.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ subroutine more_invalid_atomic_update_stmts()
x = x

!$omp atomic update
!ERROR: The atomic variable x should appear as an argument in the update operation
!ERROR: This is not a valid ATOMIC UPDATE operation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the bugfix the functional change here?

Copy link
Contributor Author

@kparzysz kparzysz Jul 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Edit: The problem shows for "x = <constant>". For Constant<T> it used to return "Identity" as the operation code, which is the code for Designator<T>. "Identity" (i.e. x = x) is a valid update operation (as an extension), so we checked for the presence of x among the operands to make sure it's x = x, and not x = y (hence the previous message). "Constant" is not a valid operation, and the new message reflects that.

x = 1

!$omp atomic update
Expand Down
2 changes: 1 addition & 1 deletion flang/test/Semantics/OpenMP/atomic05.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ program OmpAtomic
x = 2 * 4
!ERROR: At most one clause from the 'memory-order' group is allowed on ATOMIC construct
!$omp atomic update release, seq_cst
!ERROR: The atomic variable x should appear as an argument in the update operation
!ERROR: This is not a valid ATOMIC UPDATE operation
x = 10
!ERROR: At most one clause from the 'memory-order' group is allowed on ATOMIC construct
!$omp atomic capture release, seq_cst
Expand Down