[OpenMP][MLIR] Descriptor explicit member map lowering changes #113556
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-offload @llvm/pr-subscribers-mlir-openmp Author: None (agozillon) ChangesThis is one of 3 PRs in a PR stack that aims to add support for explicit mapping of allocatable members in derived types. The primary changes in this PR are the OpenMPToLLVMIRTranslation.cpp changes, which are small and seek to alter the current member mapping to add an additional map insertion for pointers. Effectively, if the member is a pointer (currently indicated by having a varPtrPtr field) we add an additional map for the pointer and then alter the subsequent mapping of the member (the data) to utilise the member rather than the parents base pointer. This appears to be necessary in certain cases when mapping pointer data within record types to avoid segfaulting on device (due to incorrect data mapping). In general this record type mapping may be simplifiable in the future. There are also additions of tests which should help to showcase the affect of the changes above. Patch is 35.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/113556.diff 7 Files Affected:
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 626539cb7bde42..348c1b9c2b8bdf 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -895,7 +895,7 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> {
TypeAttr:$var_type,
Optional<OpenMP_PointerLikeType>:$var_ptr_ptr,
Variadic<OpenMP_PointerLikeType>:$members,
- OptionalAttr<AnyIntElementsAttr>:$members_index,
+ OptionalAttr<IndexListArrayAttr>:$members_index,
Variadic<OpenMP_MapBoundsType>:$bounds, /* rank-0 to rank-{n-1} */
OptionalAttr<UI64Attr>:$map_type,
OptionalAttr<VariableCaptureKindAttr>:$map_capture_type,
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index e1df647d6a3c71..8d31cda3a33ee9 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1395,16 +1395,15 @@ static void printMapClause(OpAsmPrinter &p, Operation *op,
}
static ParseResult parseMembersIndex(OpAsmParser &parser,
- DenseIntElementsAttr &membersIdx) {
- SmallVector<APInt> values;
- int64_t value;
- int64_t shape[2] = {0, 0};
- unsigned shapeTmp = 0;
+ ArrayAttr &membersIdx) {
+ SmallVector<Attribute> values, memberIdxs;
+
auto parseIndices = [&]() -> ParseResult {
+ int64_t value;
if (parser.parseInteger(value))
return failure();
- shapeTmp++;
- values.push_back(APInt(32, value, /*isSigned=*/true));
+ values.push_back(IntegerAttr::get(parser.getBuilder().getIntegerType(64),
+ APInt(64, value, /*isSigned=*/false)));
return success();
};
@@ -1418,52 +1417,29 @@ static ParseResult parseMembersIndex(OpAsmParser &parser,
if (failed(parser.parseRSquare()))
return failure();
- // Only set once, if any indices are not the same size
- // we error out in the next check as that's unsupported
- if (shape[1] == 0)
- shape[1] = shapeTmp;
-
- // Verify that the recently parsed list is equal to the
- // first one we parsed, they must be equal lengths to
- // keep the rectangular shape DenseIntElementsAttr
- // requires
- if (shapeTmp != shape[1])
- return failure();
-
- shapeTmp = 0;
- shape[0]++;
+ memberIdxs.push_back(ArrayAttr::get(parser.getContext(), values));
+ values.clear();
} while (succeeded(parser.parseOptionalComma()));
- if (!values.empty()) {
- ShapedType valueType =
- VectorType::get(shape, IntegerType::get(parser.getContext(), 32));
- membersIdx = DenseIntElementsAttr::get(valueType, values);
- }
+ if (!memberIdxs.empty())
+ membersIdx = ArrayAttr::get(parser.getContext(), memberIdxs);
return success();
}
static void printMembersIndex(OpAsmPrinter &p, MapInfoOp op,
- DenseIntElementsAttr membersIdx) {
- llvm::ArrayRef<int64_t> shape = membersIdx.getShapedType().getShape();
- assert(shape.size() <= 2);
-
+ ArrayAttr membersIdx) {
if (!membersIdx)
return;
- for (int i = 0; i < shape[0]; ++i) {
+ llvm::interleaveComma(membersIdx, p, [&p](Attribute v) {
p << "[";
- int rowOffset = i * shape[1];
- for (int j = 0; j < shape[1]; ++j) {
- p << membersIdx.getValues<int32_t>()[rowOffset + j];
- if ((j + 1) < shape[1])
- p << ",";
- }
+ auto memberIdx = cast<ArrayAttr>(v);
+ llvm::interleaveComma(memberIdx.getValue(), p, [&p](Attribute v2) {
+ p << cast<IntegerAttr>(v2).getInt();
+ });
p << "]";
-
- if ((i + 1) < shape[0])
- p << ", ";
- }
+ });
}
static void printCaptureType(OpAsmPrinter &p, Operation *op,
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 27cd38dc3c62d9..8b932d966b4da6 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -2386,46 +2386,36 @@ static int getMapDataMemberIdx(MapInfoData &mapData, omp::MapInfoOp memberOp) {
static omp::MapInfoOp getFirstOrLastMappedMemberPtr(omp::MapInfoOp mapInfo,
bool first) {
- DenseIntElementsAttr indexAttr = mapInfo.getMembersIndexAttr();
-
+ ArrayAttr indexAttr = mapInfo.getMembersIndexAttr();
// Only 1 member has been mapped, we can return it.
if (indexAttr.size() == 1)
- if (auto mapOp =
- dyn_cast<omp::MapInfoOp>(mapInfo.getMembers()[0].getDefiningOp()))
- return mapOp;
+ return cast<omp::MapInfoOp>(mapInfo.getMembers()[0].getDefiningOp());
- llvm::ArrayRef<int64_t> shape = indexAttr.getShapedType().getShape();
- llvm::SmallVector<size_t> indices(shape[0]);
+ llvm::SmallVector<size_t> indices(indexAttr.size());
std::iota(indices.begin(), indices.end(), 0);
llvm::sort(indices.begin(), indices.end(),
[&](const size_t a, const size_t b) {
- auto indexValues = indexAttr.getValues<int32_t>();
- for (int i = 0; i < shape[1]; ++i) {
- int aIndex = indexValues[a * shape[1] + i];
- int bIndex = indexValues[b * shape[1] + i];
+ auto memberIndicesA = cast<ArrayAttr>(indexAttr[a]);
+ auto memberIndicesB = cast<ArrayAttr>(indexAttr[b]);
+ for (const auto it : llvm::zip(memberIndicesA, memberIndicesB)) {
+ int64_t aIndex = cast<IntegerAttr>(std::get<0>(it)).getInt();
+ int64_t bIndex = cast<IntegerAttr>(std::get<1>(it)).getInt();
if (aIndex == bIndex)
continue;
- if (aIndex != -1 && bIndex == -1)
- return false;
-
- if (aIndex == -1 && bIndex != -1)
- return true;
-
- // A is earlier in the record type layout than B
if (aIndex < bIndex)
return first;
- if (bIndex < aIndex)
+ if (aIndex > bIndex)
return !first;
}
- // Iterated the entire list and couldn't make a decision, all
- // elements were likely the same. Return false, since the sort
- // comparator should return false for equal elements.
- return false;
+ // Iterated the up until the end of the smallest member and
+ // they were found to be equal up to that point, so select
+ // the member with the lowest index count, so the "parent"
+ return memberIndicesA.size() < memberIndicesB.size();
});
return llvm::cast<omp::MapInfoOp>(
@@ -2596,17 +2586,8 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
/*isSigned=*/false);
combinedInfo.Sizes.push_back(size);
- // TODO: This will need to be expanded to include the whole host of logic for
- // the map flags that Clang currently supports (e.g. it should take the map
- // flag of the parent map flag, remove the OMP_MAP_TARGET_PARAM and do some
- // further case specific flag modifications). For the moment, it handles what
- // we support as expected.
- llvm::omp::OpenMPOffloadMappingFlags mapFlag =
- llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
-
llvm::omp::OpenMPOffloadMappingFlags memberOfFlag =
ompBuilder.getMemberOfFlag(combinedInfo.BasePointers.size() - 1);
- ompBuilder.setCorrectMemberOfFlag(mapFlag, memberOfFlag);
// This creates the initial MEMBER_OF mapping that consists of
// the parent/top level container (same as above effectively, except
@@ -2615,6 +2596,12 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
// only relevant if the structure in its totality is being mapped,
// otherwise the above suffices.
if (!parentClause.getPartialMap()) {
+ // TODO: This will need to be expanded to include the whole host of logic
+ // for the map flags that Clang currently supports (e.g. it should do some
+ // further case specific flag modifications). For the moment, it handles
+ // what we support as expected.
+ llvm::omp::OpenMPOffloadMappingFlags mapFlag = mapData.Types[mapDataIndex];
+ ompBuilder.setCorrectMemberOfFlag(mapFlag, memberOfFlag);
combinedInfo.Types.emplace_back(mapFlag);
combinedInfo.DevicePointers.emplace_back(
llvm::OpenMPIRBuilder::DeviceInfoTy::None);
@@ -2665,6 +2652,31 @@ static void processMapMembersWithParent(
assert(memberDataIdx >= 0 && "could not find mapped member of structure");
+ // If we're currently mapping a pointer to a block of data, we must
+ // initially map the pointer, and then attatch/bind the data with a
+ // subsequent map to the pointer. This segment of code generates the
+ // pointer mapping, which can in certain cases be optimised out as Clang
+ // currently does in its lowering. However, for the moment we do not do so,
+ // in part as we currently have substantially less information on the data
+ // being mapped at this stage.
+ if (checkIfPointerMap(memberClause)) {
+ auto mapFlag = llvm::omp::OpenMPOffloadMappingFlags(
+ memberClause.getMapType().value());
+ mapFlag &= ~llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TARGET_PARAM;
+ mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF;
+ ompBuilder.setCorrectMemberOfFlag(mapFlag, memberOfFlag);
+ combinedInfo.Types.emplace_back(mapFlag);
+ combinedInfo.DevicePointers.emplace_back(
+ llvm::OpenMPIRBuilder::DeviceInfoTy::None);
+ combinedInfo.Names.emplace_back(
+ LLVM::createMappingInformation(memberClause.getLoc(), ompBuilder));
+ combinedInfo.BasePointers.emplace_back(
+ mapData.BasePointers[mapDataIndex]);
+ combinedInfo.Pointers.emplace_back(mapData.BasePointers[memberDataIdx]);
+ combinedInfo.Sizes.emplace_back(builder.getInt64(
+ moduleTranslation.getLLVMModule()->getDataLayout().getPointerSize()));
+ }
+
// Same MemberOfFlag to indicate its link with parent and other members
// of.
auto mapFlag =
@@ -2680,7 +2692,10 @@ static void processMapMembersWithParent(
mapData.DevicePointers[memberDataIdx]);
combinedInfo.Names.emplace_back(
LLVM::createMappingInformation(memberClause.getLoc(), ompBuilder));
- combinedInfo.BasePointers.emplace_back(mapData.BasePointers[mapDataIndex]);
+ uint64_t basePointerIndex =
+ checkIfPointerMap(memberClause) ? memberDataIdx : mapDataIndex;
+ combinedInfo.BasePointers.emplace_back(
+ mapData.BasePointers[basePointerIndex]);
combinedInfo.Pointers.emplace_back(mapData.Pointers[memberDataIdx]);
combinedInfo.Sizes.emplace_back(mapData.Sizes[memberDataIdx]);
}
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index 6f11b451fa00a3..63e53ee2e2612e 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -2633,8 +2633,8 @@ func.func @omp_map_with_members(%arg0: !llvm.ptr, %arg1: !llvm.ptr, %arg2: !llvm
// CHECK: %[[MAP4:.*]] = omp.map.info var_ptr(%[[ARG4]] : !llvm.ptr, f32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
%mapv5 = omp.map.info var_ptr(%arg4 : !llvm.ptr, f32) map_clauses(from) capture(ByRef) -> !llvm.ptr {name = ""}
- // CHECK: %[[MAP5:.*]] = omp.map.info var_ptr(%[[ARG5]] : !llvm.ptr, !llvm.struct<(i32, struct<(i32, f32)>)>) map_clauses(from) capture(ByRef) members(%[[MAP3]], %[[MAP4]] : [1,0], [1,1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
- %mapv6 = omp.map.info var_ptr(%arg5 : !llvm.ptr, !llvm.struct<(i32, struct<(i32, f32)>)>) map_clauses(from) capture(ByRef) members(%mapv4, %mapv5 : [1,0], [1,1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
+ // CHECK: %[[MAP5:.*]] = omp.map.info var_ptr(%[[ARG5]] : !llvm.ptr, !llvm.struct<(i32, struct<(i32, f32)>)>) map_clauses(from) capture(ByRef) members(%[[MAP3]], %[[MAP4]] : [1, 0], [1, 1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
+ %mapv6 = omp.map.info var_ptr(%arg5 : !llvm.ptr, !llvm.struct<(i32, struct<(i32, f32)>)>) map_clauses(from) capture(ByRef) members(%mapv4, %mapv5 : [1, 0], [1, 1] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "", partial_map = true}
// CHECK: omp.target_exit_data map_entries(%[[MAP3]], %[[MAP4]], %[[MAP5]] : !llvm.ptr, !llvm.ptr, !llvm.ptr)
omp.target_exit_data map_entries(%mapv4, %mapv5, %mapv6 : !llvm.ptr, !llvm.ptr, !llvm.ptr){}
diff --git a/mlir/test/Target/LLVMIR/omptarget-nested-ptr-record-type-mapping-host.mlir b/mlir/test/Target/LLVMIR/omptarget-nested-ptr-record-type-mapping-host.mlir
new file mode 100644
index 00000000000000..5d772a13fe578a
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-nested-ptr-record-type-mapping-host.mlir
@@ -0,0 +1,66 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// This test checks the offload sizes, map types and base pointers and pointers
+// provided to the OpenMP kernel argument structure are correct when lowering
+// to LLVM-IR from MLIR when performing explicit member mapping of a record type
+// that includes records with pointer members in various locations of the record
+// types hierarchy.
+
+module attributes {omp.is_target_device = false, omp.target_triples = ["amdgcn-amd-amdhsa"]} {
+ llvm.func @omp_nested_derived_type_alloca_map(%arg0: !llvm.ptr) {
+ %0 = llvm.mlir.constant(4 : index) : i64
+ %1 = llvm.mlir.constant(1 : index) : i64
+ %2 = llvm.mlir.constant(2 : index) : i64
+ %3 = llvm.mlir.constant(0 : index) : i64
+ %4 = llvm.mlir.constant(6 : index) : i64
+ %5 = omp.map.bounds lower_bound(%3 : i64) upper_bound(%0 : i64) extent(%0 : i64) stride(%1 : i64) start_idx(%3 : i64) {stride_in_bytes = true}
+ %6 = llvm.getelementptr %arg0[0, 6] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(f32, struct<(ptr, i64, i32, i8, i8, i8, i8)>, array<10 x i32>, f32, struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>, i32, struct<(f32, array<10 x i32>, struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>, i32)>)>
+ %7 = llvm.getelementptr %6[0, 2] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(f32, array<10 x i32>, struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>, i32)>
+ %8 = llvm.getelementptr %7[0, 0] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>
+ %9 = omp.map.info var_ptr(%7 : !llvm.ptr, i32) var_ptr_ptr(%8 : !llvm.ptr) map_clauses(tofrom) capture(ByRef) bounds(%5) -> !llvm.ptr {name = ""}
+ %10 = omp.map.info var_ptr(%7 : !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = "one_l%nest%array_k"}
+ %11 = omp.map.info var_ptr(%arg0 : !llvm.ptr, !llvm.struct<(f32, struct<(ptr, i64, i32, i8, i8, i8, i8)>, array<10 x i32>, f32, struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>, i32, struct<(f32, array<10 x i32>, struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>, i32)>)>) map_clauses(tofrom) capture(ByRef) members(%10, %9 : [6,2], [6,2,0] : !llvm.ptr, !llvm.ptr) -> !llvm.ptr {name = "one_l", partial_map = true}
+ omp.target map_entries(%10 -> %arg1, %9 -> %arg2, %11 -> %arg3 : !llvm.ptr, !llvm.ptr, !llvm.ptr) {
+ omp.terminator
+ }
+ llvm.return
+ }
+}
+
+// CHECK: @.offload_sizes{{.*}} = private unnamed_addr constant [4 x i64] [i64 0, i64 48, i64 8, i64 20]
+// CHECK: @.offload_maptypes{{.*}} = private unnamed_addr constant [4 x i64] [i64 32, i64 281474976710659, i64 281474976710659, i64 281474976710675]
+
+// CHECK: define void @omp_nested_derived_type_alloca_map(ptr %[[ARG:.*]]) {
+
+// CHECK: %[[NESTED_DTYPE_MEMBER_GEP:.*]] = getelementptr { float, { ptr, i64, i32, i8, i8, i8, i8 }, [10 x i32], float, { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, i32, { float, [10 x i32], { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, i32 } }, ptr %[[ARG]], i32 0, i32 6
+// CHECK: %[[NESTED_STRUCT_PTR_MEMBER_GEP:.*]] = getelementptr { float, [10 x i32], { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, i32 }, ptr %[[NESTED_DTYPE_MEMBER_GEP]], i32 0, i32 2
+// CHECK: %[[NESTED_STRUCT_PTR_MEMBER_BADDR_GEP:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr %[[NESTED_STRUCT_PTR_MEMBER_GEP]], i32 0, i32 0
+// CHECK: %[[NESTED_STRUCT_PTR_MEMBER_BADDR_LOAD:.*]] = load ptr, ptr %[[NESTED_STRUCT_PTR_MEMBER_BADDR_GEP]], align 8
+// CHECK: %[[ARR_OFFSET:.*]] = getelementptr inbounds i32, ptr %[[NESTED_STRUCT_PTR_MEMBER_BADDR_LOAD]], i64 0
+// CHECK: %[[DTYPE_SIZE_SEGMENT_CALC_1:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr %[[NESTED_STRUCT_PTR_MEMBER_GEP]], i64 1
+// CHECK: %[[DTYPE_SIZE_SEGMENT_CALC_2:.*]] = ptrtoint ptr %[[DTYPE_SIZE_SEGMENT_CALC_1]] to i64
+// CHECK: %[[DTYPE_SIZE_SEGMENT_CALC_3:.*]] = ptrtoint ptr %[[NESTED_STRUCT_PTR_MEMBER_GEP]] to i64
+// CHECK: %[[DTYPE_SIZE_SEGMENT_CALC_4:.*]] = sub i64 %[[DTYPE_SIZE_SEGMENT_CALC_2]], %[[DTYPE_SIZE_SEGMENT_CALC_3]]
+// CHECK: %[[DTYPE_SIZE_SEGMENT_CALC_5:.*]] = sdiv exact i64 %[[DTYPE_SIZE_SEGMENT_CALC_4]], ptrtoint (ptr getelementptr (i8, ptr null, i32 1) to i64)
+
+// CHECK: %[[BASE_PTRS:.*]] = getelementptr inbounds [4 x ptr], ptr %.offload_baseptrs, i32 0, i32 0
+// CHECK: store ptr %[[ARG]], ptr %[[BASE_PTRS]], align 8
+// CHECK: %[[OFFLOAD_PTRS:.*]] = getelementptr inbounds [4 x ptr], ptr %.offload_ptrs, i32 0, i32 0
+// CHECK: store ptr %[[NESTED_STRUCT_PTR_MEMBER_GEP]], ptr %[[OFFLOAD_PTRS]], align 8
+// CHECK: %[[OFFLOAD_SIZES:.*]] = getelementptr inbounds [4 x i64], ptr %.offload_sizes, i32 0, i32 0
+// CHECK: store i64 %[[DTYPE_SIZE_SEGMENT_CALC_5]], ptr %[[OFFLOAD_SIZES]], align 8
+
+// CHECK: %[[BASE_PTRS:.*]] = getelementptr inbounds [4 x ptr], ptr %.offload_baseptrs, i32 0, i32 1
+// CHECK: store ptr %[[ARG]], ptr %[[BASE_PTRS]], align 8
+// CHECK: %[[OFFLOAD_PTRS:.*]] = getelementptr inbounds [4 x ptr], ptr %.offload_ptrs, i32 0, i32 1
+// CHECK: store ptr %[[NESTED_STRUCT_PTR_MEMBER_GEP]], ptr %[[OFFLOAD_PTRS]], align 8
+
+// CHECK: %[[BASE_PTRS:.*]] = getelementptr inbounds [4 x ptr], ptr %.offload_baseptrs, i32 0, i32 2
+// CHECK: store ptr %[[ARG]], ptr %[[BASE_PTRS]], align 8
+// CHECK: %[[OFFLOAD_PTRS:.*]] = getelementptr inbounds [4 x ptr], ptr %.offload_ptrs, i32 0, i32 2
+// CHECK: store ptr %[[NESTED_STRUCT_PTR_MEMBER_BADDR_GEP]], ptr %[[OFFLOAD_PTRS]], align 8
+
+// CHECK: %[[BASE_PTRS:.*]] = getelementptr inbounds [4 x ptr], ptr %.offload_baseptrs, i32 0, i32 3
+// CHECK: store ptr %[[NESTED_STRUCT_PTR_MEMBER_BADDR_GEP]], ptr %[[BASE_PTRS]], align 8
+// CHECK: %[[OFFLOAD_PTRS:.*]] = getelementptr inbounds [4 x ptr], ptr %.offload_ptrs, i32 0, i32 3
+// CHECK: store ptr %[[ARR_OFFSET]], ptr %[[OFFLOAD_PTRS]], align 8
diff --git a/mlir/test/Target/LLVMIR/omptarget-nested-record-type-mapping-host.mlir b/mlir/test/Target/LLVMIR/omptarget-nested-record-type-mapping-host.mlir
index 8c1182c839a257..8837b42f70a447 100644
--- a/mlir/test/Target/LLVMIR/omptarget-nested-record-type-mapping-host.mlir
+++ b/mlir/test/Target/LLVMIR/omptarget-nested-record-type-mapping-host.mlir
@@ -21,7 +21,7 @@ llvm.func @_QQmain() {
%9 = llvm.getelementptr %4[0, 1] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(f32, array<10 x i32>, struct<(f32, i32)>, i32)>
%10 = omp.map.bounds lower_bound(%2 : i64) upper_bound(%1 : i64) extent(%0 : i64) stride(%2 : i64) start_idx(%2 : i64)
%11 = omp.map.info var_ptr(%9 : !llvm.ptr, !llvm.array<10 x i32>) map_clauses(tofrom) capture(ByRef) bounds(%10) -> !llvm.ptr
- %12 = omp.map.info var_ptr(%4 : !llvm.ptr, !llvm.struct<(f32, array<10 x i32>, struct<(f32, i32)>, i32)>) map_clauses(tofrom) capture(ByRef) members(%6, %8, %11 : [3, -1], [2, 1], [1, -1] : !llvm.ptr, !llvm.ptr, !llvm.ptr) -> !llvm.ptr {partial_map = true}
+ %12 = omp.map.info var_ptr(%4 : !llvm.ptr, !llvm.struct<(f32, array<10 x i32>, str...
[truncated]
|
skatrak
reviewed
Oct 30, 2024
I have a feeling it's just not setup correctly to layer on top of it's dependent PRs, hence the errors. As in conjunction it all builds fine and runs on my machine, I'll double check though, perhaps something got lost in translation :-) Would love an approval if you're happy with the PRs current state! And would love an approval or further review comments from @ergawy @TIFitis if at all possible, would be wonderful to be able to land this in the next week or two as it seems to be approaching closure :-) |
agozillon
force-pushed
the
users/agozillo/alloca-member-map-1
branch
from
4c33328 to
5fca59a
Compare
|
Ah yes, this one won't build, as MapInfoFinalization.cpp is a Flang change, as is the Utils.cpp/hpp and other complaints. This patch requires the changeset from 113557, so it's 113557 we should be looking at to pass, which it currently doesn't, so I'll have a look there. But don't expect this one to pass as it doesn't contain the changeset required to do so, it'd require both the Flang frontend changes and the changeset in this PR which is only the MLIR project level changes. |
agozillon
force-pushed
the
users/agozillo/alloca-member-map-2
branch
2 times, most recently
from
b27db91 to
70265b8
Compare
skatrak
approved these changes
Oct 31, 2024
agozillon
force-pushed
the
users/agozillo/alloca-member-map-1
branch
from
5fca59a to
c97ce21
Compare
agozillon
force-pushed
the
users/agozillo/alloca-member-map-2
branch
from
70265b8 to
d0373c8
Compare
agozillon
changed the base branch from
users/agozillo/alloca-member-map-1
to
main
agozillon
changed the base branch from
main
to
users/agozillo/alloca-member-map-1
agozillon
force-pushed
the
users/agozillo/alloca-member-map-2
branch
from
d0373c8 to
8dcb02f
Compare
agozillon
force-pushed
the
users/agozillo/alloca-member-map-1
branch
from
c97ce21 to
7e123d0
Compare
agozillon
force-pushed
the
users/agozillo/alloca-member-map-2
branch
from
8dcb02f to
7a6a02b
Compare
This is one of 3 PRs in a PR stack that aims to add support for explicit mapping of allocatable members in derived types. The primary changes in this PR are the OpenMPToLLVMIRTranslation.cpp changes, which are small and seek to alter the current member mapping to add an additional map insertion for pointers. Effectively, if the member is a pointer (currently indicated by having a varPtrPtr field) we add an additional map for the pointer and then alter the subsequent mapping of the member (the data) to utilise the member rather than the parents base pointer. This appears to be necessary in certain cases when mapping pointer data within record types to avoid segfaulting on device (due to incorrect data mapping). In general this record type mapping may be simplifiable in the future. There are also additions of tests which should help to showcase the affect of the changes above.
agozillon
force-pushed
the
users/agozillo/alloca-member-map-2
branch
from
7a6a02b to
c366a1a
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/9615 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/14849 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/12925 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/177/builds/8417 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/116/builds/6448 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/117/builds/3786 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/35/builds/3708 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/10694 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/53/builds/7985 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/50/builds/6464 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/130/builds/6265 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/172/builds/6112 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/80/builds/6380 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/29/builds/6594 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/20/builds/5633 Here is the relevant piece of the build log for the reference |
|
All expected explosions unfortunately as it depends on the other PR, this is the PR to worry about failures on: #113557 as it's the one layered on top that should pass. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/5367 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is one of 3 PRs in a PR stack that aims to add support for explicit mapping of allocatable members in derived types.
The primary changes in this PR are the OpenMPToLLVMIRTranslation.cpp changes, which are small and seek to alter the current member mapping to add an additional map insertion for pointers. Effectively, if the member is a pointer (currently indicated by having a varPtrPtr field) we add an additional map for the pointer and then alter the subsequent mapping of the member (the data) to utilise the member rather than the parents base pointer. This appears to be necessary in certain cases when mapping pointer data within record types to avoid segfaulting on device (due to incorrect data mapping). In general this record type mapping may be simplifiable in the future.
There are also additions of tests which should help to showcase the affect of the changes above.