From e82399dac2f1f09319243dc39d9e05c7b9b8c6d2 Mon Sep 17 00:00:00 2001 From: Kareem Ergawy Date: Fri, 9 Jan 2026 15:15:10 +0100 Subject: [PATCH] [flang][OpenMP] Prevent `omp.map.info` ops with user-defined mappers from being marked as parial maps (#175133) The following test was triggering a runtime crash **on the host before launching the kernel**: ```fortran program test_omp_target_map_bug_v5 implicit none type nested_type real, allocatable :: alloc_field(:) end type nested_type type nesting_type integer :: int_field type(nested_type) :: derived_field end type nesting_type type(nesting_type) :: config allocate(config%derived_field%alloc_field(1)) !$OMP TARGET ENTER DATA MAP(TO:config, config%derived_field%alloc_field) !$OMP TARGET config%derived_field%alloc_field(1) = 1.0 !$OMP END TARGET deallocate(config%derived_field%alloc_field) end program test_omp_target_map_bug_v5 ``` In particular, the runtime was producing a segmentation fault when the test is compiled with any optimization level > 0; if you compile with -O0 the sample ran fine. After debugging the runtime, it turned out the crash was happening at the point where the runtime calls the default mapper emitted by the compiler for `nesting_type; in particular at this point in the runtime: https://github.com/llvm/llvm-project/blob/c62cd2877cc25a0d708ad22a70c2a57590449c4d/offload/libomptarget/omptarget.cpp#L307. Bisecting the optimization pipeline using `-mllvm -opt-bisect-limit=N`, the first pass that triggered the issue on `O1` was the `instcombine` pass. Debugging this further, the issue narrows down to canonicalizing `getelementptr` instructions from using struct types (in this case the `nesting_type` in the sample above) to using addressing bytes (`i8`). In particular, in `O0`, you would see something like this: ```llvm define internal void @.omp_mapper._QQFnesting_type_omp_default_mapper(ptr noundef %0, ptr noundef %1, ptr noundef %2, i64 noundef %3, i64 noundef %4, ptr noundef %5) #6 { entry: %6 = udiv exact i64 %3, 56 %7 = getelementptr %_QFTnesting_type, ptr %2, i64 %6 .... } ``` ```llvm define internal void @.omp_mapper._QQFnesting_type_omp_default_mapper(ptr noundef %0, ptr noundef %1, ptr noundef %2, i64 noundef %3, i64 noundef %4, ptr noundef %5) #6 { entry: %6 = getelementptr i8, ptr %2, i64 %3 .... } ``` The `udiv exact` instruction emitted by the OMP IR Builder (see: https://github.com/llvm/llvm-project/blob/c62cd2877cc25a0d708ad22a70c2a57590449c4d/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp#L9154) allows `instcombine` to assume that `%3` is divisible by the struct size (here `56`) and, therefore, replaces the result of the division with direct GEP on `i8` rather than the struct type. However, the runtime was calling `@.omp_mapper._QQFnesting_type_omp_default_mapper` not with `56` (the proper struct size) but with `48`! Debugging this further, I found that the size of `omp.map.info` operation to which the default mapper is attached computes the value of `48` because we set the map to partial (see: https://github.com/llvm/llvm-project/blob/c62cd2877cc25a0d708ad22a70c2a57590449c4d/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp#L1146 and https://github.com/llvm/llvm-project/blob/c62cd2877cc25a0d708ad22a70c2a57590449c4d/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp#L4501-L4512). However, I think this is incorrect since the emitted mapper (and user-defined mappers in general) are defined on the whole struct type and should never be marked as partial. Hence, the fix in this PR. --- .../Optimizer/OpenMP/MapInfoFinalization.cpp | 3 +- ...p-map-info-finalization-implicit-field.fir | 26 +++++++++++++- .../default-mapper-nested-derived-type.f90 | 34 +++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 offload/test/offloading/fortran/default-mapper-nested-derived-type.f90 diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp index 3fe133d63d24..a60960e739d2 100644 --- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp +++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp @@ -1143,7 +1143,8 @@ class MapInfoFinalizationPass newMemberIndices.emplace_back(path); op.setMembersIndexAttr(builder.create2DI64ArrayAttr(newMemberIndices)); - op.setPartialMap(true); + // Set to partial map only if there is no user-defined mapper. + op.setPartialMap(op.getMapperIdAttr() == nullptr); return mlir::WalkResult::advance(); }); diff --git a/flang/test/Transforms/omp-map-info-finalization-implicit-field.fir b/flang/test/Transforms/omp-map-info-finalization-implicit-field.fir index 632525b4b43c..d3e8125d2ee3 100644 --- a/flang/test/Transforms/omp-map-info-finalization-implicit-field.fir +++ b/flang/test/Transforms/omp-map-info-finalization-implicit-field.fir @@ -15,13 +15,24 @@ fir.global internal @_QFEdst_record : !record_t { fir.has_value %0 : !record_t } +omp.declare_mapper @record_mapper : !record_t { +^bb0(%arg0: !fir.ref): + %0 = omp.map.info var_ptr(%arg0: !fir.ref, !record_t) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref + omp.declare_mapper.info map_entries(%0: !fir.ref) +} + func.func @_QQmain() { %6 = fir.address_of(@_QFEdst_record) : !fir.ref %7:2 = hlfir.declare %6 {uniq_name = "_QFEdst_record"} : (!fir.ref) -> (!fir.ref, !fir.ref) %16 = omp.map.info var_ptr(%7#1 : !fir.ref, !record_t) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref {name = "dst_record"} - omp.target map_entries(%16 -> %arg0 : !fir.ref) { + %17 = omp.map.info var_ptr(%7#1 : !fir.ref, !record_t) map_clauses(implicit, tofrom) capture(ByRef) mapper(@record_mapper) -> !fir.ref {name = "dst_record_with_mapper"} + omp.target map_entries(%16 -> %arg0, %17 -> %arg1 : !fir.ref, !fir.ref) { %20:2 = hlfir.declare %arg0 {uniq_name = "_QFEdst_record"} : (!fir.ref) -> (!fir.ref, !fir.ref) + %21:2 = hlfir.declare %arg1 {uniq_name = "_QFEdst_record"} : (!fir.ref) -> (!fir.ref, !fir.ref) + %23 = hlfir.designate %20#0{"to_implicitly_map"} {fortran_attrs = #fir.var_attrs} : (!fir.ref) -> !fir.ref>>> + + %24 = hlfir.designate %21#0{"to_implicitly_map"} {fortran_attrs = #fir.var_attrs} : (!fir.ref) -> !fir.ref>>> omp.terminator } return @@ -56,6 +67,19 @@ func.func @_QQmain() { // CHECK-SAME: [1], [1, 0] : {{.*}}) -> {{.*}}> {name = // CHECK-SAME: "dst_record", partial_map = true} +// Verify map ops when using a mapper: +// Implicit field mapping is the same as for the non-mapper case. +// CHECK: omp.map.info +// CHECK: omp.map.info + +// Verify that partial-map is not set if the map info op uses a user-defined (or +// compiler-emitted) mapper. +// CHECK: %[[RECORD_MAP_MAPPER:.*]] = omp.map.info var_ptr( +// CHECK-SAME: %[[RECORD_DECL]]#1 : {{.*}}) map_clauses( +// CHECK-SAME: implicit, tofrom) capture(ByRef) mapper(@record_mapper) +// CHECK-SAME: members(%{{.*}}, %{{.*}} : [1], [1, 0] : {{.*}}) -> {{.*}}> {name = +// CHECK-SAME: "dst_record_with_mapper"} + // CHECK: omp.target map_entries( // CHECK-SAME: %[[RECORD_MAP]] -> %{{[^[:space:]]+}}, // CHECK-SAME: %[[FIELD_MAP]] -> %{{[^[:space:]]+}}, diff --git a/offload/test/offloading/fortran/default-mapper-nested-derived-type.f90 b/offload/test/offloading/fortran/default-mapper-nested-derived-type.f90 new file mode 100644 index 000000000000..5d69fa072fd6 --- /dev/null +++ b/offload/test/offloading/fortran/default-mapper-nested-derived-type.f90 @@ -0,0 +1,34 @@ +! Regression test for default mappers emitted for nested derived types. Some +! optimization passes and instrumentation callbacks cause crashes in emitted +! mappers and this test guards against such crashes. + +! REQUIRES: flang, amdgpu + +! RUN: %libomptarget-compile-fortran-generic +! RUN: env LIBOMPTARGET_INFO=16 %libomptarget-run-generic 2>&1 | %fcheck-generic + +program test_omp_target_map_bug_v5 + implicit none + type nested_type + real, allocatable :: alloc_field(:) + end type nested_type + + type nesting_type + integer :: int_field + type(nested_type) :: derived_field + end type nesting_type + + type(nesting_type) :: config + + allocate(config%derived_field%alloc_field(1)) + + !$OMP TARGET ENTER DATA MAP(TO:config, config%derived_field%alloc_field) + + !$OMP TARGET + config%derived_field%alloc_field(1) = 1.0 + !$OMP END TARGET + + deallocate(config%derived_field%alloc_field) +end program test_omp_target_map_bug_v5 + +! CHECK: "PluginInterface" device {{[0-9]+}} info: Launching kernel {{.*}}