Skip to content

Commit debdcff

Browse files
agozillonagozillon
committed
[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.
1 parent 7c53c61 commit debdcff

File tree

2 files changed

+163
-2
lines changed

2 files changed

+163
-2
lines changed

‎flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp‎

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,21 @@ struct MapInfoOpConversion
6060
: public OpenMPFIROpConversion<mlir::omp::MapInfoOp> {
6161
using OpenMPFIROpConversion::OpenMPFIROpConversion;
6262

63+
mlir::omp::MapBoundsOp
64+
createBoundsForCharString(mlir::ConversionPatternRewriter &rewriter,
65+
unsigned int len, mlir::Location loc) const {
66+
mlir::Type i64Ty = rewriter.getIntegerType(64);
67+
auto lBound = mlir::LLVM::ConstantOp::create(rewriter, loc, i64Ty, 0);
68+
auto uBoundAndExt =
69+
mlir::LLVM::ConstantOp::create(rewriter, loc, i64Ty, len - 1);
70+
auto stride = mlir::LLVM::ConstantOp::create(rewriter, loc, i64Ty, 1);
71+
auto baseLb = mlir::LLVM::ConstantOp::create(rewriter, loc, i64Ty, 1);
72+
auto mapBoundType = rewriter.getType<mlir::omp::MapBoundsType>();
73+
return mlir::omp::MapBoundsOp::create(rewriter, loc, mapBoundType, lBound,
74+
uBoundAndExt, uBoundAndExt, stride,
75+
/*strideInBytes*/ false, baseLb);
76+
}
77+
6378
llvm::LogicalResult
6479
matchAndRewrite(mlir::omp::MapInfoOp curOp, OpAdaptor adaptor,
6580
mlir::ConversionPatternRewriter &rewriter) const override {
@@ -69,13 +84,58 @@ struct MapInfoOpConversion
6984
return mlir::failure();
7085

7186
llvm::SmallVector<mlir::NamedAttribute> newAttrs;
72-
mlir::omp::MapInfoOp newOp;
87+
mlir::omp::MapBoundsOp mapBoundsOp;
7388
for (mlir::NamedAttribute attr : curOp->getAttrs()) {
7489
if (auto typeAttr = mlir::dyn_cast<mlir::TypeAttr>(attr.getValue())) {
7590
mlir::Type newAttr;
7691
if (fir::isTypeWithDescriptor(typeAttr.getValue())) {
7792
newAttr = lowerTy().convertBoxTypeAsStruct(
7893
mlir::cast<fir::BaseBoxType>(typeAttr.getValue()));
94+
} else if (fir::isa_char_string(fir::unwrapSequenceType(
95+
fir::unwrapPassByRefType(typeAttr.getValue()))) &&
96+
!characterWithDynamicLen(
97+
fir::unwrapPassByRefType(typeAttr.getValue()))) {
98+
// Characters with a LEN param are represented as char
99+
// arrays/strings, the initial lowering doesn't generate
100+
// bounds for these, however, we require them to map the
101+
// data appropriately in the later lowering stages. This
102+
// is to prevent the need for unecessary caveats
103+
// specific to Flang. We also strip the array from the
104+
// type so that all variations of strings are treated
105+
// identically and there's no caveats or specialisations
106+
// required in the later stages. As an example, Boxed
107+
// char strings will emit a single char array no matter
108+
// the number of dimensions caused by additional array
109+
// dimensions which needs specialised for, as it differs
110+
// from the non-box variation which will emit each array
111+
// wrapping the character array, e.g. given a type of
112+
// the same dimensions, if one is boxed, the types would
113+
// end up:
114+
//
115+
// array<i8 x 16>
116+
// vs
117+
// array<10 x array< 10 x array<i8 x 16>>>
118+
//
119+
// This means we have to treat one specially in the
120+
// lowering. So we try to "canonicalize" it here.
121+
// TODO: Handle dynamic LEN characters.
122+
if (auto ct = mlir::dyn_cast_or_null<fir::CharacterType>(
123+
fir::unwrapSequenceType(typeAttr.getValue()))) {
124+
newAttr = converter->convertType(
125+
fir::unwrapSequenceType(typeAttr.getValue()));
126+
if (auto type = mlir::dyn_cast<mlir::LLVM::LLVMArrayType>(newAttr))
127+
newAttr = type.getElementType();
128+
// We do not generate for device, as MapBoundsOps are
129+
// unsupported, as they're currently unused.
130+
auto offloadMod =
131+
llvm::dyn_cast_or_null<mlir::omp::OffloadModuleInterface>(
132+
*curOp->getParentOfType<mlir::ModuleOp>());
133+
if (!offloadMod.getIsTargetDevice())
134+
mapBoundsOp = createBoundsForCharString(rewriter, ct.getLen(),
135+
curOp.getLoc());
136+
} else {
137+
newAttr = converter->convertType(typeAttr.getValue());
138+
}
79139
} else {
80140
newAttr = converter->convertType(typeAttr.getValue());
81141
}
@@ -85,8 +145,13 @@ struct MapInfoOpConversion
85145
}
86146
}
87147

88-
rewriter.replaceOpWithNewOp<mlir::omp::MapInfoOp>(
148+
auto newOp = rewriter.replaceOpWithNewOp<mlir::omp::MapInfoOp>(
89149
curOp, resTypes, adaptor.getOperands(), newAttrs);
150+
if (mapBoundsOp) {
151+
rewriter.startOpModification(newOp);
152+
newOp.getBoundsMutable().append(mlir::ValueRange{mapBoundsOp});
153+
rewriter.finalizeOpModification(newOp);
154+
}
90155

91156
return mlir::success();
92157
}

‎flang/test/Fir/OpenMP/bounds-generation-for-char-arrays.f90‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// RUN: fir-opt --cfg-conversion --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s
2+
3+
module attributes {omp.is_target_device = false} {
4+
func.func @_QPchar_array(%arg0 : !fir.ref<!fir.array<10x10x!fir.char<1,16>>>) {
5+
%c9 = arith.constant 9 : index
6+
%c0 = arith.constant 0 : index
7+
%c1 = arith.constant 1 : index
8+
%c10 = arith.constant 10 : index
9+
%0 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%c9 : index) extent(%c10 : index) stride(%c1 : index) start_idx(%c1 : index)
10+
%1 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%c9 : index) extent(%c10 : index) stride(%c1 : index) start_idx(%c1 : index)
11+
%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 = ""}
12+
omp.target map_entries(%2 -> %arg1 : !fir.ref<!fir.array<10x10x!fir.char<1,16>>>) {
13+
omp.terminator
14+
}
15+
return
16+
}
17+
18+
// CHECK-LABEL: llvm.func @_QPchar_array(
19+
// CHECK-SAME: %[[ARG0:.*]]: !llvm.ptr) {
20+
// CHECK: %[[VAL_0:.*]] = llvm.mlir.constant(9 : index) : i64
21+
// CHECK: %[[VAL_1:.*]] = llvm.mlir.constant(0 : index) : i64
22+
// CHECK: %[[VAL_2:.*]] = llvm.mlir.constant(1 : index) : i64
23+
// CHECK: %[[VAL_3:.*]] = llvm.mlir.constant(10 : index) : i64
24+
// 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)
25+
// 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)
26+
// CHECK: %[[VAL_6:.*]] = llvm.mlir.constant(0 : i64) : i64
27+
// CHECK: %[[VAL_7:.*]] = llvm.mlir.constant(15 : i64) : i64
28+
// CHECK: %[[VAL_8:.*]] = llvm.mlir.constant(1 : i64) : i64
29+
// CHECK: %[[VAL_9:.*]] = llvm.mlir.constant(1 : i64) : i64
30+
// 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)
31+
// 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 = ""}
32+
// CHECK: omp.target map_entries(%[[VAL_11]] -> %[[VAL_12:.*]] : !llvm.ptr) {
33+
// CHECK: omp.terminator
34+
// CHECK: }
35+
// CHECK: llvm.return
36+
// CHECK: }
37+
38+
func.func @_QPallocatable_char_array(%arg0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>>) {
39+
%c1 = arith.constant 1 : index
40+
%c0 = arith.constant 0 : index
41+
%0 = fir.load %arg0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>>
42+
%1:3 = fir.box_dims %0, %c0 : (!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>, index) -> (index, index, index)
43+
%2 = arith.subi %1#1, %c1 : index
44+
%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}
45+
%4 = arith.muli %1#2, %1#1 : index
46+
%5:3 = fir.box_dims %0, %c1 : (!fir.box<!fir.heap<!fir.array<?x?x!fir.char<1,16>>>>, index) -> (index, index, index)
47+
%6 = arith.subi %5#1, %c1 : index
48+
%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}
49+
%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>>>>
50+
%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 = ""}
51+
%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"}
52+
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>>>>) {
53+
omp.terminator
54+
}
55+
return
56+
}
57+
58+
// CHECK-LABEL: llvm.func @_QPallocatable_char_array(
59+
// CHECK-SAME: %[[ARG0:.*]]: !llvm.ptr) {
60+
// CHECK: %[[VAL_0:.*]] = llvm.mlir.constant(1 : i32) : i32
61+
// 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
62+
// CHECK: %[[VAL_2:.*]] = llvm.mlir.constant(1 : index) : i64
63+
// CHECK: %[[VAL_3:.*]] = llvm.mlir.constant(0 : index) : i64
64+
// CHECK: %[[VAL_4:.*]] = llvm.mlir.constant(72 : i32) : i32
65+
// CHECK: "llvm.intr.memcpy"(%[[VAL_1]], %[[ARG0]], %[[VAL_4]]) <{isVolatile = false}> : (!llvm.ptr, !llvm.ptr, i32) -> ()
66+
// 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>>)>
67+
// CHECK: %[[VAL_6:.*]] = llvm.load %[[VAL_5]] : !llvm.ptr -> i64
68+
// 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>>)>
69+
// CHECK: %[[VAL_8:.*]] = llvm.load %[[VAL_7]] : !llvm.ptr -> i64
70+
// 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>>)>
71+
// CHECK: %[[VAL_10:.*]] = llvm.load %[[VAL_9]] : !llvm.ptr -> i64
72+
// CHECK: %[[VAL_11:.*]] = llvm.sub %[[VAL_8]], %[[VAL_2]] : i64
73+
// 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}
74+
// CHECK: %[[VAL_13:.*]] = llvm.mul %[[VAL_10]], %[[VAL_8]] : i64
75+
// 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>>)>
76+
// CHECK: %[[VAL_15:.*]] = llvm.load %[[VAL_14]] : !llvm.ptr -> i64
77+
// 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>>)>
78+
// CHECK: %[[VAL_17:.*]] = llvm.load %[[VAL_16]] : !llvm.ptr -> i64
79+
// 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>>)>
80+
// CHECK: %[[VAL_19:.*]] = llvm.load %[[VAL_18]] : !llvm.ptr -> i64
81+
// CHECK: %[[VAL_20:.*]] = llvm.sub %[[VAL_17]], %[[VAL_2]] : i64
82+
// 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}
83+
// 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>>)>
84+
// CHECK: %[[VAL_23:.*]] = llvm.mlir.constant(0 : i64) : i64
85+
// CHECK: %[[VAL_24:.*]] = llvm.mlir.constant(15 : i64) : i64
86+
// CHECK: %[[VAL_25:.*]] = llvm.mlir.constant(1 : i64) : i64
87+
// CHECK: %[[VAL_26:.*]] = llvm.mlir.constant(1 : i64) : i64
88+
// 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)
89+
// 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 = ""}
90+
// 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"}
91+
// CHECK: omp.target map_entries(%[[VAL_29]] -> %[[VAL_30:.*]], %[[VAL_28]] -> %[[VAL_31:.*]] : !llvm.ptr, !llvm.ptr) {
92+
// CHECK: omp.terminator
93+
// CHECK: }
94+
// CHECK: llvm.return
95+
// CHECK: }
96+
}

0 commit comments

Comments
 (0)