[flang-commits] [flang] [llvm] [flang][OpenMP] Decompose compound constructs, do recursive lowering (PR #90098)

[Sergio Afonso via flang-commits]

================
@@ -0,0 +1,404 @@
+//===- ConstructCompositionT.h -- Composing compound constructs -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// Given a list of leaf construct, each with a set of clauses, generate the
+// compound construct whose leaf constructs are the given list, and whose clause
+// list is the merged lists of individual leaf clauses.
+//
+// *** At the moment it assumes that the individual constructs and their clauses
+// *** are a subset of those created by splitting a valid compound construct.
+//===----------------------------------------------------------------------===//
+#ifndef LLVM_FRONTEND_OPENMP_CONSTRUCTCOMPOSITIONT_H
+#define LLVM_FRONTEND_OPENMP_CONSTRUCTCOMPOSITIONT_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/BitVector.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Frontend/OpenMP/ClauseT.h"
+#include "llvm/Frontend/OpenMP/OMP.h"
+
+#include <iterator>
+#include <optional>
+#include <tuple>
+#include <unordered_map>
+#include <unordered_set>
+#include <utility>
+
+namespace detail {
+template <typename T> struct trivial_hash {
+  size_t operator()(const T &) const { return 1u; }
+};
+} // namespace detail
+
+namespace tomp {
+template <typename ClauseType> struct ConstructCompositionT {
+  using ClauseTy = ClauseType;
+
+  using TypeTy = typename ClauseTy::TypeTy;
+  using IdTy = typename ClauseTy::IdTy;
+  using ExprTy = typename ClauseTy::ExprTy;
+
+  ConstructCompositionT(uint32_t version,
+                        llvm::ArrayRef<DirectiveWithClauses<ClauseTy>> leafs);
+
+  DirectiveWithClauses<ClauseTy> merged;
+
+private:
+  using ClauseSet =
+      std::unordered_set<ClauseTy, detail::trivial_hash<ClauseTy>>;
+
+  enum class Presence {
+    All,  // Clause is preesnt on all leaf constructs that allow it.
+    Some, // Clause is present on some, but not on all constructs.
+    None, // Clause is absent on all constructs.
+  };
+
+  template <typename S>
+  ClauseTy makeClause(llvm::omp::Clause clauseId, S &&specific) {
+    return ClauseTy{clauseId, std::move(specific)};
+  }
+
+  llvm::omp::Directive
+  makeCompound(llvm::ArrayRef<DirectiveWithClauses<ClauseTy>> parts);
+
+  Presence checkPresence(llvm::omp::Clause clauseId);
+
+  // There are clauses that need special handling:
+  // 1. "if": the "directive-name-modifier" on the merged clause may need
+  // to be set appropriately.
+  // 2. "reduction": implies "privateness" of all objects (incompatible
+  // with "shared"); there are rules for merging modifiers
+  void mergeIf();
+  void mergeReduction();
+  void mergeDSA();
+
+  uint32_t version;
+  llvm::ArrayRef<DirectiveWithClauses<ClauseTy>> leafs;
+
+  // clause id -> set of leaf constructs that contain it
+  std::unordered_map<llvm::omp::Clause, llvm::BitVector> clausePresence;
+  // clause id -> set of instances of that clause
+  std::unordered_map<llvm::omp::Clause, ClauseSet> clauseSets;
+};
+
+template <typename C>
+ConstructCompositionT<C>::ConstructCompositionT(
+    uint32_t version, llvm::ArrayRef<DirectiveWithClauses<C>> leafs)
+    : version(version), leafs(leafs) {
+  // Merge the list of constructs with clauses into a compound construct
----------------
skatrak wrote:

The fundamental issue I have with this approach is that we're splitting clauses by leaf construct first and then re-combining a subset of them if it refers to a composite construct in `buildConstructQueue`. Later on, this re-combined single list of clauses will be passed to the composite lowering `genCompositeXyz` functions, which in turn will now have to re-split them so that the right clauses are attached to each leaf operation.

Wouldn't it make more sense to split once and not try to re-combine composite constructs? In particular, I'm thinking about not keeping composite constructs as a unit as a result of `buildConstructQueue` (removing the need for this `ConstructCompositionT` class) and instead updating `genOMPDispatch` to potentially match multiple leaves in the construct queue when those refer to a composite construct. Then, the single corresponding composite construct lowering function can be called passing a separate clause list for each leaf construct, removing the need to run any logic to re-attach clauses to the applicable operation again.

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