[flang] [llvm] [Flang][OpenMP] Fix mapping of character type with LEN > 1 specified (PR #154172)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 9 07:15:59 PDT 2025
https://github.com/agozillon updated https://github.com/llvm/llvm-project/pull/154172
>From 73ddd7d5b358633913aa551f9f54202621e428ed Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Mon, 18 Aug 2025 13:05:59 -0500
Subject: [PATCH 1/4] [Flang][OpenMP] Fix mapping of character type with LEN >
1 specified
Currently, there's a number of issues with mapping characters with LEN's specified (strings effectively). They're
represented as a char type in FIR with a len parameter, and then later on they're expanded into an array of characters
when we're translating to the LLVM dialect. However, we don't generate a bounds for these at lowering. The fix in this
PR for this is to generate a bounds from the LEN parameter and attatch it to the map on lowering from FIR to the LLVM
dialect when we encounter this type.
---
flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp | 69 ++++++++++++-
.../bounds-generation-for-char-arrays.f90 | 96 +++++++++++++++++++
2 files changed, 163 insertions(+), 2 deletions(-)
create mode 100644 flang/test/Fir/OpenMP/bounds-generation-for-char-arrays.f90
diff --git a/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp b/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
index 97912bda79b08..1c3fc04779377 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
@@ -60,6 +60,21 @@ struct MapInfoOpConversion
: public OpenMPFIROpConversion<mlir::omp::MapInfoOp> {
using OpenMPFIROpConversion::OpenMPFIROpConversion;
+ mlir::omp::MapBoundsOp
+ createBoundsForCharString(mlir::ConversionPatternRewriter &rewriter,
+ unsigned int len, mlir::Location loc) const {
+ mlir::Type i64Ty = rewriter.getIntegerType(64);
+ auto lBound = mlir::LLVM::ConstantOp::create(rewriter, loc, i64Ty, 0);
+ auto uBoundAndExt =
+ mlir::LLVM::ConstantOp::create(rewriter, loc, i64Ty, len - 1);
+ auto stride = mlir::LLVM::ConstantOp::create(rewriter, loc, i64Ty, 1);
+ auto baseLb = mlir::LLVM::ConstantOp::create(rewriter, loc, i64Ty, 1);
+ auto mapBoundType = rewriter.getType<mlir::omp::MapBoundsType>();
+ return mlir::omp::MapBoundsOp::create(rewriter, loc, mapBoundType, lBound,
+ uBoundAndExt, uBoundAndExt, stride,
+ /*strideInBytes*/ false, baseLb);
+ }
+
llvm::LogicalResult
matchAndRewrite(mlir::omp::MapInfoOp curOp, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const override {
@@ -69,13 +84,58 @@ struct MapInfoOpConversion
return mlir::failure();
llvm::SmallVector<mlir::NamedAttribute> newAttrs;
- mlir::omp::MapInfoOp newOp;
+ mlir::omp::MapBoundsOp mapBoundsOp;
for (mlir::NamedAttribute attr : curOp->getAttrs()) {
if (auto typeAttr = mlir::dyn_cast<mlir::TypeAttr>(attr.getValue())) {
mlir::Type newAttr;
if (fir::isTypeWithDescriptor(typeAttr.getValue())) {
newAttr = lowerTy().convertBoxTypeAsStruct(
mlir::cast<fir::BaseBoxType>(typeAttr.getValue()));
+ } else if (fir::isa_char_string(fir::unwrapSequenceType(
+ fir::unwrapPassByRefType(typeAttr.getValue()))) &&
+ !characterWithDynamicLen(
+ fir::unwrapPassByRefType(typeAttr.getValue()))) {
+ // Characters with a LEN param are represented as char
+ // arrays/strings, the initial lowering doesn't generate
+ // bounds for these, however, we require them to map the
+ // data appropriately in the later lowering stages. This
+ // is to prevent the need for unecessary caveats
+ // specific to Flang. We also strip the array from the
+ // type so that all variations of strings are treated
+ // identically and there's no caveats or specialisations
+ // required in the later stages. As an example, Boxed
+ // char strings will emit a single char array no matter
+ // the number of dimensions caused by additional array
+ // dimensions which needs specialised for, as it differs
+ // from the non-box variation which will emit each array
+ // wrapping the character array, e.g. given a type of
+ // the same dimensions, if one is boxed, the types would
+ // end up:
+ //
+ // array<i8 x 16>
+ // vs
+ // array<10 x array< 10 x array<i8 x 16>>>
+ //
+ // This means we have to treat one specially in the
+ // lowering. So we try to "canonicalize" it here.
+ // TODO: Handle dynamic LEN characters.
+ if (auto ct = mlir::dyn_cast_or_null<fir::CharacterType>(
+ fir::unwrapSequenceType(typeAttr.getValue()))) {
+ newAttr = converter->convertType(
+ fir::unwrapSequenceType(typeAttr.getValue()));
+ if (auto type = mlir::dyn_cast<mlir::LLVM::LLVMArrayType>(newAttr))
+ newAttr = type.getElementType();
+ // We do not generate for device, as MapBoundsOps are
+ // unsupported, as they're currently unused.
+ auto offloadMod =
+ llvm::dyn_cast_or_null<mlir::omp::OffloadModuleInterface>(
+ *curOp->getParentOfType<mlir::ModuleOp>());
+ if (!offloadMod.getIsTargetDevice())
+ mapBoundsOp = createBoundsForCharString(rewriter, ct.getLen(),
+ curOp.getLoc());
+ } else {
+ newAttr = converter->convertType(typeAttr.getValue());
+ }
} else {
newAttr = converter->convertType(typeAttr.getValue());
}
@@ -85,8 +145,13 @@ struct MapInfoOpConversion
}
}
- rewriter.replaceOpWithNewOp<mlir::omp::MapInfoOp>(
+ auto newOp = rewriter.replaceOpWithNewOp<mlir::omp::MapInfoOp>(
curOp, resTypes, adaptor.getOperands(), newAttrs);
+ if (mapBoundsOp) {
+ rewriter.startOpModification(newOp);
+ newOp.getBoundsMutable().append(mlir::ValueRange{mapBoundsOp});
+ rewriter.finalizeOpModification(newOp);
+ }
return mlir::success();
}
diff --git a/flang/test/Fir/OpenMP/bounds-generation-for-char-arrays.f90 b/flang/test/Fir/OpenMP/bounds-generation-for-char-arrays.f90
new file mode 100644
index 0000000000000..d9d54ee72edb8
--- /dev/null
+++ b/flang/test/Fir/OpenMP/bounds-generation-for-char-arrays.f90
@@ -0,0 +1,96 @@
+// RUN: fir-opt --cfg-conversion --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s
+
+module attributes {omp.is_target_device = false} {
+ func.func @_QPchar_array(%arg0 : !fir.ref<!fir.array<10x10x!fir.char<1,16>>>) {
+ %c9 = arith.constant 9 : index
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %c10 = arith.constant 10 : index
+ %0 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%c9 : index) extent(%c10 : index) stride(%c1 : index) start_idx(%c1 : index)
+ %1 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%c9 : index) extent(%c10 : index) stride(%c1 : index) start_idx(%c1 : index)
+ %2 = omp.map.info var_ptr(%arg0 : !fir.ref<!fir.array<10x10x!fir.char<1,16>>>, !fir.array<10x10x!fir.char<1,16>>) map_clauses(tofrom) capture(ByRef) bounds(%0, %1) -> !fir.ref<!fir.array<10x10x!fir.char<1,16>>> {name = ""}
+ omp.target map_entries(%2 -> %arg1 : !fir.ref<!fir.array<10x10x!fir.char<1,16>>>) {
+ omp.terminator
+ }
+ return
+ }
+
+// CHECK-LABEL: llvm.func @_QPchar_array(
+// CHECK-SAME: %[[ARG0:.*]]: !llvm.ptr) {
+// CHECK: %[[VAL_0:.*]] = llvm.mlir.constant(9 : index) : i64
+// CHECK: %[[VAL_1:.*]] = llvm.mlir.constant(0 : index) : i64
+// CHECK: %[[VAL_2:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: %[[VAL_3:.*]] = llvm.mlir.constant(10 : index) : i64
+// CHECK: %[[VAL_4:.*]] = omp.map.bounds lower_bound(%[[VAL_1]] : i64) upper_bound(%[[VAL_0]] : i64) extent(%[[VAL_3]] : i64) stride(%[[VAL_2]] : i64) start_idx(%[[VAL_2]] : i64)
+// CHECK: %[[VAL_5:.*]] = omp.map.bounds lower_bound(%[[VAL_1]] : i64) upper_bound(%[[VAL_0]] : i64) extent(%[[VAL_3]] : i64) stride(%[[VAL_2]] : i64) start_idx(%[[VAL_2]] : i64)
+// CHECK: %[[VAL_6:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK: %[[VAL_7:.*]] = llvm.mlir.constant(15 : i64) : i64
+// CHECK: %[[VAL_8:.*]] = llvm.mlir.constant(1 : i64) : i64
+// CHECK: %[[VAL_9:.*]] = llvm.mlir.constant(1 : i64) : i64
+// CHECK: %[[VAL_10:.*]] = omp.map.bounds lower_bound(%[[VAL_6]] : i64) upper_bound(%[[VAL_7]] : i64) extent(%[[VAL_7]] : i64) stride(%[[VAL_8]] : i64) start_idx(%[[VAL_9]] : i64)
+// CHECK: %[[VAL_11:.*]] = omp.map.info var_ptr(%[[ARG0]] : !llvm.ptr, i8) map_clauses(tofrom) capture(ByRef) bounds(%[[VAL_4]], %[[VAL_5]], %[[VAL_10]]) -> !llvm.ptr {name = ""}
+// CHECK: omp.target map_entries(%[[VAL_11]] -> %[[VAL_12:.*]] : !llvm.ptr) {
+// CHECK: omp.terminator
+// CHECK: }
+// CHECK: llvm.return
+// CHECK: }
+
+ func.func @_QPallocatable_char_array(%arg0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>>) {
+ %c1 = arith.constant 1 : index
+ %c0 = arith.constant 0 : index
+ %0 = fir.load %arg0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>>
+ %1:3 = fir.box_dims %0, %c0 : (!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>, index) -> (index, index, index)
+ %2 = arith.subi %1#1, %c1 : index
+ %3 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%2 : index) extent(%1#1 : index) stride(%1#2 : index) start_idx(%1#0 : index) {stride_in_bytes = true}
+ %4 = arith.muli %1#2, %1#1 : index
+ %5:3 = fir.box_dims %0, %c1 : (!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>, index) -> (index, index, index)
+ %6 = arith.subi %5#1, %c1 : index
+ %7 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%6 : index) extent(%5#1 : index) stride(%4 : index) start_idx(%5#0 : index) {stride_in_bytes = true}
+ %8 = fir.box_offset %arg0 base_addr : (!fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>>) -> !fir.llvm_ptr<!fir.ref<!fir.array<?x?x!fir.char<1,16>>>>
+ %9 = omp.map.info var_ptr(%arg0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>>, !fir.char<1,16>) map_clauses(tofrom) capture(ByRef) var_ptr_ptr(%8 : !fir.llvm_ptr<!fir.ref<!fir.array<?x?x!fir.char<1,16>>>>) bounds(%3, %7) -> !fir.llvm_ptr<!fir.ref<!fir.array<?x?x!fir.char<1,16>>>> {name = ""}
+ %10 = omp.map.info var_ptr(%arg0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>>, !fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>) map_clauses(to) capture(ByRef) members(%9 : [0] : !fir.llvm_ptr<!fir.ref<!fir.array<?x?x!fir.char<1,16>>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>> {name = "csv_chem_list_a"}
+ omp.target map_entries(%10 -> %arg1, %9 -> %arg2 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>>, !fir.llvm_ptr<!fir.ref<!fir.array<?x?x!fir.char<1,16>>>>) {
+ omp.terminator
+ }
+ return
+ }
+
+// CHECK-LABEL: llvm.func @_QPallocatable_char_array(
+// CHECK-SAME: %[[ARG0:.*]]: !llvm.ptr) {
+// CHECK: %[[VAL_0:.*]] = llvm.mlir.constant(1 : i32) : i32
+// CHECK: %[[VAL_1:.*]] = llvm.alloca %[[VAL_0]] x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)> {alignment = 8 : i64} : (i32) -> !llvm.ptr
+// CHECK: %[[VAL_2:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: %[[VAL_3:.*]] = llvm.mlir.constant(0 : index) : i64
+// CHECK: %[[VAL_4:.*]] = llvm.mlir.constant(72 : i32) : i32
+// CHECK: "llvm.intr.memcpy"(%[[VAL_1]], %[[ARG0]], %[[VAL_4]]) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i32) -> ()
+// CHECK: %[[VAL_5:.*]] = llvm.getelementptr %[[VAL_1]][0, 7, %[[VAL_3]], 0] : (!llvm.ptr, i64) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)>
+// CHECK: %[[VAL_6:.*]] = llvm.load %[[VAL_5]] : !llvm.ptr -> i64
+// CHECK: %[[VAL_7:.*]] = llvm.getelementptr %[[VAL_1]][0, 7, %[[VAL_3]], 1] : (!llvm.ptr, i64) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)>
+// CHECK: %[[VAL_8:.*]] = llvm.load %[[VAL_7]] : !llvm.ptr -> i64
+// CHECK: %[[VAL_9:.*]] = llvm.getelementptr %[[VAL_1]][0, 7, %[[VAL_3]], 2] : (!llvm.ptr, i64) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)>
+// CHECK: %[[VAL_10:.*]] = llvm.load %[[VAL_9]] : !llvm.ptr -> i64
+// CHECK: %[[VAL_11:.*]] = llvm.sub %[[VAL_8]], %[[VAL_2]] : i64
+// CHECK: %[[VAL_12:.*]] = omp.map.bounds lower_bound(%[[VAL_3]] : i64) upper_bound(%[[VAL_11]] : i64) extent(%[[VAL_8]] : i64) stride(%[[VAL_10]] : i64) start_idx(%[[VAL_6]] : i64) {stride_in_bytes = true}
+// CHECK: %[[VAL_13:.*]] = llvm.mul %[[VAL_10]], %[[VAL_8]] : i64
+// CHECK: %[[VAL_14:.*]] = llvm.getelementptr %[[VAL_1]][0, 7, %[[VAL_2]], 0] : (!llvm.ptr, i64) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)>
+// CHECK: %[[VAL_15:.*]] = llvm.load %[[VAL_14]] : !llvm.ptr -> i64
+// CHECK: %[[VAL_16:.*]] = llvm.getelementptr %[[VAL_1]][0, 7, %[[VAL_2]], 1] : (!llvm.ptr, i64) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)>
+// CHECK: %[[VAL_17:.*]] = llvm.load %[[VAL_16]] : !llvm.ptr -> i64
+// CHECK: %[[VAL_18:.*]] = llvm.getelementptr %[[VAL_1]][0, 7, %[[VAL_2]], 2] : (!llvm.ptr, i64) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)>
+// CHECK: %[[VAL_19:.*]] = llvm.load %[[VAL_18]] : !llvm.ptr -> i64
+// CHECK: %[[VAL_20:.*]] = llvm.sub %[[VAL_17]], %[[VAL_2]] : i64
+// CHECK: %[[VAL_21:.*]] = omp.map.bounds lower_bound(%[[VAL_3]] : i64) upper_bound(%[[VAL_20]] : i64) extent(%[[VAL_17]] : i64) stride(%[[VAL_13]] : i64) start_idx(%[[VAL_15]] : i64) {stride_in_bytes = true}
+// CHECK: %[[VAL_22:.*]] = llvm.getelementptr %[[ARG0]][0, 0] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)>
+// CHECK: %[[VAL_23:.*]] = llvm.mlir.constant(0 : i64) : i64
+// CHECK: %[[VAL_24:.*]] = llvm.mlir.constant(15 : i64) : i64
+// CHECK: %[[VAL_25:.*]] = llvm.mlir.constant(1 : i64) : i64
+// CHECK: %[[VAL_26:.*]] = llvm.mlir.constant(1 : i64) : i64
+// CHECK: %[[VAL_27:.*]] = omp.map.bounds lower_bound(%[[VAL_23]] : i64) upper_bound(%[[VAL_24]] : i64) extent(%[[VAL_24]] : i64) stride(%[[VAL_25]] : i64) start_idx(%[[VAL_26]] : i64)
+// CHECK: %[[VAL_28:.*]] = omp.map.info var_ptr(%[[ARG0]] : !llvm.ptr, i8) map_clauses(tofrom) capture(ByRef) var_ptr_ptr(%[[VAL_22]] : !llvm.ptr) bounds(%[[VAL_12]], %[[VAL_21]], %[[VAL_27]]) -> !llvm.ptr {name = ""}
+// CHECK: %[[VAL_29:.*]] = omp.map.info var_ptr(%[[ARG0]] : !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<2 x array<3 x i64>>)>) map_clauses(to) capture(ByRef) members(%[[VAL_28]] : [0] : !llvm.ptr) -> !llvm.ptr {name = "csv_chem_list_a"}
+// CHECK: omp.target map_entries(%[[VAL_29]] -> %[[VAL_30:.*]], %[[VAL_28]] -> %[[VAL_31:.*]] : !llvm.ptr, !llvm.ptr) {
+// CHECK: omp.terminator
+// CHECK: }
+// CHECK: llvm.return
+// CHECK: }
+}
>From 7427d818c7f8589967cd08500ed3473dbac2964b Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Mon, 1 Sep 2025 13:50:33 -0500
Subject: [PATCH 2/4] [Flang][OpenMP][Offload] Add new runtime tests for issue
cases
---
.../fortran/dtype-char-array-map-2.f90 | 25 +++++++++++++++++
.../fortran/dtype-char-array-map.f90 | 27 +++++++++++++++++++
2 files changed, 52 insertions(+)
create mode 100644 offload/test/offloading/fortran/dtype-char-array-map-2.f90
create mode 100644 offload/test/offloading/fortran/dtype-char-array-map.f90
diff --git a/offload/test/offloading/fortran/dtype-char-array-map-2.f90 b/offload/test/offloading/fortran/dtype-char-array-map-2.f90
new file mode 100644
index 0000000000000..f17ea9e53853b
--- /dev/null
+++ b/offload/test/offloading/fortran/dtype-char-array-map-2.f90
@@ -0,0 +1,25 @@
+! Offloading test that verifies certain type of character string arrays
+! map to and from device without problem.
+! REQUIRES: flang, amdgpu
+
+! RUN: %libomptarget-compile-fortran-run-and-check-generic
+program main
+ implicit none
+ type char_t
+ CHARACTER(LEN=16), dimension(10,10) :: char_arr
+ end type char_t
+ type(char_t) :: dtype_char
+
+!$omp target enter data map(alloc:dtype_char%char_arr)
+
+!$omp target
+ dtype_char%char_arr(2,2) = 'c'
+!$omp end target
+
+!$omp target update from(dtype_char%char_arr)
+
+
+ print *, dtype_char%char_arr(2,2)
+end program
+
+!CHECK: c
diff --git a/offload/test/offloading/fortran/dtype-char-array-map.f90 b/offload/test/offloading/fortran/dtype-char-array-map.f90
new file mode 100644
index 0000000000000..6b72c9e951016
--- /dev/null
+++ b/offload/test/offloading/fortran/dtype-char-array-map.f90
@@ -0,0 +1,27 @@
+! Offloading test that verifies certain type of character string arrays
+! (in this case allocatable) map to and from device without problem.
+! REQUIRES: flang, amdgpu
+
+! RUN: %libomptarget-compile-fortran-run-and-check-generic
+program main
+ implicit none
+ type char_t
+ CHARACTER(LEN=16), dimension(:,:), allocatable :: char_arr
+ end type char_t
+ type(char_t) :: dtype_char
+
+ allocate(dtype_char%char_arr(10,10))
+
+!$omp target enter data map(alloc:dtype_char%char_arr)
+
+!$omp target
+ dtype_char%char_arr(2,2) = 'c'
+!$omp end target
+
+!$omp target update from(dtype_char%char_arr)
+
+
+ print *, dtype_char%char_arr(2,2)
+end program
+
+!CHECK: c
>From 74d59ba2b4ea1a38d278065011f0ab4cafd28eee Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Mon, 1 Sep 2025 20:22:20 -0500
Subject: [PATCH 3/4] [Flang][OpenMP] Extend / elaborate on comment describing
reasoning further
---
flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp | 62 ++++++++++++-------
1 file changed, 41 insertions(+), 21 deletions(-)
diff --git a/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp b/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
index 1c3fc04779377..7f6198cf37d26 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
@@ -95,29 +95,49 @@ struct MapInfoOpConversion
fir::unwrapPassByRefType(typeAttr.getValue()))) &&
!characterWithDynamicLen(
fir::unwrapPassByRefType(typeAttr.getValue()))) {
- // Characters with a LEN param are represented as char
- // arrays/strings, the initial lowering doesn't generate
- // bounds for these, however, we require them to map the
- // data appropriately in the later lowering stages. This
- // is to prevent the need for unecessary caveats
- // specific to Flang. We also strip the array from the
- // type so that all variations of strings are treated
- // identically and there's no caveats or specialisations
- // required in the later stages. As an example, Boxed
- // char strings will emit a single char array no matter
- // the number of dimensions caused by additional array
- // dimensions which needs specialised for, as it differs
- // from the non-box variation which will emit each array
- // wrapping the character array, e.g. given a type of
- // the same dimensions, if one is boxed, the types would
- // end up:
+ // Characters with a LEN param are represented as strings
+ // (array of characters), the lowering to LLVM dialect
+ // doesn't generate bounds for these (and this is not
+ // done at the initial lowering either) and there is
+ // minor inconsistencies in the variable types we
+ // create for the map without this step when converting
+ // to the LLVM dialect.
//
- // array<i8 x 16>
- // vs
- // array<10 x array< 10 x array<i8 x 16>>>
+ // For example, given the types:
//
- // This means we have to treat one specially in the
- // lowering. So we try to "canonicalize" it here.
+ // 1) CHARACTER(LEN=16), dimension(:,:), allocatable :: char_arr
+ // 2) CHARACTER(LEN=16), dimension(10,10) :: char_arr
+ //
+ // We get the FIR types (note for 1: we already peeled off the
+ // dynamic extents from the type at this stage, but the conversion
+ // to llvm dialect does that in any case, so the final result
+ // is the same):
+ //
+ // 1) !fir.char<1,16>
+ // 2) !fir.array<10x10x!fir.char<1,16>>
+ //
+ // Which are converted to the LLVM dialect types:
+ //
+ // 1) !llvm.array<16 x i8>
+ // 2) llvm.array<10 x array<10 x array<16 x i8>>
+ //
+ // And in both cases, we are missing the innermost bounds for
+ // the !fir.char<1,16> which is expanded into a 16 x i8 array
+ // in the conversion to LLVM dialect.
+ //
+ // The problem with this is that we would like to treat these
+ // cases identically and not have to create specialised
+ // lowerings for either of these in the lowering to LLVM-IR
+ // and treat them like any other array that passes through.
+ //
+ // To do so below, we generate an extra bound for the
+ // innermost array (the char type/string) using the LEN
+ // parameter of the character type. And we "canonicalize"
+ // the type, stripping it down to the base element type,
+ // which in this case is an i8. This effectively allows
+ // the lowering to treat this as a 1-D array with multiple
+ // bounds which it is capable of handling without any special
+ // casing.
// TODO: Handle dynamic LEN characters.
if (auto ct = mlir::dyn_cast_or_null<fir::CharacterType>(
fir::unwrapSequenceType(typeAttr.getValue()))) {
>From a6bcb51ee345e6d7c8a2922be5990bb79050c298 Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Tue, 9 Sep 2025 09:13:19 -0500
Subject: [PATCH 4/4] [Flang] Make comment less ambigious
---
flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp b/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
index 7f6198cf37d26..381b2a29c517a 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
@@ -145,8 +145,9 @@ struct MapInfoOpConversion
fir::unwrapSequenceType(typeAttr.getValue()));
if (auto type = mlir::dyn_cast<mlir::LLVM::LLVMArrayType>(newAttr))
newAttr = type.getElementType();
- // We do not generate for device, as MapBoundsOps are
- // unsupported, as they're currently unused.
+ // We do not generate MapBoundsOps for the device pass, as
+ // MapBoundsOps are not generated for the device pass, as
+ // they're unused in the device lowering.
auto offloadMod =
llvm::dyn_cast_or_null<mlir::omp::OffloadModuleInterface>(
*curOp->getParentOfType<mlir::ModuleOp>());
More information about the llvm-commits
mailing list