[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #113557)

Sergio Afonso via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Nov 4 04:29:52 PST 2024


================
@@ -183,173 +466,119 @@ getComponentObject(std::optional<Object> object,
   return getComponentObject(baseObj.value(), semaCtx);
 }
 
-static void
-generateMemberPlacementIndices(const Object &object,
-                               llvm::SmallVectorImpl<int> &indices,
-                               semantics::SemanticsContext &semaCtx) {
+void generateMemberPlacementIndices(const Object &object,
+                                    llvm::SmallVectorImpl<int64_t> &indices,
+                                    semantics::SemanticsContext &semaCtx) {
+  assert(indices.empty() && "indices vector passed to "
+                            "generateMemberPlacementIndices should be empty");
   auto compObj = getComponentObject(object, semaCtx);
+
   while (compObj) {
-    indices.push_back(getComponentPlacementInParent(compObj->sym()));
+    int64_t index = getComponentPlacementInParent(compObj->sym());
+    assert(
+        index >= 0 &&
+        "unexpected index value returned from getComponentPlacementInParent");
+    indices.push_back(index);
     compObj =
         getComponentObject(getBaseObject(compObj.value(), semaCtx), semaCtx);
   }
 
-  indices = llvm::SmallVector<int>{llvm::reverse(indices)};
+  indices = llvm::SmallVector<int64_t>{llvm::reverse(indices)};
 }
 
-void addChildIndexAndMapToParent(
-    const omp::Object &object,
-    std::map<const semantics::Symbol *,
-             llvm::SmallVector<OmpMapMemberIndicesData>> &parentMemberIndices,
-    mlir::omp::MapInfoOp &mapOp, semantics::SemanticsContext &semaCtx) {
-  std::optional<evaluate::DataRef> dataRef = ExtractDataRef(object.ref());
-  assert(dataRef.has_value() &&
-         "DataRef could not be extracted during mapping of derived type "
-         "cannot proceed");
-  const semantics::Symbol *parentSym = &dataRef->GetFirstSymbol();
-  assert(parentSym && "Could not find parent symbol during lower of "
-                      "a component member in OpenMP map clause");
-  llvm::SmallVector<int> indices;
+void OmpMapParentAndMemberData::addChildIndexAndMapToParent(
+    const omp::Object &object, mlir::omp::MapInfoOp &mapOp,
+    semantics::SemanticsContext &semaCtx) {
+  llvm::SmallVector<int64_t> indices;
   generateMemberPlacementIndices(object, indices, semaCtx);
-  parentMemberIndices[parentSym].push_back({indices, mapOp});
+  memberPlacementIndices.push_back(indices);
+  memberMap.push_back(mapOp);
 }
 
-static void calculateShapeAndFillIndices(
-    llvm::SmallVectorImpl<int64_t> &shape,
-    llvm::SmallVectorImpl<OmpMapMemberIndicesData> &memberPlacementData) {
-  shape.push_back(memberPlacementData.size());
-  size_t largestIndicesSize =
-      std::max_element(memberPlacementData.begin(), memberPlacementData.end(),
-                       [](auto a, auto b) {
-                         return a.memberPlacementIndices.size() <
-                                b.memberPlacementIndices.size();
-                       })
-          ->memberPlacementIndices.size();
-  shape.push_back(largestIndicesSize);
-
-  // DenseElementsAttr expects a rectangular shape for the data, so all
-  // index lists have to be of the same length, this emplaces -1 as filler.
-  for (auto &v : memberPlacementData) {
-    if (v.memberPlacementIndices.size() < largestIndicesSize) {
-      auto *prevEnd = v.memberPlacementIndices.end();
-      v.memberPlacementIndices.resize(largestIndicesSize);
-      std::fill(prevEnd, v.memberPlacementIndices.end(), -1);
-    }
+bool isMemberOrParentAllocatableOrPointer(
+    const Object &object, semantics::SemanticsContext &semaCtx) {
+  if (semantics::IsAllocatableOrObjectPointer(object.sym()))
+    return true;
+
+  auto compObj = getBaseObject(object, semaCtx);
+  while (compObj) {
+    if (semantics::IsAllocatableOrObjectPointer(compObj.value().sym()))
+      return true;
+    compObj = getBaseObject(compObj.value(), semaCtx);
   }
-}
 
-static mlir::DenseIntElementsAttr createDenseElementsAttrFromIndices(
-    llvm::SmallVectorImpl<OmpMapMemberIndicesData> &memberPlacementData,
-    fir::FirOpBuilder &builder) {
-  llvm::SmallVector<int64_t> shape;
-  calculateShapeAndFillIndices(shape, memberPlacementData);
-
-  llvm::SmallVector<int> indicesFlattened =
-      std::accumulate(memberPlacementData.begin(), memberPlacementData.end(),
-                      llvm::SmallVector<int>(),
-                      [](llvm::SmallVector<int> &x, OmpMapMemberIndicesData y) {
-                        x.insert(x.end(), y.memberPlacementIndices.begin(),
-                                 y.memberPlacementIndices.end());
-                        return x;
-                      });
-
-  return mlir::DenseIntElementsAttr::get(
-      mlir::VectorType::get(shape,
-                            mlir::IntegerType::get(builder.getContext(), 32)),
-      indicesFlattened);
+  return false;
 }
 
 void insertChildMapInfoIntoParent(
-    lower::AbstractConverter &converter,
-    std::map<const semantics::Symbol *,
-             llvm::SmallVector<OmpMapMemberIndicesData>> &parentMemberIndices,
+    lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx,
+    lower::StatementContext &stmtCtx,
+    std::map<Object, OmpMapParentAndMemberData> &parentMemberIndices,
     llvm::SmallVectorImpl<mlir::Value> &mapOperands,
     llvm::SmallVectorImpl<const semantics::Symbol *> &mapSyms) {
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   for (auto indices : parentMemberIndices) {
-    bool parentExists = false;
-    size_t parentIdx;
-    for (parentIdx = 0; parentIdx < mapSyms.size(); ++parentIdx) {
-      if (mapSyms[parentIdx] == indices.first) {
-        parentExists = true;
-        break;
-      }
-    }
-
-    if (parentExists) {
+    auto *parentIter = llvm::find_if(
+        mapSyms, [&indices](auto v) { return v == indices.first.sym(); });
+    if (parentIter != mapSyms.end()) {
       auto mapOp = llvm::cast<mlir::omp::MapInfoOp>(
-          mapOperands[parentIdx].getDefiningOp());
+          mapOperands[std::distance(mapSyms.begin(), parentIter)]
+              .getDefiningOp());
 
       // NOTE: To maintain appropriate SSA ordering, we move the parent map
       // which will now have references to its children after the last
       // of its members to be generated. This is necessary when a user
       // has defined a series of parent and children maps where the parent
       // precedes the children. An alternative, may be to do
       // delayed generation of map info operations from the clauses and
-      // organize them first before generation.
-      mapOp->moveAfter(indices.second.back().memberMap);
+      // organize them first before generation. Or to use the
+      // topologicalSort utility which will enforce a stronger SSA
+      // dominance ordering at the cost of efficiency/time.
+      mapOp->moveAfter(indices.second.memberMap.back());
 
-      for (auto memberIndicesData : indices.second)
-        mapOp.getMembersMutable().append(
-            memberIndicesData.memberMap.getResult());
+      for (mlir::omp::MapInfoOp memberMap : indices.second.memberMap)
+        mapOp.getMembersMutable().append(memberMap.getResult());
 
-      mapOp.setMembersIndexAttr(createDenseElementsAttrFromIndices(
-          indices.second, converter.getFirOpBuilder()));
+      mapOp.setMembersIndexAttr(firOpBuilder.create2DI64ArrayAttr(
+          indices.second.memberPlacementIndices));
     } else {
       // NOTE: We take the map type of the first child, this may not
       // be the correct thing to do, however, we shall see. For the moment
       // it allows this to work with enter and exit without causing MLIR
       // verification issues. The more appropriate thing may be to take
       // the "main" map type clause from the directive being used.
-      uint64_t mapType = indices.second[0].memberMap.getMapType().value_or(0);
-
-      // create parent to emplace and bind members
-      mlir::Value origSymbol = converter.getSymbolAddress(*indices.first);
+      uint64_t mapType = indices.second.memberMap[0].getMapType().value_or(0);
 
       llvm::SmallVector<mlir::Value> members;
-      for (OmpMapMemberIndicesData memberIndicesData : indices.second)
-        members.push_back((mlir::Value)memberIndicesData.memberMap);
-
-      mlir::Value mapOp = createMapInfoOp(
-          converter.getFirOpBuilder(), origSymbol.getLoc(), origSymbol,
-          /*varPtrPtr=*/mlir::Value(), indices.first->name().ToString(),
-          /*bounds=*/{}, members,
-          createDenseElementsAttrFromIndices(indices.second,
-                                             converter.getFirOpBuilder()),
-          mapType, mlir::omp::VariableCaptureKind::ByRef, origSymbol.getType(),
+      for (mlir::omp::MapInfoOp memberMap : indices.second.memberMap)
----------------
skatrak wrote:

Nit: Call `members.reserve(indices.second.memberMap.size())` before the loop.

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


More information about the llvm-branch-commits mailing list