Skip to content

Commit 765254b

Browse files
skatrakskatrak
committed
[MLIR][OpenMP] Improve omp.map.info verification (llvm#132066)
This patch makes the `map_type` and `map_capture_type` arguments of the `omp.map.info` operation required, which was already an invariant being verified by its users via `verifyMapClause()`. This makes it clearer, as getters no longer return misleading `std::optional` values. Checks for the `mapper_id` argument are moved to a verifier for the operation, rather than being checked by users. Functionally NFC, but not marked as such due to a reordering of arguments in the assembly format of `omp.map.info`.
1 parent 42ce4c6 commit 765254b

27 files changed

+134
-134
lines changed

‎flang/docs/OpenMP-descriptor-management.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ omp.target map_entries(%12 -> %arg1, %13 -> %arg2 : !fir.ref<!fir.box<!fir.ptr<!
6868

6969
...
7070
%12 = fir.box_offset %1#1 base_addr : (!fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>
71-
%13 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.array<?xi32>) var_ptr_ptr(%12 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>) map_clauses(tofrom) capture(ByRef) bounds(%11) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>> {name = ""}
71+
%13 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.array<?xi32>) map_clauses(tofrom) capture(ByRef) var_ptr_ptr(%12 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>) bounds(%11) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>> {name = ""}
7272
%14 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.box<!fir.ptr<!fir.array<?xi32>>>) map_clauses(tofrom) capture(ByRef) members(%13 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>) -> !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>> {name = "arg_alloc"}
7373
...
7474
omp.target map_entries(%13 -> %arg1, %14 -> %arg2, %15 -> %arg3 : !fir.llvm_ptr<!fir.ref<!fir.array<?xi32>>>, !fir.ref<!fir.box<!fir.ptr<!fir.array<?xi32>>>>, !fir.ref<i32>) {

‎flang/lib/Lower/OpenMP/Utils.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,10 @@ createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
148148
varType = mlir::TypeAttr::get(seqType.getEleTy());
149149

150150
mlir::omp::MapInfoOp op = builder.create<mlir::omp::MapInfoOp>(
151-
loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
151+
loc, retTy, baseAddr, varType,
152152
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
153-
mapperId,
154153
builder.getAttr<mlir::omp::VariableCaptureKindAttr>(mapCaptureType),
154+
varPtrPtr, members, membersIndex, bounds, mapperId,
155155
builder.getStringAttr(name), builder.getBoolAttr(partialMap));
156156
return op;
157157
}
@@ -556,7 +556,7 @@ void insertChildMapInfoIntoParent(
556556
// it allows this to work with enter and exit without causing MLIR
557557
// verification issues. The more appropriate thing may be to take
558558
// the "main" map type clause from the directive being used.
559-
uint64_t mapType = indices.second.memberMap[0].getMapType().value_or(0);
559+
uint64_t mapType = indices.second.memberMap[0].getMapType();
560560

561561
llvm::SmallVector<mlir::Value> members;
562562
members.reserve(indices.second.memberMap.size());

‎flang/lib/Optimizer/OpenMP/DoConcurrentConversion.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ mlir::omp::MapInfoOp createMapInfoOp(
6969
varType = mlir::TypeAttr::get(seqType.getEleTy());
7070

7171
mlir::omp::MapInfoOp op = builder.create<mlir::omp::MapInfoOp>(
72-
loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
72+
loc, retTy, baseAddr, varType,
7373
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
74-
mapperId,
7574
builder.getAttr<mlir::omp::VariableCaptureKindAttr>(mapCaptureType),
75+
varPtrPtr, members, membersIndex, bounds, mapperId,
7676
builder.getStringAttr(name), builder.getBoolAttr(partialMap));
7777

7878
return op;

‎flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -180,13 +180,13 @@ class MapInfoFinalizationPass
180180
// Member of the descriptor pointing at the allocated data
181181
return builder.create<mlir::omp::MapInfoOp>(
182182
loc, baseAddrAddr.getType(), descriptor,
183-
mlir::TypeAttr::get(underlyingVarType), baseAddrAddr,
184-
/*members=*/mlir::SmallVector<mlir::Value>{},
185-
/*membersIndex=*/mlir::ArrayAttr{}, bounds,
183+
mlir::TypeAttr::get(underlyingVarType),
186184
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
187-
/*mapperId*/ mlir::FlatSymbolRefAttr(),
188185
builder.getAttr<mlir::omp::VariableCaptureKindAttr>(
189186
mlir::omp::VariableCaptureKind::ByRef),
187+
baseAddrAddr, /*members=*/mlir::SmallVector<mlir::Value>{},
188+
/*membersIndex=*/mlir::ArrayAttr{}, bounds,
189+
/*mapperId*/ mlir::FlatSymbolRefAttr(),
190190
/*name=*/builder.getStringAttr(""),
191191
/*partial_map=*/builder.getBoolAttr(false));
192192
}
@@ -322,8 +322,8 @@ class MapInfoFinalizationPass
322322
assert(mapMemberUsers.size() == 1 &&
323323
"OMPMapInfoFinalization currently only supports single users of a "
324324
"MapInfoOp");
325-
auto baseAddr = genBaseAddrMap(descriptor, op.getBounds(),
326-
op.getMapType().value_or(0), builder);
325+
auto baseAddr =
326+
genBaseAddrMap(descriptor, op.getBounds(), op.getMapType(), builder);
327327
ParentAndPlacement mapUser = mapMemberUsers[0];
328328
adjustMemberIndices(memberIndices, mapUser.index);
329329
llvm::SmallVector<mlir::Value> newMemberOps;
@@ -336,8 +336,8 @@ class MapInfoFinalizationPass
336336
mapUser.parent.setMembersIndexAttr(
337337
builder.create2DI64ArrayAttr(memberIndices));
338338
} else if (!IsHasDeviceAddr) {
339-
auto baseAddr = genBaseAddrMap(descriptor, op.getBounds(),
340-
op.getMapType().value_or(0), builder);
339+
auto baseAddr =
340+
genBaseAddrMap(descriptor, op.getBounds(), op.getMapType(), builder);
341341
newMembers.push_back(baseAddr);
342342
if (!op.getMembers().empty()) {
343343
for (auto &indices : memberIndices)
@@ -355,13 +355,12 @@ class MapInfoFinalizationPass
355355
builder.create<mlir::omp::MapInfoOp>(
356356
op->getLoc(), op.getResult().getType(), descriptor,
357357
mlir::TypeAttr::get(fir::unwrapRefType(descriptor.getType())),
358-
/*varPtrPtr=*/mlir::Value{}, newMembers, newMembersAttr,
359-
/*bounds=*/mlir::SmallVector<mlir::Value>{},
360358
builder.getIntegerAttr(
361359
builder.getIntegerType(64, false),
362-
getDescriptorMapType(op.getMapType().value_or(0), target)),
363-
/*mapperId*/ mlir::FlatSymbolRefAttr(), op.getMapCaptureTypeAttr(),
364-
op.getNameAttr(),
360+
getDescriptorMapType(op.getMapType(), target)),
361+
op.getMapCaptureTypeAttr(), /*varPtrPtr=*/mlir::Value{}, newMembers,
362+
newMembersAttr, /*bounds=*/mlir::SmallVector<mlir::Value>{},
363+
/*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(),
365364
/*partial_map=*/builder.getBoolAttr(false));
366365
op.replaceAllUsesWith(newDescParentMapOp.getResult());
367366
op->erase();
@@ -661,13 +660,12 @@ class MapInfoFinalizationPass
661660
// fieldCoord.getResult(),
662661
// mlir::TypeAttr::get(
663662
// fir::unwrapRefType(fieldCoord.getResult().getType())),
664-
// /*varPtrPtr=*/mlir::Value{},
665-
// /*members=*/mlir::ValueRange{},
666-
// /*members_index=*/mlir::ArrayAttr{},
667-
// /*bounds=*/bounds, op.getMapTypeAttr(),
668-
// /*mapperId*/ mlir::FlatSymbolRefAttr(),
663+
// op.getMapTypeAttr(),
669664
// builder.getAttr<mlir::omp::VariableCaptureKindAttr>(
670665
// mlir::omp::VariableCaptureKind::ByRef),
666+
// /*varPtrPtr=*/mlir::Value{}, /*members=*/mlir::ValueRange{},
667+
// /*members_index=*/mlir::ArrayAttr{}, bounds,
668+
// /*mapperId*/ mlir::FlatSymbolRefAttr(),
671669
// builder.getStringAttr(op.getNameAttr().strref() + "." +
672670
// field + ".implicit_map"),
673671
// /*partial_map=*/builder.getBoolAttr(false));

‎flang/lib/Optimizer/OpenMP/MapsForPrivatizedSymbols.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,16 @@ class MapsForPrivatizedSymbolsPass
8585
loc, varPtr.getType(), varPtr,
8686
TypeAttr::get(llvm::cast<omp::PointerLikeType>(varPtr.getType())
8787
.getElementType()),
88-
/*varPtrPtr=*/Value{},
89-
/*members=*/SmallVector<Value>{},
90-
/*member_index=*/mlir::ArrayAttr{},
91-
/*bounds=*/ValueRange{},
9288
builder.getIntegerAttr(builder.getIntegerType(64, /*isSigned=*/false),
9389
mapTypeTo),
94-
/*mapperId*/ mlir::FlatSymbolRefAttr(),
9590
builder.getAttr<omp::VariableCaptureKindAttr>(
9691
omp::VariableCaptureKind::ByRef),
97-
StringAttr(), builder.getBoolAttr(false));
92+
/*varPtrPtr=*/Value{},
93+
/*members=*/SmallVector<Value>{},
94+
/*member_index=*/mlir::ArrayAttr{},
95+
/*bounds=*/ValueRange{},
96+
/*mapperId=*/mlir::FlatSymbolRefAttr(), /*name=*/StringAttr(),
97+
builder.getBoolAttr(false));
9898
}
9999
void addMapInfoOp(omp::TargetOp targetOp, omp::MapInfoOp mapInfoOp) {
100100
auto argIface = llvm::cast<omp::BlockArgOpenMPOpInterface>(*targetOp);

‎flang/test/Analysis/AliasAnalysis/alias-analysis-omp-target-1.fir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func.func @_QPTestAllocatableArray() {
3030
%8 = arith.subi %7#1, %c1 : index
3131
%9 = omp.map.bounds lower_bound(%c0_1 : index) upper_bound(%8 : index) extent(%7#1 : index) stride(%7#2 : index) start_idx(%6#0 : index) {stride_in_bytes = true}
3232
%10 = fir.box_offset %1#1 base_addr : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>
33-
%11 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.array<?xf64>) var_ptr_ptr(%10 : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) map_clauses(implicit, tofrom) capture(ByRef) bounds(%9) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>> {name = ""}
33+
%11 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.array<?xf64>) map_clauses(implicit, tofrom) capture(ByRef) var_ptr_ptr(%10 : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) bounds(%9) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>> {name = ""}
3434
%12 = omp.map.info var_ptr(%1#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.box<!fir.heap<!fir.array<?xf64>>>) map_clauses(implicit, tofrom) capture(ByRef) members(%11 : [0] : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>> {name = "a"}
3535
%13 = fir.load %3#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>
3636
%c1_2 = arith.constant 1 : index
@@ -43,7 +43,7 @@ func.func @_QPTestAllocatableArray() {
4343
%17 = arith.subi %16#1, %c1_2 : index
4444
%18 = omp.map.bounds lower_bound(%c0_5 : index) upper_bound(%17 : index) extent(%16#1 : index) stride(%16#2 : index) start_idx(%15#0 : index) {stride_in_bytes = true}
4545
%19 = fir.box_offset %3#1 base_addr : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>
46-
%20 = omp.map.info var_ptr(%3#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.array<?xf64>) var_ptr_ptr(%19 : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) map_clauses(implicit, tofrom) capture(ByRef) bounds(%18) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>> {name = ""}
46+
%20 = omp.map.info var_ptr(%3#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.array<?xf64>) map_clauses(implicit, tofrom) capture(ByRef) var_ptr_ptr(%19 : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) bounds(%18) -> !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>> {name = ""}
4747
%21 = omp.map.info var_ptr(%3#1 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.box<!fir.heap<!fir.array<?xf64>>>) map_clauses(implicit, tofrom) capture(ByRef) members(%20 : [0] : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>> {name = "b"}
4848
omp.target map_entries(%11 -> %arg0, %12 -> %arg1, %20 -> %arg2, %21 -> %arg3 : !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.llvm_ptr<!fir.ref<!fir.array<?xf64>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) {
4949
%22:2 = hlfir.declare %arg1 {fortran_attrs = #fir.var_attrs<allocatable>, uniq_name = "_QFEa"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>) -> (!fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xf64>>>>)

0 commit comments

Comments
 (0)