[flang-commits] [flang] [flang][Lower] Add alternative real expression lowering (PR #207371)

[via flang-commits]

================
@@ -51,6 +57,113 @@ static bool isParenthesized(const Fortran::evaluate::Expr<T> &expr) {
   }
 }
 
+template <int KIND>
+using Real = Fortran::evaluate::Type<Fortran::common::TypeCategory::Real, KIND>;
+
+template <int KIND>
+using RealExpr = Fortran::evaluate::Expr<Real<KIND>>;
+
+template <int KIND>
+static void flattenTopLevelAdds(const RealExpr<KIND> &expr,
+                                llvm::SmallVectorImpl<RealExpr<KIND>> &terms) {
+  if (const auto *add =
+          std::get_if<Fortran::evaluate::Add<Real<KIND>>>(&expr.u)) {
+    flattenTopLevelAdds(add->left(), terms);
+    flattenTopLevelAdds(add->right(), terms);
+    return;
+  }
+  terms.push_back(expr);
+}
+
+template <int KIND>
+static RealExpr<KIND>
+buildRightAssociatedAddFold(llvm::ArrayRef<RealExpr<KIND>> terms) {
+  assert(!terms.empty() && "cannot build empty add fold");
+  if (terms.size() == 1)
+    return terms.front();
+  RealExpr<KIND> result{terms.back()};
+  for (const RealExpr<KIND> &term : llvm::reverse(terms.drop_back()))
+    result = RealExpr<KIND>{Fortran::evaluate::Add<Real<KIND>>{term, result}};
+  return result;
+}
+
+template <typename L, typename R>
+static std::optional<Fortran::lower::SomeExpr>
+tryBuildSplitSumExpressionTree(const L &, const R &) {
+  return std::nullopt;
+}
+
+template <int KIND>
+static std::optional<Fortran::lower::SomeExpr>
+tryBuildSplitSumExpressionTree(const RealExpr<KIND> &lhs,
+                               const RealExpr<KIND> &rhs) {
+  if (!std::get_if<Fortran::evaluate::Add<Real<KIND>>>(&rhs.u))
+    return std::nullopt;
+
+  llvm::SmallVector<RealExpr<KIND>, 8> terms;
+  flattenTopLevelAdds(rhs, terms);
+  if (terms.size() <= 2)
+    return std::nullopt;
+
+  llvm::SmallVector<RealExpr<KIND>, 2> head{terms[0], terms[1]};
+  llvm::SmallVector<RealExpr<KIND>, 8> tail(terms.begin() + 2, terms.end());
+  RealExpr<KIND> headExpr = buildRightAssociatedAddFold<KIND>(head);
+  RealExpr<KIND> tailExpr = buildRightAssociatedAddFold<KIND>(tail);
+  return Fortran::lower::SomeExpr{RealExpr<KIND>{
+      Fortran::evaluate::Add<Real<KIND>>{std::move(tailExpr), headExpr}}};
+}
+
+template <Fortran::common::TypeCategory CAT>
+static std::optional<Fortran::lower::SomeExpr> tryBuildSplitSumExpressionTree(
+    const Fortran::evaluate::Expr<Fortran::evaluate::SomeKind<CAT>> &lhs,
+    const Fortran::evaluate::Expr<Fortran::evaluate::SomeKind<CAT>> &rhs) {
+  if constexpr (CAT == Fortran::common::TypeCategory::Real) {
+    return Fortran::common::visit(
+        [&](const auto &typedLhs) -> std::optional<Fortran::lower::SomeExpr> {
+          return Fortran::common::visit(
+              [&](const auto &typedRhs)
+                  -> std::optional<Fortran::lower::SomeExpr> {
+                return tryBuildSplitSumExpressionTree(typedLhs, typedRhs);
+              },
+              rhs.u);
+        },
+        lhs.u);
+  }
+  return std::nullopt;
+}
+
+static bool
+canBuildSplitSumExpressionTree(const Fortran::lower::SomeExpr &lhs,
+                               const Fortran::lower::SomeExpr &rhs) {
+  // The split rewrites a top-level addition chain. Subtraction would need to
+  // be carried as signed terms; division is safe here because it remains inside
+  // an individual term rather than changing the additive chain.
----------------
jeanPerier wrote:

> The intention was to merge something conservative and then extend if other useful cases were found. Would you like support for this in a follow up patch?

This sounds good to me as this is experimental.

My thinking is that there is no point in limiting the rewrite when there are parenthesis/substruction in sub-expression. I would imagine the dependency chain reduction in `x = x + (a-b) + (c-d) + (e-f)` would be as important as in `x = x + a*b + c*d +e*f` and that nothing in the subtractions/parenthesis really justifies not moving `x` around.

You may learn more by being very aggressive in the experimentations to learn about the cases where the rewrite is an issue from a performance/precision point of view.

However, I am happy with you experimenting first with more conservative rewrites and the patch LGTM.

https://github.com/llvm/llvm-project/pull/207371