[Flang][OpenMP] - When mapping a fir.boxchar, map the underlying data pointer as a member

flang/include/flang/Optimizer/Builder/DirectivesCommon.h

@@ -91,6 +91,16 @@ inline AddrAndBoundsInfo getDataOperandBaseAddr(fir::FirOpBuilder &builder,
 
     return AddrAndBoundsInfo(symAddr, rawInput, isPresent, boxTy);
   }
+  // For boxchar references, do the same as what is done above for box
+  // references - Load the boxchar so that it is easier to retrieve the length
+  // of the underlying character and the data pointer.
+  if (auto boxCharType = mlir::dyn_cast<fir::BoxCharType>(
+          fir::unwrapRefType((symAddr.getType())))) {
+    if (!isOptional && mlir::isa<fir::ReferenceType>(symAddr.getType())) {
+      mlir::Value boxChar = builder.create<fir::LoadOp>(loc, symAddr);
+      return AddrAndBoundsInfo(boxChar, rawInput, isPresent);
+    }
+  }
   return AddrAndBoundsInfo(symAddr, rawInput, isPresent);
 }
 
@@ -137,26 +147,64 @@ template <typename BoundsOp, typename BoundsType>
 mlir::Value
 genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
                        fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
-  // TODO: Handle info.isPresent.
-  if (auto boxCharType =
-          mlir::dyn_cast<fir::BoxCharType>(info.addr.getType())) {
-    mlir::Type idxTy = builder.getIndexType();
-    mlir::Type lenType = builder.getCharacterLengthType();
+
+  if (!mlir::isa<fir::BoxCharType>(fir::unwrapRefType(info.addr.getType())))
+    return mlir::Value{};
+
+  mlir::Type idxTy = builder.getIndexType();
+  mlir::Type lenType = builder.getCharacterLengthType();
+  mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
+  mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
+  using ExtentAndStride = std::tuple<mlir::Value, mlir::Value>;
+  auto [extent, stride] = [&]() -> ExtentAndStride {
+    if (info.isPresent) {
+      llvm::SmallVector<mlir::Type> resTypes = {idxTy, idxTy};
+      mlir::Operation::result_range ifRes =
+          builder
+              .genIfOp(loc, resTypes, info.isPresent, /*withElseRegion=*/true)
+              .genThen([&]() {
+                mlir::Value boxChar =
+                    fir::isa_ref_type(info.addr.getType())
+                        ? builder.create<fir::LoadOp>(loc, info.addr)
+                        : info.addr;
+                fir::BoxCharType boxCharType =
+                    mlir::cast<fir::BoxCharType>(boxChar.getType());
+                mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
+                auto unboxed = builder.create<fir::UnboxCharOp>(
+                    loc, refType, lenType, boxChar);
+                mlir::SmallVector<mlir::Value> results = {unboxed.getResult(1),
+                                                          one};
+                builder.create<fir::ResultOp>(loc, results);
+              })
+              .genElse([&]() {
+                mlir::SmallVector<mlir::Value> results = {zero, zero};
+                builder.create<fir::ResultOp>(loc, results);
+              })
+              .getResults();
+      return {ifRes[0], ifRes[1]};
+    }
+    // We have already established that info.addr.getType() is a boxchar
+    // or a boxchar address. If an address, load the boxchar.
+    mlir::Value boxChar = fir::isa_ref_type(info.addr.getType())
+                              ? builder.create<fir::LoadOp>(loc, info.addr)
+                              : info.addr;
+    fir::BoxCharType boxCharType =
+        mlir::cast<fir::BoxCharType>(boxChar.getType());
     mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
     auto unboxed =
-        builder.create<fir::UnboxCharOp>(loc, refType, lenType, info.addr);
-    mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
-    mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-    mlir::Value extent = unboxed.getResult(1);
-    mlir::Value stride = one;
-    mlir::Value ub = builder.create<mlir::arith::SubIOp>(loc, extent, one);
-    mlir::Type boundTy = builder.getType<mlir::omp::MapBoundsType>();
-    return builder.create<mlir::omp::MapBoundsOp>(
-        loc, boundTy, /*lower_bound=*/zero,
-        /*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
-        /*stride_in_bytes=*/true, /*start_idx=*/zero);
-  }
-  return mlir::Value{};
+        builder.create<fir::UnboxCharOp>(loc, refType, lenType, boxChar);
+    return {unboxed.getResult(1), one};
+  }();
+
+  mlir::Value ub = builder.create<mlir::arith::SubIOp>(loc, extent, one);
+  mlir::Type boundTy = builder.getType<BoundsType>();
+  return builder.create<BoundsOp>(loc, boundTy,
+                                  /*lower_bound=*/zero,
+                                  /*upper_bound=*/ub,
+                                  /*extent=*/extent,
+                                  /*stride=*/stride,
+                                  /*stride_in_bytes=*/true,
+                                  /*start_idx=*/zero);
 }
 
 /// Generate the bounds operation from the descriptor information.
