[flang-commits] [flang] [flang][openacc/mp] Do not read bounds on absent box (PR #75252)

Valentin Clement バレンタイン クレメン via flang-commits flang-commits at lists.llvm.org
Fri Dec 15 10:53:49 PST 2023


https://github.com/clementval updated https://github.com/llvm/llvm-project/pull/75252

>From 137c0330d3eab5bdb500f5807b826af0ace5fce4 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Tue, 12 Dec 2023 13:36:23 -0800
Subject: [PATCH 1/2] [flang][openacc] Do not load optional box if not present

---
 flang/lib/Lower/DirectivesCommon.h      | 184 ++++++++++++++++++------
 flang/lib/Lower/OpenACC.cpp             | 108 ++++++++------
 flang/lib/Lower/OpenMP.cpp              |  27 ++--
 flang/test/Lower/OpenACC/acc-bounds.f90 |  35 +++++
 flang/test/Lower/OpenACC/acc-data.f90   |   1 -
 5 files changed, 257 insertions(+), 98 deletions(-)

diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index 52d0b245001685..e0966583e64259 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -47,6 +47,18 @@
 namespace Fortran {
 namespace lower {
 
+/// Information gathered to generate bounds operation and data entry/exit
+/// operations.
+struct AddrAndBoundsInfo {
+  explicit AddrAndBoundsInfo() {}
+  explicit AddrAndBoundsInfo(mlir::Value addr) : addr(addr) {}
+  explicit AddrAndBoundsInfo(mlir::Value addr, mlir::Value isPresent)
+      : addr(addr), isPresent(isPresent) {}
+  // const Fortran::semantics::Symbol *sym;
+  mlir::Value addr = nullptr;
+  mlir::Value isPresent = nullptr;
+};
+
 /// Checks if the assignment statement has a single variable on the RHS.
 static inline bool checkForSingleVariableOnRHS(
     const Fortran::parser::AssignmentStmt &assignmentStmt) {
@@ -598,7 +610,7 @@ void createEmptyRegionBlocks(
   }
 }
 
-inline mlir::Value
+inline AddrAndBoundsInfo
 getDataOperandBaseAddr(Fortran::lower::AbstractConverter &converter,
                        fir::FirOpBuilder &builder,
                        Fortran::lower::SymbolRef sym, mlir::Location loc) {
@@ -620,25 +632,42 @@ getDataOperandBaseAddr(Fortran::lower::AbstractConverter &converter,
 
     // Load the box when baseAddr is a `fir.ref<fir.box<T>>` or a
     // `fir.ref<fir.class<T>>` type.
-    if (symAddr.getType().isa<fir::ReferenceType>())
-      return builder.create<fir::LoadOp>(loc, symAddr);
+    if (symAddr.getType().isa<fir::ReferenceType>()) {
+      if (Fortran::semantics::IsOptional(sym)) {
+        mlir::Value isPresent =
+            builder.create<fir::IsPresentOp>(loc, builder.getI1Type(), symAddr);
+        mlir::Value addr =
+            builder.genIfOp(loc, {boxTy}, isPresent, /*withElseRegion=*/true)
+                .genThen([&]() {
+                  mlir::Value load = builder.create<fir::LoadOp>(loc, symAddr);
+                  builder.create<fir::ResultOp>(loc, mlir::ValueRange{load});
+                })
+                .genElse([&] {
+                  mlir::Value absent =
+                      builder.create<fir::AbsentOp>(loc, boxTy);
+                  builder.create<fir::ResultOp>(loc, mlir::ValueRange{absent});
+                })
+                .getResults()[0];
+        return AddrAndBoundsInfo(addr, isPresent);
+      }
+      mlir::Value addr = builder.create<fir::LoadOp>(loc, symAddr);
+      return AddrAndBoundsInfo(addr);
+      ;
+    }
   }
-  return symAddr;
+  return AddrAndBoundsInfo(symAddr);
 }
 
-/// Generate the bounds operation from the descriptor information.
 template <typename BoundsOp, typename BoundsType>
 llvm::SmallVector<mlir::Value>
-genBoundsOpsFromBox(fir::FirOpBuilder &builder, mlir::Location loc,
-                    Fortran::lower::AbstractConverter &converter,
-                    fir::ExtendedValue dataExv, mlir::Value box) {
-  llvm::SmallVector<mlir::Value> bounds;
+gatherBoundsOrBoundValues(fir::FirOpBuilder &builder, mlir::Location loc,
+                          fir::ExtendedValue dataExv, mlir::Value box,
+                          bool collectValuesOnly = false) {
+  llvm::SmallVector<mlir::Value> values;
+  mlir::Value byteStride;
   mlir::Type idxTy = builder.getIndexType();
   mlir::Type boundTy = builder.getType<BoundsType>();
   mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-  assert(box.getType().isa<fir::BaseBoxType>() &&
-         "expect fir.box or fir.class");
-  mlir::Value byteStride;
   for (unsigned dim = 0; dim < dataExv.rank(); ++dim) {
     mlir::Value d = builder.createIntegerConstant(loc, idxTy, dim);
     mlir::Value baseLb =
@@ -650,12 +679,79 @@ genBoundsOpsFromBox(fir::FirOpBuilder &builder, mlir::Location loc,
         builder.create<mlir::arith::SubIOp>(loc, dimInfo.getExtent(), one);
     if (dim == 0) // First stride is the element size.
       byteStride = dimInfo.getByteStride();
-    mlir::Value bound = builder.create<BoundsOp>(
-        loc, boundTy, lb, ub, dimInfo.getExtent(), byteStride, true, baseLb);
+    if (collectValuesOnly) {
+      values.push_back(lb);
+      values.push_back(ub);
+      values.push_back(dimInfo.getExtent());
+      values.push_back(byteStride);
+      values.push_back(baseLb);
+    } else {
+      mlir::Value bound = builder.create<BoundsOp>(
+          loc, boundTy, lb, ub, dimInfo.getExtent(), byteStride, true, baseLb);
+      values.push_back(bound);
+    }
     // Compute the stride for the next dimension.
     byteStride = builder.create<mlir::arith::MulIOp>(loc, byteStride,
                                                      dimInfo.getExtent());
-    bounds.push_back(bound);
+  }
+  return values;
+}
+
+/// Generate the bounds operation from the descriptor information.
+template <typename BoundsOp, typename BoundsType>
+llvm::SmallVector<mlir::Value>
+genBoundsOpsFromBox(fir::FirOpBuilder &builder, mlir::Location loc,
+                    Fortran::lower::AbstractConverter &converter,
+                    fir::ExtendedValue dataExv,
+                    Fortran::lower::AddrAndBoundsInfo &info) {
+  llvm::SmallVector<mlir::Value> bounds;
+  mlir::Type idxTy = builder.getIndexType();
+  mlir::Type boundTy = builder.getType<BoundsType>();
+
+  assert(info.addr.getType().isa<fir::BaseBoxType>() &&
+         "expect fir.box or fir.class");
+
+  if (info.isPresent) {
+    llvm::SmallVector<mlir::Type> resTypes;
+    constexpr unsigned nbValuesPerBound = 5;
+    for (unsigned dim = 0; dim < dataExv.rank() * nbValuesPerBound; ++dim)
+      resTypes.push_back(idxTy);
+
+    mlir::Operation::result_range ifRes =
+        builder.genIfOp(loc, resTypes, info.isPresent, /*withElseRegion=*/true)
+            .genThen([&]() {
+              llvm::SmallVector<mlir::Value> boundValues =
+                  gatherBoundsOrBoundValues<BoundsOp, BoundsType>(
+                      builder, loc, dataExv, info.addr,
+                      /*collectValuesOnly=*/true);
+              builder.create<fir::ResultOp>(loc, boundValues);
+            })
+            .genElse([&] {
+              // Box is not present. Populate bound values with default values.
+              llvm::SmallVector<mlir::Value> boundValues;
+              mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
+              mlir::Value mOne = builder.createIntegerConstant(loc, idxTy, -1);
+              for (unsigned dim = 0; dim < dataExv.rank(); ++dim) {
+                boundValues.push_back(zero); // lb
+                boundValues.push_back(mOne); // ub
+                boundValues.push_back(zero); // extent
+                boundValues.push_back(zero); // byteStride
+                boundValues.push_back(zero); // baseLb
+              }
+              builder.create<fir::ResultOp>(loc, boundValues);
+            })
+            .getResults();
+    // Create the bound operations outside the if-then-else with the if op
+    // results.
+    for (unsigned i = 0; i < ifRes.size(); i += nbValuesPerBound) {
+      mlir::Value bound = builder.create<BoundsOp>(
+          loc, boundTy, ifRes[i], ifRes[i + 1], ifRes[i + 2], ifRes[i + 3],
+          true, ifRes[i + 4]);
+      bounds.push_back(bound);
+    }
+  } else {
+    bounds = gatherBoundsOrBoundValues<BoundsOp, BoundsType>(
+        builder, loc, dataExv, info.addr);
   }
   return bounds;
 }
@@ -843,14 +939,13 @@ genBoundsOps(fir::FirOpBuilder &builder, mlir::Location loc,
 }
 
 template <typename ObjectType, typename BoundsOp, typename BoundsType>
-mlir::Value gatherDataOperandAddrAndBounds(
+AddrAndBoundsInfo gatherDataOperandAddrAndBounds(
     Fortran::lower::AbstractConverter &converter, fir::FirOpBuilder &builder,
     Fortran::semantics::SemanticsContext &semanticsContext,
     Fortran::lower::StatementContext &stmtCtx, const ObjectType &object,
     mlir::Location operandLocation, std::stringstream &asFortran,
     llvm::SmallVector<mlir::Value> &bounds, bool treatIndexAsSection = false) {
-  mlir::Value baseAddr;
-
+  AddrAndBoundsInfo info;
   std::visit(
       Fortran::common::visitors{
           [&](const Fortran::parser::Designator &designator) {
@@ -872,13 +967,13 @@ mlir::Value gatherDataOperandAddrAndBounds(
                       semanticsContext, arrayElement->base);
                   dataExv = converter.genExprAddr(operandLocation, *exprBase,
                                                   stmtCtx);
-                  baseAddr = fir::getBase(dataExv);
+                  info.addr = fir::getBase(dataExv);
                   asFortran << (*exprBase).AsFortran();
                 } else {
                   const Fortran::parser::Name &name =
                       Fortran::parser::GetLastName(*dataRef);
-                  baseAddr = getDataOperandBaseAddr(
-                      converter, builder, *name.symbol, operandLocation);
+                  info = getDataOperandBaseAddr(converter, builder,
+                                                *name.symbol, operandLocation);
                   dataExv = converter.getSymbolExtendedValue(*name.symbol);
                   asFortran << name.ToString();
                 }
@@ -887,27 +982,33 @@ mlir::Value gatherDataOperandAddrAndBounds(
                   asFortran << '(';
                   bounds = genBoundsOps<BoundsOp, BoundsType>(
                       builder, operandLocation, converter, stmtCtx,
-                      arrayElement->subscripts, asFortran, dataExv, baseAddr,
+                      arrayElement->subscripts, asFortran, dataExv, info.addr,
                       treatIndexAsSection);
                 }
                 asFortran << ')';
-              } else if (Fortran::parser::Unwrap<
+              } else if (auto structComp = Fortran::parser::Unwrap<
                              Fortran::parser::StructureComponent>(designator)) {
                 fir::ExtendedValue compExv =
                     converter.genExprAddr(operandLocation, *expr, stmtCtx);
-                baseAddr = fir::getBase(compExv);
-                if (fir::unwrapRefType(baseAddr.getType())
+                info.addr = fir::getBase(compExv);
+                if (fir::unwrapRefType(info.addr.getType())
                         .isa<fir::SequenceType>())
                   bounds = genBaseBoundsOps<BoundsOp, BoundsType>(
                       builder, operandLocation, converter, compExv);
                 asFortran << (*expr).AsFortran();
 
+                bool isOptional = Fortran::semantics::IsOptional(
+                    *Fortran::parser::GetLastName(*structComp).symbol);
+                if (isOptional)
+                  info.isPresent = builder.create<fir::IsPresentOp>(
+                      operandLocation, builder.getI1Type(), info.addr);
+
                 if (auto loadOp = mlir::dyn_cast_or_null<fir::LoadOp>(
-                        baseAddr.getDefiningOp())) {
+                        info.addr.getDefiningOp())) {
                   if (fir::isAllocatableType(loadOp.getType()) ||
                       fir::isPointerType(loadOp.getType()))
-                    baseAddr = builder.create<fir::BoxAddrOp>(operandLocation,
-                                                              baseAddr);
+                    info.addr = builder.create<fir::BoxAddrOp>(operandLocation,
+                                                               info.addr);
                 }
 
                 // If the component is an allocatable or pointer the result of
@@ -915,10 +1016,10 @@ mlir::Value gatherDataOperandAddrAndBounds(
                 // a fir.box_addr has been inserted just before.
                 // Retrieve the box so we handle it like other descriptor.
                 if (auto boxAddrOp = mlir::dyn_cast_or_null<fir::BoxAddrOp>(
-                        baseAddr.getDefiningOp())) {
-                  baseAddr = boxAddrOp.getVal();
+                        info.addr.getDefiningOp())) {
+                  info.addr = boxAddrOp.getVal();
                   bounds = genBoundsOpsFromBox<BoundsOp, BoundsType>(
-                      builder, operandLocation, converter, compExv, baseAddr);
+                      builder, operandLocation, converter, compExv, info);
                 }
               } else {
                 if (Fortran::parser::Unwrap<Fortran::parser::ArrayElement>(
@@ -930,7 +1031,7 @@ mlir::Value gatherDataOperandAddrAndBounds(
                   (void)arrayElement;
                   fir::ExtendedValue compExv =
                       converter.genExprAddr(operandLocation, *expr, stmtCtx);
-                  baseAddr = fir::getBase(compExv);
+                  info.addr = fir::getBase(compExv);
                   asFortran << (*expr).AsFortran();
                 } else if (const auto *dataRef{
                                std::get_if<Fortran::parser::DataRef>(
@@ -940,13 +1041,14 @@ mlir::Value gatherDataOperandAddrAndBounds(
                       Fortran::parser::GetLastName(*dataRef);
                   fir::ExtendedValue dataExv =
                       converter.getSymbolExtendedValue(*name.symbol);
-                  baseAddr = getDataOperandBaseAddr(
-                      converter, builder, *name.symbol, operandLocation);
-                  if (fir::unwrapRefType(baseAddr.getType())
-                          .isa<fir::BaseBoxType>())
+                  info = getDataOperandBaseAddr(converter, builder,
+                                                *name.symbol, operandLocation);
+                  if (fir::unwrapRefType(info.addr.getType())
+                          .isa<fir::BaseBoxType>()) {
                     bounds = genBoundsOpsFromBox<BoundsOp, BoundsType>(
-                        builder, operandLocation, converter, dataExv, baseAddr);
-                  if (fir::unwrapRefType(baseAddr.getType())
+                        builder, operandLocation, converter, dataExv, info);
+                  }
+                  if (fir::unwrapRefType(info.addr.getType())
                           .isa<fir::SequenceType>())
                     bounds = genBaseBoundsOps<BoundsOp, BoundsType>(
                         builder, operandLocation, converter, dataExv);
@@ -959,12 +1061,12 @@ mlir::Value gatherDataOperandAddrAndBounds(
             }
           },
           [&](const Fortran::parser::Name &name) {
-            baseAddr = getDataOperandBaseAddr(converter, builder, *name.symbol,
-                                              operandLocation);
+            info = getDataOperandBaseAddr(converter, builder, *name.symbol,
+                                          operandLocation);
             asFortran << name.ToString();
           }},
       object.u);
-  return baseAddr;
+  return info;
 }
 
 } // namespace lower
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index 531685948bc843..75432db33a790d 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -62,11 +62,29 @@ static Op createDataEntryOp(fir::FirOpBuilder &builder, mlir::Location loc,
                             mlir::Value baseAddr, std::stringstream &name,
                             mlir::SmallVector<mlir::Value> bounds,
                             bool structured, bool implicit,
-                            mlir::acc::DataClause dataClause,
-                            mlir::Type retTy) {
+                            mlir::acc::DataClause dataClause, mlir::Type retTy,
+                            mlir::Value isPresent = {}) {
   mlir::Value varPtrPtr;
   if (auto boxTy = baseAddr.getType().dyn_cast<fir::BaseBoxType>()) {
-    baseAddr = builder.create<fir::BoxAddrOp>(loc, baseAddr);
+    if (isPresent) {
+      baseAddr =
+          builder
+              .genIfOp(loc, {boxTy.getEleTy()}, isPresent,
+                       /*withElseRegion=*/true)
+              .genThen([&]() {
+                mlir::Value boxAddr =
+                    builder.create<fir::BoxAddrOp>(loc, baseAddr);
+                builder.create<fir::ResultOp>(loc, mlir::ValueRange{boxAddr});
+              })
+              .genElse([&] {
+                mlir::Value absent =
+                    builder.create<fir::AbsentOp>(loc, boxTy.getEleTy());
+                builder.create<fir::ResultOp>(loc, mlir::ValueRange{absent});
+              })
+              .getResults()[0];
+    } else {
+      baseAddr = builder.create<fir::BoxAddrOp>(loc, baseAddr);
+    }
     retTy = baseAddr.getType();
   }
 
@@ -265,15 +283,17 @@ genDataOperandOperations(const Fortran::parser::AccObjectList &objectList,
     llvm::SmallVector<mlir::Value> bounds;
     std::stringstream asFortran;
     mlir::Location operandLocation = genOperandLocation(converter, accObject);
-    mlir::Value baseAddr = Fortran::lower::gatherDataOperandAddrAndBounds<
-        Fortran::parser::AccObject, mlir::acc::DataBoundsOp,
-        mlir::acc::DataBoundsType>(converter, builder, semanticsContext,
-                                   stmtCtx, accObject, operandLocation,
-                                   asFortran, bounds,
-                                   /*treatIndexAsSection=*/true);
-    Op op = createDataEntryOp<Op>(builder, operandLocation, baseAddr, asFortran,
-                                  bounds, structured, implicit, dataClause,
-                                  baseAddr.getType());
+    Fortran::lower::AddrAndBoundsInfo info =
+        Fortran::lower::gatherDataOperandAddrAndBounds<
+            Fortran::parser::AccObject, mlir::acc::DataBoundsOp,
+            mlir::acc::DataBoundsType>(converter, builder, semanticsContext,
+                                       stmtCtx, accObject, operandLocation,
+                                       asFortran, bounds,
+                                       /*treatIndexAsSection=*/true);
+
+    Op op = createDataEntryOp<Op>(
+        builder, operandLocation, info.addr, asFortran, bounds, structured,
+        implicit, dataClause, info.addr.getType(), info.isPresent);
     dataOperands.push_back(op.getAccPtr());
   }
 }
@@ -291,27 +311,28 @@ static void genDeclareDataOperandOperations(
     llvm::SmallVector<mlir::Value> bounds;
     std::stringstream asFortran;
     mlir::Location operandLocation = genOperandLocation(converter, accObject);
-    mlir::Value baseAddr = Fortran::lower::gatherDataOperandAddrAndBounds<
-        Fortran::parser::AccObject, mlir::acc::DataBoundsOp,
-        mlir::acc::DataBoundsType>(converter, builder, semanticsContext,
-                                   stmtCtx, accObject, operandLocation,
-                                   asFortran, bounds);
+    Fortran::lower::AddrAndBoundsInfo info =
+        Fortran::lower::gatherDataOperandAddrAndBounds<
+            Fortran::parser::AccObject, mlir::acc::DataBoundsOp,
+            mlir::acc::DataBoundsType>(converter, builder, semanticsContext,
+                                       stmtCtx, accObject, operandLocation,
+                                       asFortran, bounds);
     EntryOp op = createDataEntryOp<EntryOp>(
-        builder, operandLocation, baseAddr, asFortran, bounds, structured,
-        implicit, dataClause, baseAddr.getType());
+        builder, operandLocation, info.addr, asFortran, bounds, structured,
+        implicit, dataClause, info.addr.getType());
     dataOperands.push_back(op.getAccPtr());
     addDeclareAttr(builder, op.getVarPtr().getDefiningOp(), dataClause);
-    if (mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(baseAddr.getType()))) {
+    if (mlir::isa<fir::BaseBoxType>(fir::unwrapRefType(info.addr.getType()))) {
       mlir::OpBuilder modBuilder(builder.getModule().getBodyRegion());
       modBuilder.setInsertionPointAfter(builder.getFunction());
       std::string prefix =
           converter.mangleName(getSymbolFromAccObject(accObject));
       createDeclareAllocFuncWithArg<EntryOp>(
-          modBuilder, builder, operandLocation, baseAddr.getType(), prefix,
+          modBuilder, builder, operandLocation, info.addr.getType(), prefix,
           asFortran, dataClause);
       if constexpr (!std::is_same_v<EntryOp, ExitOp>)
         createDeclareDeallocFuncWithArg<ExitOp>(
-            modBuilder, builder, operandLocation, baseAddr.getType(), prefix,
+            modBuilder, builder, operandLocation, info.addr.getType(), prefix,
             asFortran, dataClause);
     }
   }
@@ -749,21 +770,21 @@ genPrivatizations(const Fortran::parser::AccObjectList &objectList,
     llvm::SmallVector<mlir::Value> bounds;
     std::stringstream asFortran;
     mlir::Location operandLocation = genOperandLocation(converter, accObject);
-    mlir::Value baseAddr = Fortran::lower::gatherDataOperandAddrAndBounds<
-        Fortran::parser::AccObject, mlir::acc::DataBoundsOp,
-        mlir::acc::DataBoundsType>(converter, builder, semanticsContext,
-                                   stmtCtx, accObject, operandLocation,
-                                   asFortran, bounds);
-
+    Fortran::lower::AddrAndBoundsInfo info =
+        Fortran::lower::gatherDataOperandAddrAndBounds<
+            Fortran::parser::AccObject, mlir::acc::DataBoundsOp,
+            mlir::acc::DataBoundsType>(converter, builder, semanticsContext,
+                                       stmtCtx, accObject, operandLocation,
+                                       asFortran, bounds);
     RecipeOp recipe;
-    mlir::Type retTy = getTypeFromBounds(bounds, baseAddr.getType());
+    mlir::Type retTy = getTypeFromBounds(bounds, info.addr.getType());
     if constexpr (std::is_same_v<RecipeOp, mlir::acc::PrivateRecipeOp>) {
       std::string recipeName =
           fir::getTypeAsString(retTy, converter.getKindMap(), "privatization");
       recipe = Fortran::lower::createOrGetPrivateRecipe(builder, recipeName,
                                                         operandLocation, retTy);
       auto op = createDataEntryOp<mlir::acc::PrivateOp>(
-          builder, operandLocation, baseAddr, asFortran, bounds, true,
+          builder, operandLocation, info.addr, asFortran, bounds, true,
           /*implicit=*/false, mlir::acc::DataClause::acc_private, retTy);
       dataOperands.push_back(op.getAccPtr());
     } else {
@@ -774,7 +795,7 @@ genPrivatizations(const Fortran::parser::AccObjectList &objectList,
       recipe = Fortran::lower::createOrGetFirstprivateRecipe(
           builder, recipeName, operandLocation, retTy, bounds);
       auto op = createDataEntryOp<mlir::acc::FirstprivateOp>(
-          builder, operandLocation, baseAddr, asFortran, bounds, true,
+          builder, operandLocation, info.addr, asFortran, bounds, true,
           /*implicit=*/false, mlir::acc::DataClause::acc_firstprivate, retTy);
       dataOperands.push_back(op.getAccPtr());
     }
@@ -1326,13 +1347,14 @@ genReductions(const Fortran::parser::AccObjectListWithReduction &objectList,
     llvm::SmallVector<mlir::Value> bounds;
     std::stringstream asFortran;
     mlir::Location operandLocation = genOperandLocation(converter, accObject);
-    mlir::Value baseAddr = Fortran::lower::gatherDataOperandAddrAndBounds<
-        Fortran::parser::AccObject, mlir::acc::DataBoundsOp,
-        mlir::acc::DataBoundsType>(converter, builder, semanticsContext,
-                                   stmtCtx, accObject, operandLocation,
-                                   asFortran, bounds);
-
-    mlir::Type reductionTy = fir::unwrapRefType(baseAddr.getType());
+    Fortran::lower::AddrAndBoundsInfo info =
+        Fortran::lower::gatherDataOperandAddrAndBounds<
+            Fortran::parser::AccObject, mlir::acc::DataBoundsOp,
+            mlir::acc::DataBoundsType>(converter, builder, semanticsContext,
+                                       stmtCtx, accObject, operandLocation,
+                                       asFortran, bounds);
+
+    mlir::Type reductionTy = fir::unwrapRefType(info.addr.getType());
     if (auto seqTy = mlir::dyn_cast<fir::SequenceType>(reductionTy))
       reductionTy = seqTy.getEleTy();
 
@@ -1340,14 +1362,14 @@ genReductions(const Fortran::parser::AccObjectListWithReduction &objectList,
       TODO(operandLocation, "reduction with unsupported type");
 
     auto op = createDataEntryOp<mlir::acc::ReductionOp>(
-        builder, operandLocation, baseAddr, asFortran, bounds,
+        builder, operandLocation, info.addr, asFortran, bounds,
         /*structured=*/true, /*implicit=*/false,
-        mlir::acc::DataClause::acc_reduction, baseAddr.getType());
+        mlir::acc::DataClause::acc_reduction, info.addr.getType());
     mlir::Type ty = op.getAccPtr().getType();
     if (!areAllBoundConstant(bounds) ||
-        fir::isAssumedShape(baseAddr.getType()) ||
-        fir::isAllocatableOrPointerArray(baseAddr.getType()))
-      ty = baseAddr.getType();
+        fir::isAssumedShape(info.addr.getType()) ||
+        fir::isAllocatableOrPointerArray(info.addr.getType()))
+      ty = info.addr.getType();
     std::string suffix =
         areAllBoundConstant(bounds) ? getBoundsString(bounds) : "";
     std::string recipeName = fir::getTypeAsString(
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 22d7cc24418867..9213cff95d3f11 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -1794,27 +1794,28 @@ bool ClauseProcessor::processMap(
              std::get<Fortran::parser::OmpObjectList>(mapClause->v.t).v) {
           llvm::SmallVector<mlir::Value> bounds;
           std::stringstream asFortran;
-          mlir::Value baseAddr = Fortran::lower::gatherDataOperandAddrAndBounds<
-              Fortran::parser::OmpObject, mlir::omp::DataBoundsOp,
-              mlir::omp::DataBoundsType>(
-              converter, firOpBuilder, semanticsContext, stmtCtx, ompObject,
-              clauseLocation, asFortran, bounds, treatIndexAsSection);
+          Fortran::lower::AddrAndBoundsInfo info =
+              Fortran::lower::gatherDataOperandAddrAndBounds<
+                  Fortran::parser::OmpObject, mlir::omp::DataBoundsOp,
+                  mlir::omp::DataBoundsType>(
+                  converter, firOpBuilder, semanticsContext, stmtCtx, ompObject,
+                  clauseLocation, asFortran, bounds, treatIndexAsSection);
 
           // Explicit map captures are captured ByRef by default,
           // optimisation passes may alter this to ByCopy or other capture
           // types to optimise
           mlir::Value mapOp = createMapInfoOp(
-              firOpBuilder, clauseLocation, baseAddr, asFortran, bounds,
+              firOpBuilder, clauseLocation, info.addr, asFortran, bounds,
               static_cast<
                   std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
                   mapTypeBits),
-              mlir::omp::VariableCaptureKind::ByRef, baseAddr.getType());
+              mlir::omp::VariableCaptureKind::ByRef, info.addr.getType());
 
           mapOperands.push_back(mapOp);
           if (mapSymTypes)
-            mapSymTypes->push_back(baseAddr.getType());
+            mapSymTypes->push_back(info.addr.getType());
           if (mapSymLocs)
-            mapSymLocs->push_back(baseAddr.getLoc());
+            mapSymLocs->push_back(info.addr.getLoc());
           if (mapSymbols)
             mapSymbols->push_back(getOmpObjectSymbol(ompObject));
         }
@@ -2655,16 +2656,16 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
         fir::ExtendedValue dataExv = converter.getSymbolExtendedValue(sym);
         name << sym.name().ToString();
 
-        mlir::Value baseAddr =
+        Fortran::lower::AddrAndBoundsInfo info =
             getDataOperandBaseAddr(converter, converter.getFirOpBuilder(), sym,
                                    converter.getCurrentLocation());
-        if (fir::unwrapRefType(baseAddr.getType()).isa<fir::BaseBoxType>())
+        if (fir::unwrapRefType(info.addr.getType()).isa<fir::BaseBoxType>())
           bounds =
               Fortran::lower::genBoundsOpsFromBox<mlir::omp::DataBoundsOp,
                                                   mlir::omp::DataBoundsType>(
                   converter.getFirOpBuilder(), converter.getCurrentLocation(),
-                  converter, dataExv, baseAddr);
-        if (fir::unwrapRefType(baseAddr.getType()).isa<fir::SequenceType>())
+                  converter, dataExv, info);
+        if (fir::unwrapRefType(info.addr.getType()).isa<fir::SequenceType>())
           bounds = Fortran::lower::genBaseBoundsOps<mlir::omp::DataBoundsOp,
                                                     mlir::omp::DataBoundsType>(
               converter.getFirOpBuilder(), converter.getCurrentLocation(),
diff --git a/flang/test/Lower/OpenACC/acc-bounds.f90 b/flang/test/Lower/OpenACC/acc-bounds.f90
index 8db18ab5aa9c4b..9e8e54bc2f7fa6 100644
--- a/flang/test/Lower/OpenACC/acc-bounds.f90
+++ b/flang/test/Lower/OpenACC/acc-bounds.f90
@@ -116,4 +116,39 @@ subroutine acc_multi_strides(a)
 ! CHECK: %[[PRESENT:.*]] = acc.present varPtr(%[[BOX_ADDR]] : !fir.ref<!fir.array<?x?x?xf32>>) bounds(%29, %33, %37) -> !fir.ref<!fir.array<?x?x?xf32>> {name = "a"}
 ! CHECK: acc.kernels dataOperands(%[[PRESENT]] : !fir.ref<!fir.array<?x?x?xf32>>) {
 
+  subroutine acc_optional_data(a)
+    real, pointer, optional :: a(:)
+    !$acc data attach(a)
+    !$acc end data
+  end subroutine
+  
+! CHECK-LABEL: func.func @_QMopenacc_boundsPacc_optional_data(
+! CHECK-SAME: %[[ARG0:.*]]: !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>> {fir.bindc_name = "a", fir.optional}) {
+! CHECK: %[[ARG0_DECL:.*]]:2 = hlfir.declare %arg0 {fortran_attrs = #fir.var_attrs<optional, pointer>, uniq_name = "_QMopenacc_boundsFacc_optional_dataEa"} : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>)
+! CHECK: %[[IS_PRESENT:.*]] = fir.is_present %[[ARG0_DECL]]#1 : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>) -> i1
+! CHECK: %[[BOX:.*]] = fir.if %[[IS_PRESENT]] -> (!fir.box<!fir.ptr<!fir.array<?xf32>>>) {
+! CHECK:   %[[LOAD:.*]] = fir.load %[[ARG0_DECL]]#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xf32>>>>
+! CHECK:   fir.result %[[LOAD]] : !fir.box<!fir.ptr<!fir.array<?xf32>>>
+! CHECK: } else {
+! CHECK:   %[[ABSENT:.*]] = fir.absent !fir.box<!fir.ptr<!fir.array<?xf32>>>
+! CHECK:   fir.result %[[ABSENT]] : !fir.box<!fir.ptr<!fir.array<?xf32>>>
+! CHECK: }
+! CHECK: %[[RES:.*]]:5 = fir.if %[[IS_PRESENT]] -> (index, index, index, index, index) {
+! CHECK:   fir.result %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}} : index, index, index, index, index
+! CHECK: } else {
+! CHECK:   %[[C0:.*]] = arith.constant 0 : index
+! CHECK:   %[[CM1:.*]] = arith.constant -1 : index
+! CHECK:   fir.result %[[C0]], %[[CM1]], %[[C0]], %[[C0]], %[[C0]] : index, index, index, index, index
+! CHECK: }
+! CHECK: %[[BOUND:.*]] = acc.bounds lowerbound(%[[RES]]#0 : index) upperbound(%[[RES]]#1 : index) extent(%[[RES]]#2 : index) stride(%[[RES]]#3 : index) startIdx(%[[RES]]#4 : index) {strideInBytes = true}
+! CHECK: %[[BOX_ADDR:.*]] = fir.if %[[IS_PRESENT]] -> (!fir.ptr<!fir.array<?xf32>>) {
+! CHECK:   %[[ADDR:.*]] = fir.box_addr %[[BOX]] : (!fir.box<!fir.ptr<!fir.array<?xf32>>>) -> !fir.ptr<!fir.array<?xf32>> 
+! CHECK:   fir.result %[[ADDR]] : !fir.ptr<!fir.array<?xf32>>
+! CHECK: } else {
+! CHECK:   %[[ABSENT:.*]] = fir.absent !fir.ptr<!fir.array<?xf32>>
+! CHECK:   fir.result %[[ABSENT]] : !fir.ptr<!fir.array<?xf32>>
+! CHECK: }
+! CHECK: %[[ATTACH:.*]] = acc.attach varPtr(%[[BOX_ADDR]] : !fir.ptr<!fir.array<?xf32>>) bounds(%[[BOUND]]) -> !fir.ptr<!fir.array<?xf32>> {name = "a"}
+! CHECK: acc.data dataOperands(%[[ATTACH]] : !fir.ptr<!fir.array<?xf32>>)
+
 end module
diff --git a/flang/test/Lower/OpenACC/acc-data.f90 b/flang/test/Lower/OpenACC/acc-data.f90
index d302be85c5df46..a6572e14707606 100644
--- a/flang/test/Lower/OpenACC/acc-data.f90
+++ b/flang/test/Lower/OpenACC/acc-data.f90
@@ -198,4 +198,3 @@ subroutine acc_data
 ! CHECK-NOT: acc.data
 
 end subroutine acc_data
-

>From 74228ac77e7b85d67cb3a8f233fb0754c0eaf772 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Thu, 14 Dec 2023 10:49:39 -0800
Subject: [PATCH 2/2] Remove commented code

---
 flang/lib/Lower/DirectivesCommon.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index e0966583e64259..ffbd8ae1558ed5 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -54,7 +54,6 @@ struct AddrAndBoundsInfo {
   explicit AddrAndBoundsInfo(mlir::Value addr) : addr(addr) {}
   explicit AddrAndBoundsInfo(mlir::Value addr, mlir::Value isPresent)
       : addr(addr), isPresent(isPresent) {}
-  // const Fortran::semantics::Symbol *sym;
   mlir::Value addr = nullptr;
   mlir::Value isPresent = nullptr;
 };



More information about the flang-commits mailing list