@@ -296,11 +344,11 @@ genImplicitBoundsOps(fir::FirOpBuilder &builder, AddrAndBoundsInfo &info,
     bounds = genBaseBoundsOps<BoundsOp, BoundsType>(builder, loc, dataExv,
                                                     dataExvIsAssumedSize);
   }
-  if (characterWithDynamicLen(fir::unwrapRefType(baseOp.getType()))) {
+  if (characterWithDynamicLen(fir::unwrapRefType(baseOp.getType())) ||
+      mlir::isa<fir::BoxCharType>(fir::unwrapRefType(info.addr.getType()))) {
     bounds = {genBoundsOpFromBoxChar<BoundsOp, BoundsType>(builder, loc,
                                                            dataExv, info)};
   }
-
   return bounds;
 }
 

flang/include/flang/Optimizer/Dialect/FIROps.td

@@ -3240,11 +3240,17 @@ def fir_BoxOffsetOp : fir_Op<"box_offset", [NoMemoryEffect]> {
     descriptor implementation must have, only the base_addr and derived_type
     descriptor fields can be addressed.
 
+    It also accepts the address of a fir.boxchar and returns
+    address of the data pointer encapsulated by the fir.boxchar.
+
     ```
         %addr = fir.box_offset %box base_addr : (!fir.ref<!fir.box<!fir.array<?xi32>>>) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>
         %tdesc = fir.box_offset %box derived_type : (!fir.ref<!fir.box<!fir.type<t>>>) -> !fir.llvm_ptr<!fir.tdesc<!fir.type<t>>>
 
+        %addr1 = fir.box_offset %boxchar base_addr : (!fir.ref<!fir.boxchar<1>>) -> !fir.llvm_ptr<!fir.ref<fir.char<1,?>>>
     ```
+
+    The derived_type field cannot be used when the input to this op is a reference to a fir.boxchar.
   }];
 
   let arguments = (ins

flang/lib/Optimizer/CodeGen/CodeGen.cpp

@@ -3930,12 +3930,25 @@ struct BoxOffsetOpConversion : public fir::FIROpConversion<fir::BoxOffsetOp> {
                   mlir::ConversionPatternRewriter &rewriter) const override {
 
     mlir::Type pty = ::getLlvmPtrType(boxOffset.getContext());
-    mlir::Type boxType = fir::unwrapRefType(boxOffset.getBoxRef().getType());
-    mlir::Type llvmBoxTy =
-        lowerTy().convertBoxTypeAsStruct(mlir::cast<fir::BaseBoxType>(boxType));
-    int fieldId = boxOffset.getField() == fir::BoxFieldAttr::derived_type
-                      ? getTypeDescFieldId(boxType)
-                      : kAddrPosInBox;
+    mlir::Type boxRefType = fir::unwrapRefType(boxOffset.getBoxRef().getType());
+
+    assert((mlir::isa<fir::BaseBoxType>(boxRefType) ||
+            mlir::isa<fir::BoxCharType>(boxRefType)) &&
+           "boxRef should be a reference to either fir.box or fir.boxchar");
+
+    mlir::Type llvmBoxTy;
+    int fieldId;
+    if (auto boxType = mlir::dyn_cast_or_null<fir::BaseBoxType>(boxRefType)) {
+      llvmBoxTy = lowerTy().convertBoxTypeAsStruct(
+          mlir::cast<fir::BaseBoxType>(boxType));
+      fieldId = boxOffset.getField() == fir::BoxFieldAttr::derived_type
+                    ? getTypeDescFieldId(boxType)
+                    : kAddrPosInBox;
+    } else {
+      auto boxCharType = mlir::cast<fir::BoxCharType>(boxRefType);
+      llvmBoxTy = lowerTy().convertType(boxCharType);
+      fieldId = kAddrPosInBox;
+    }
     rewriter.replaceOpWithNewOp<mlir::LLVM::GEPOp>(
         boxOffset, pty, llvmBoxTy, adaptor.getBoxRef(),
         llvm::ArrayRef<mlir::LLVM::GEPArg>{0, fieldId});

flang/lib/Optimizer/Dialect/FIROps.cpp

@@ -4484,15 +4484,24 @@ void fir::IfOp::resultToSourceOps(llvm::SmallVectorImpl<mlir::Value> &results,
 llvm::LogicalResult fir::BoxOffsetOp::verify() {
   auto boxType = mlir::dyn_cast_or_null<fir::BaseBoxType>(
       fir::dyn_cast_ptrEleTy(getBoxRef().getType()));
-  if (!boxType)
-    return emitOpError("box_ref operand must have !fir.ref<!fir.box<T>> type");
+  mlir::Type boxCharType;
+  if (!boxType) {
+    boxCharType = mlir::dyn_cast_or_null<fir::BoxCharType>(
+        fir::dyn_cast_ptrEleTy(getBoxRef().getType()));
+    if (!boxCharType)
+      return emitOpError("box_ref operand must have !fir.ref<!fir.box<T>> or "
+                         "!fir.ref<!fir.boxchar<k>> type");
+    if (getField() == fir::BoxFieldAttr::derived_type)
+      return emitOpError("cannot address derived_type field of a fir.boxchar");
+  }
   if (getField() != fir::BoxFieldAttr::base_addr &&
       getField() != fir::BoxFieldAttr::derived_type)
     return emitOpError("cannot address provided field");
-  if (getField() == fir::BoxFieldAttr::derived_type)
+  if (getField() == fir::BoxFieldAttr::derived_type) {
     if (!fir::boxHasAddendum(boxType))
       return emitOpError("can only address derived_type field of derived type "
                          "or unlimited polymorphic fir.box");
+  }
   return mlir::success();
 }
 

flang/lib/Optimizer/Dialect/FIRType.cpp

@@ -255,7 +255,7 @@ mlir::Type dyn_cast_ptrOrBoxEleTy(mlir::Type t) {
   return llvm::TypeSwitch<mlir::Type, mlir::Type>(t)
       .Case<fir::ReferenceType, fir::PointerType, fir::HeapType,
             fir::LLVMPointerType>([](auto p) { return p.getEleTy(); })
-      .Case<fir::BaseBoxType>(
+      .Case<fir::BaseBoxType, fir::BoxCharType>(
           [](auto p) { return unwrapRefType(p.getEleTy()); })
       .Default([](mlir::Type) { return mlir::Type{}; });
 }

flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp

@@ -48,7 +48,7 @@
 #include <numeric>
 
 #define DEBUG_TYPE "omp-map-info-finalization"
-#define PDBGS() (llvm::dbgs() << "[" << DEBUG_TYPE << "]: ")
+
 namespace flangomp {
 #define GEN_PASS_DEF_MAPINFOFINALIZATIONPASS
 #include "flang/Optimizer/OpenMP/Passes.h.inc"
@@ -285,6 +285,62 @@ class MapInfoFinalizationPass
     return false;
   }
 
+  mlir::omp::MapInfoOp genBoxcharMemberMap(mlir::omp::MapInfoOp op,
+                                           fir::FirOpBuilder &builder) {
+    if (!op.getMembers().empty())
+      return op;
+    mlir::Location loc = op.getVarPtr().getLoc();
+    mlir::Value boxChar = op.getVarPtr();
+
+    if (mlir::isa<fir::ReferenceType>(op.getVarPtr().getType()))
+      boxChar = builder.create<fir::LoadOp>(loc, op.getVarPtr());
+
+    fir::BoxCharType boxCharType =
+        mlir::dyn_cast<fir::BoxCharType>(boxChar.getType());
+    mlir::Value boxAddr = builder.create<fir::BoxOffsetOp>(
+        loc, op.getVarPtr(), fir::BoxFieldAttr::base_addr);
+
+    uint64_t mapTypeToImplicit = static_cast<
+        std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+        llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO |
+        llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT);
+
+    mlir::ArrayAttr newMembersAttr;
+    llvm::SmallVector<llvm::SmallVector<int64_t>> memberIdx = {{0}};
+    newMembersAttr = builder.create2DI64ArrayAttr(memberIdx);
+
+    mlir::Value varPtr = op.getVarPtr();
+    mlir::omp::MapInfoOp memberMapInfoOp = builder.create<mlir::omp::MapInfoOp>(
+        op.getLoc(), varPtr.getType(), varPtr,
+        mlir::TypeAttr::get(boxCharType.getEleTy()),
+        builder.getIntegerAttr(builder.getIntegerType(64, /*isSigned=*/false),
+                               mapTypeToImplicit),
+        builder.getAttr<mlir::omp::VariableCaptureKindAttr>(
+            mlir::omp::VariableCaptureKind::ByRef),
+        /*varPtrPtr=*/boxAddr,
+        /*members=*/llvm::SmallVector<mlir::Value>{},
+        /*member_index=*/mlir::ArrayAttr{},
+        /*bounds=*/op.getBounds(),
+        /*mapperId=*/mlir::FlatSymbolRefAttr(), /*name=*/op.getNameAttr(),
+        builder.getBoolAttr(false));
+
+    mlir::omp::MapInfoOp newMapInfoOp = builder.create<mlir::omp::MapInfoOp>(
+        op.getLoc(), op.getResult().getType(), varPtr,
+        mlir::TypeAttr::get(
+            llvm::cast<mlir::omp::PointerLikeType>(varPtr.getType())
+                .getElementType()),
+        op.getMapTypeAttr(), op.getMapCaptureTypeAttr(),
+        /*varPtrPtr=*/mlir::Value{},
+        /*members=*/llvm::SmallVector<mlir::Value>{memberMapInfoOp},
+        /*member_index=*/newMembersAttr,
+        /*bounds=*/llvm::SmallVector<mlir::Value>{},
+        /*mapperId=*/mlir::FlatSymbolRefAttr(), op.getNameAttr(),
+        /*partial_map=*/builder.getBoolAttr(false));
+    op.replaceAllUsesWith(newMapInfoOp.getResult());
+    op->erase();
+    return newMapInfoOp;
+  }
+
   mlir::omp::MapInfoOp genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
                                                fir::FirOpBuilder &builder,
                                                mlir::Operation *target) {
@@ -575,6 +631,7 @@ class MapInfoFinalizationPass
         fir::factory::AddrAndBoundsInfo info =
             fir::factory::getDataOperandBaseAddr(
                 builder, varPtr, /*isOptional=*/false, varPtr.getLoc());
+
         fir::ExtendedValue extendedValue =
             hlfir::translateToExtendedValue(varPtr.getLoc(), builder,
                                             hlfir::Entity{info.addr},
@@ -743,6 +800,37 @@ class MapInfoFinalizationPass
         return mlir::WalkResult::advance();
       });
 
+      func->walk([&](mlir::omp::MapInfoOp op) {
+        if (!op.getMembers().empty())
+          return;
+
+        if (!mlir::isa<fir::BoxCharType>(fir::unwrapRefType(op.getVarType())))
+          return;
+
+        // POSSIBLE_HACK_ALERT: If the boxchar has been implicitly mapped then
+        // it is likely that the underlying pointer to the data
+        // (!fir.ref<fir.char<k,?>>) has already been mapped. So, skip such
+        // boxchars. We are primarily interested in boxchars that were mapped
+        // by passes such as MapsForPrivatizedSymbols that map boxchars that
+        // are privatized. At present, such boxchar maps are not marked
+        // implicit. Should they be? I don't know. If they should be then
+        // we need to change this check for early return OR live with
+        // over-mapping.
+        bool hasImplicitMap =
+            (llvm::omp::OpenMPOffloadMappingFlags(op.getMapType()) &
+             llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT) ==
+            llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT;
+        if (hasImplicitMap)
+          return;
+
+        assert(llvm::hasSingleElement(op->getUsers()) &&
+               "OMPMapInfoFinalization currently only supports single users "
+               "of a MapInfoOp");
+
+        builder.setInsertionPoint(op);
+        genBoxcharMemberMap(op, builder);
+      });
+
       func->walk([&](mlir::omp::MapInfoOp op) {
         // TODO: Currently only supports a single user for the MapInfoOp. This
         // is fine for the moment, as the Fortran frontend will generate a

flang/lib/Optimizer/OpenMP/MapsForPrivatizedSymbols.cpp

@@ -22,7 +22,9 @@
 // 2. Generalize this for more than just omp.target ops.
 //===----------------------------------------------------------------------===//
 
+#include "flang/Optimizer/Builder/DirectivesCommon.h"
 #include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Builder/HLFIRTools.h"
 #include "flang/Optimizer/Dialect/FIRType.h"
 #include "flang/Optimizer/Dialect/Support/KindMapping.h"
 #include "flang/Optimizer/HLFIR/HLFIROps.h"
@@ -184,37 +186,23 @@ class MapsForPrivatizedSymbolsPass
     return fir::hasDynamicSize(t);
   }
 
-  // TODO: Remove this in favor of fir::factory::genImplicitBoundsOps
-  // in a subsequent PR.
   void genBoundsOps(fir::FirOpBuilder &builder, mlir::Value var,
                     llvm::SmallVector<mlir::Value> &boundsOps) {
-    if (!fir::isBoxAddress(var.getType()))
-      return;
-
-    unsigned int rank = 0;
-    rank = fir::getBoxRank(fir::unwrapRefType(var.getType()));
     mlir::Location loc = var.getLoc();
-    mlir::Type idxTy = builder.getIndexType();
-    mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-    mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
-    mlir::Type boundTy = builder.getType<omp::MapBoundsType>();
-    mlir::Value box = builder.create<fir::LoadOp>(loc, var);
-    for (unsigned int i = 0; i < rank; ++i) {
-      mlir::Value dimNo = builder.createIntegerConstant(loc, idxTy, i);
-      auto dimInfo =
-          builder.create<fir::BoxDimsOp>(loc, idxTy, idxTy, idxTy, box, dimNo);
-      mlir::Value lb = dimInfo.getLowerBound();
-      mlir::Value extent = dimInfo.getExtent();
-      mlir::Value byteStride = dimInfo.getByteStride();
-      mlir::Value ub = builder.create<mlir::arith::SubIOp>(loc, extent, one);
-
-      mlir::Value boundsOp = builder.create<omp::MapBoundsOp>(
-          loc, boundTy, /*lower_bound=*/zero,
-          /*upper_bound=*/ub, /*extent=*/extent, /*stride=*/byteStride,
-          /*stride_in_bytes = */ true, /*start_idx=*/lb);
-      LLVM_DEBUG(PDBGS() << "Created BoundsOp " << boundsOp << "\n");
-      boundsOps.push_back(boundsOp);
-    }
+    fir::factory::AddrAndBoundsInfo info =
+        fir::factory::getDataOperandBaseAddr(builder, var,
+                                             /*isOptional=*/false, loc);
+    fir::ExtendedValue extendedValue =
+        hlfir::translateToExtendedValue(loc, builder, hlfir::Entity{info.addr},
+                                        /*continguousHint=*/true)
+            .first;
+    llvm::SmallVector<mlir::Value> boundsOpsVec =
+        fir::factory::genImplicitBoundsOps<mlir::omp::MapBoundsOp,
+                                           mlir::omp::MapBoundsType>(
+            builder, info, extendedValue,
+            /*dataExvIsAssumedSize=*/false, loc);
+    for (auto bounds : boundsOpsVec)
+      boundsOps.push_back(bounds);
   }
 };
 } // namespace

flang/test/Fir/box-offset-codegen.fir

@@ -37,3 +37,13 @@ func.func @array_tdesc(%array : !fir.ref<!fir.class<!fir.ptr<!fir.array<?x!fir.t
 // CHECK-SAME: ptr captures(none) %[[BOX:.*]]){{.*}}{
 // CHECK:    %[[VAL_0:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]], ptr, [1 x i64] }, ptr %[[BOX]], i32 0, i32 8
 // CHECK:    ret ptr %[[VAL_0]]
+
+func.func @boxchar_addr(%boxchar : !fir.ref<!fir.boxchar<1>>) -> !fir.llvm_ptr<!fir.ref<!fir.char<1,?>>> {
+  %addr = fir.box_offset %boxchar base_addr : (!fir.ref<!fir.boxchar<1>>) -> !fir.llvm_ptr<!fir.ref<!fir.char<1,?>>>
+  return %addr : !fir.llvm_ptr<!fir.ref<!fir.char<1,?>>>
+}
+
+// CHECK-LABEL: define ptr @boxchar_addr(
+// CHECK-SAME: ptr {{.*}}%[[BOXCHAR:.*]]){{.*}} {
+// CHECK: %[[VAL_0:.*]] = getelementptr { ptr, i64 }, ptr %[[BOXCHAR]], i32 0, i32 0
+// CHECK: ret ptr %[[VAL_0]]