diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index 75df4163c8a55..5530cc646d29e 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -1595,7 +1595,8 @@ void ClauseProcessor::processMapObjects( std::map &parentMemberIndices, llvm::SmallVectorImpl &mapVars, llvm::SmallVectorImpl &mapSyms, - llvm::StringRef mapperIdNameRef, bool isMotionModifier) const { + llvm::StringRef mapperIdNameRef, bool isMotionModifier, + llvm::omp::Directive directive) const { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); auto getSymbolDerivedType = [](const semantics::Symbol &symbol) @@ -1610,14 +1611,7 @@ void ClauseProcessor::processMapObjects( auto addImplicitMapper = [&](const omp::Object &object, std::string &mapperIdName, bool allowGenerate) -> mlir::FlatSymbolRefAttr { - if (mapperIdName.empty()) - return mlir::FlatSymbolRefAttr(); - - if (converter.getModuleOp().lookupSymbol(mapperIdName)) - return mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(), - mapperIdName); - - if (!allowGenerate) + if (!allowGenerate || mapperIdName.empty()) return mlir::FlatSymbolRefAttr(); const semantics::DerivedTypeSpec *typeSpec = @@ -1664,24 +1658,28 @@ void ClauseProcessor::processMapObjects( return mapperIdName; }; - // Create the mapper symbol from its name, if specified. - mlir::FlatSymbolRefAttr mapperId; - if (!mapperIdNameRef.empty() && !objects.empty() && - mapperIdNameRef != "__implicit_mapper") { - std::string mapperIdName = mapperIdNameRef.str(); - const omp::Object &object = objects.front(); - if (mapperIdNameRef == "default") { - const semantics::DerivedTypeSpec *typeSpec = - getSymbolDerivedType(*object.sym()); - if (!typeSpec && object.sym()->owner().IsDerivedType()) - typeSpec = object.sym()->owner().derivedTypeSpec(); - mapperIdName = getDefaultMapperID(typeSpec); + auto findMapperIfTypeMatch = + [&](const semantics::DerivedTypeSpec *objectTypeSpec, + llvm::StringRef explicitMapperName) -> std::string { + auto declMapperOp = + converter.getModuleOp().lookupSymbol( + explicitMapperName); + if (!declMapperOp) + return "__implicit_mapper"; + + // Verify if the explicit mapper provided matches the type being mapped. + // If it does return the mapper name, if it doesn't return null-ary. + mlir::Type mapperType = declMapperOp.getType(); + mlir::Type objectType = converter.genType(*objectTypeSpec); + auto mapperRecordType = mlir::dyn_cast(mapperType); + auto objectRecordType = mlir::dyn_cast(objectType); + if (mapperRecordType && objectRecordType && + mapperRecordType.getName() == objectRecordType.getName()) { + return explicitMapperName.str(); } - assert(converter.getModuleOp().lookupSymbol(mapperIdName) && - "mapper not found"); - mapperId = - mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(), mapperIdName); - } + + return "__implicit_mapper"; + }; for (const omp::Object &object : objects) { llvm::SmallVector bounds; @@ -1716,35 +1714,59 @@ void ClauseProcessor::processMapObjects( const semantics::DerivedTypeSpec *objectTypeSpec = getSymbolDerivedType(*object.sym()); - - if (mapperIdNameRef == "__implicit_mapper") { - if (parentObj.has_value()) { - mapperId = mlir::FlatSymbolRefAttr(); - } else if (objectTypeSpec) { - std::string mapperIdName = getDefaultMapperID(objectTypeSpec); - bool isAllocOrPointer = - semantics::IsAllocatableOrObjectPointer(object.sym()); - bool isPointer = semantics::IsPointer(*object.sym()); - bool isImplicitMap = - (mapTypeBits & mlir::omp::ClauseMapFlags::implicit) == - mlir::omp::ClauseMapFlags::implicit; - bool needsDefaultMapper = - isAllocOrPointer || - requiresImplicitDefaultDeclareMapper(*objectTypeSpec); - // For implicit captures, avoid synthesizing default mappers for pointer - // entities (which can over-map pointer payloads) and for plain - // non-allocatable/non-pointer entities. Keep implicit mapper support - // for allocatables. - if (isImplicitMap && (isPointer || !isAllocOrPointer)) - needsDefaultMapper = false; - if (!mapperIdName.empty()) + mlir::FlatSymbolRefAttr mapperId = mlir::FlatSymbolRefAttr(); + if (objectTypeSpec) { + std::string mapperIdName = mapperIdNameRef.str(); + // if we have an explicit mapper specified, we need to check it matches + // the type being mapped, if it doesn't we fallback to look for a user + // default mapper or generate an compiler defined default mapper if + // relevant. This function will return "__implicit_mapper" if we find that + // the map isn't relevant to the explicit declare mapper, which allows it + // to fallback. + if (!mapperIdName.empty() && mapperIdName != "__implicit_mapper") + mapperIdName = findMapperIfTypeMatch(objectTypeSpec, mapperIdName); + + if (mapperIdName == "__implicit_mapper") { + mapperIdName = getDefaultMapperID(objectTypeSpec); + // Currently we do not apply implicit compiler generated delcare mappers + // to enter, exit or update directives. However, we will syntheize one + // below if we're not a target enter/exit/update and no user defined + // implicit declare mapper has been defined and we meet the other + // conditions + // TODO/FIXME: Loosen this restriction to comply with the OpenMP + // specification. + auto *userDefinedDefault = + converter.getModuleOp().lookupSymbol(mapperIdName); + if (!userDefinedDefault && !parentObj.has_value() && + (directive != llvm::omp::Directive::OMPD_target_enter_data && + directive != llvm::omp::Directive::OMPD_target_exit_data && + directive != llvm::omp::Directive::OMPD_target_update)) { + bool isAllocOrPointer = + semantics::IsAllocatableOrObjectPointer(object.sym()); + bool isPointer = semantics::IsPointer(*object.sym()); + bool isImplicitMap = + (mapTypeBits & mlir::omp::ClauseMapFlags::implicit) == + mlir::omp::ClauseMapFlags::implicit; + bool needsDefaultMapper = + isAllocOrPointer || + requiresImplicitDefaultDeclareMapper(*objectTypeSpec); + // For implicit captures, avoid synthesizing default mappers for + // pointer entities (which can over-map pointer payloads) and for + // plain non-allocatable/non-pointer entities. Keep implicit mapper + // support for allocatables. + if (isImplicitMap && (isPointer || !isAllocOrPointer)) + needsDefaultMapper = false; mapperId = addImplicitMapper(object, mapperIdName, /*allowGenerate=*/needsDefaultMapper); - else - mapperId = mlir::FlatSymbolRefAttr(); - } else { - mapperId = mlir::FlatSymbolRefAttr(); + } } + + // Make sure we've generated the symbol in one of our previous steps + // before assigning the symbol. + if (!mapperIdName.empty() && + converter.getModuleOp().lookupSymbol(mapperIdName)) + mapperId = mlir::FlatSymbolRefAttr::get(&converter.getMLIRContext(), + mapperIdName); } // Explicit map captures are captured ByRef by default, @@ -1817,15 +1839,9 @@ bool ClauseProcessor::processMap( if (attachMod) TODO(currentLocation, "ATTACH modifier is not implemented yet"); mlir::omp::ClauseMapFlags mapTypeBits = mlir::omp::ClauseMapFlags::none; - // For data-motion directives we avoid auto-attaching implicit default - // mappers. Deep recursive mapping there can conflict with explicit - // component enter/exit maps users commonly spell out. + std::string mapperIdName = getMapperIdentifier(converter, mappers); - if ((directive == llvm::omp::Directive::OMPD_target_enter_data || - directive == llvm::omp::Directive::OMPD_target_exit_data || - directive == llvm::omp::Directive::OMPD_target_update) && - mapperIdName == "__implicit_mapper") - mapperIdName.clear(); + // If the map type is specified, then process it else set the appropriate // default value Map::MapType type; @@ -1875,7 +1891,7 @@ bool ClauseProcessor::processMap( processMapObjects(stmtCtx, clauseLocation, std::get(clause.t), mapTypeBits, parentMemberIndices, result.mapVars, *ptrMapSyms, - mapperIdName); + mapperIdName, /*isMotionModifier=*/false, directive); }; bool clauseFound = findRepeatableClause(process); @@ -1901,10 +1917,9 @@ bool ClauseProcessor::processMotionClauses(lower::StatementContext &stmtCtx, if (expectation && *expectation == omp::clause::To::Expectation::Present) mapTypeBits |= mlir::omp::ClauseMapFlags::present; - // Support motion modifiers: mapper, iterator. + // Support motion modifiers: iterator. std::string mapperIdName = getMapperIdentifier(converter, mapper); - if (mapperIdName == "__implicit_mapper") - mapperIdName.clear(); + if (iterator) { TODO(clauseLocation, "Iterator modifier is not supported yet"); } diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h index 6c6056aac77e3..f343ee8ff4332 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.h +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h @@ -211,8 +211,8 @@ class ClauseProcessor { std::map &parentMemberIndices, llvm::SmallVectorImpl &mapVars, llvm::SmallVectorImpl &mapSyms, - llvm::StringRef mapperIdNameRef = "", - bool isMotionModifier = false) const; + llvm::StringRef mapperIdNameRef = "", bool isMotionModifier = false, + llvm::omp::Directive directive = llvm::omp::OMPD_unknown) const; lower::AbstractConverter &converter; semantics::SemanticsContext &semaCtx; diff --git a/flang/test/Lower/OpenMP/declare-mapper.f90 b/flang/test/Lower/OpenMP/declare-mapper.f90 index 18de556e2ce7d..5383ece9e5390 100644 --- a/flang/test/Lower/OpenMP/declare-mapper.f90 +++ b/flang/test/Lower/OpenMP/declare-mapper.f90 @@ -12,6 +12,9 @@ ! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 -module-dir %t %t/omp-declare-mapper-8.mod.f90 -o - >/dev/null ! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 -J %t %t/omp-declare-mapper-8.use.f90 -o - | FileCheck %t/omp-declare-mapper-8.use.f90 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 %t/omp-declare-mapper-9.f90 -o - | FileCheck %t/omp-declare-mapper-9.f90 +! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-10.f90 -o - | FileCheck %t/omp-declare-mapper-10.f90 +! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-11.f90 -o - | FileCheck %t/omp-declare-mapper-11.f90 +! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-12.f90 -o - | FileCheck %t/omp-declare-mapper-12.f90 !--- omp-declare-mapper-1.f90 subroutine declare_mapper_1 @@ -394,3 +397,129 @@ program target_update_mapper !$omp target update to(mapper(default): t) end program target_update_mapper + +!--- omp-declare-mapper-10.f90 +! Test that default mapper is applied only to the matching type (dtype_a) and not to dtype_b +subroutine declare_mapper_10 + type dtype_a + real, allocatable :: arr(:) + real, allocatable :: arr2(:) + end type dtype_a + + type dtype_b + real, allocatable :: arr(:) + real, allocatable :: arr2(:) + end type dtype_b + + ! CHECK: omp.declare_mapper @[[MAPPER_A:_QQFdeclare_mapper_10dtype_a_omp_default_mapper]] : !fir.type<_QFdeclare_mapper_10Tdtype_a + !$omp declare mapper(dtype_a :: ty) map(ty%arr, ty%arr2) + + type(dtype_a), pointer :: dtype + type(dtype_b), pointer :: dtype2 + integer :: var_a, var_b + + ! dtype (dtype_a) should have mapper applied via var_ptr_ptr + ! CHECK: omp.map.info {{.*}} mapper(@[[MAPPER_A]]){{.*}}{name = ""} + ! CHECK: omp.map.info {{.*}} {name = "dtype"} + ! var_a and var_b are integers - no mapper + ! CHECK: omp.map.info {{.*}} map_clauses(to) capture(ByRef) -> !fir.ref {name = "var_a"} + ! CHECK: omp.map.info {{.*}} map_clauses(to) capture(ByRef) -> !fir.ref {name = "var_b"} + ! dtype2 (dtype_b) should NOT have mapper applied + ! CHECK: omp.map.info {{.*}} map_clauses(to) capture(ByRef) var_ptr_ptr({{.*}}) -> {{.*}} {name = ""} + ! CHECK-NOT: mapper(@ + ! CHECK: omp.map.info {{.*}} {name = "dtype2"} + ! CHECK: omp.target_enter_data + !$omp target enter data map(to: dtype, var_a, var_b, dtype2) +end subroutine + +!--- omp-declare-mapper-11.f90 +! Test that named mapper overrides default mapper when explicitly specified +subroutine declare_mapper_11 + type dtype_a + real, allocatable :: arr(:) + real, allocatable :: arr2(:) + end type dtype_a + + type dtype_b + real, allocatable :: arr(:) + real, allocatable :: arr2(:) + end type dtype_b + + ! CHECK-DAG: omp.declare_mapper @[[MAPPER_TESTING:_QQFdeclare_mapper_11testing]] : !fir.type<_QFdeclare_mapper_11Tdtype_a + ! CHECK-DAG: omp.declare_mapper @[[MAPPER_DEFAULT:_QQFdeclare_mapper_11dtype_a_omp_default_mapper]] : !fir.type<_QFdeclare_mapper_11Tdtype_a + !$omp declare mapper(dtype_a :: ty) map(ty%arr, ty%arr2) + !$omp declare mapper(testing : dtype_a :: ty) map(ty%arr) + + type(dtype_a), pointer :: dtype + type(dtype_b), pointer :: dtype2 + integer :: var_a, var_b + + ! dtype (dtype_a) should use named mapper "testing" when explicitly specified + ! CHECK: omp.map.info {{.*}} mapper(@[[MAPPER_TESTING]]){{.*}}{name = ""} + ! CHECK: omp.map.info {{.*}} {name = "dtype"} + ! var_a and var_b are integers - no mapper + ! CHECK: omp.map.info {{.*}} map_clauses(to) capture(ByRef) -> !fir.ref {name = "var_a"} + ! CHECK: omp.map.info {{.*}} map_clauses(to) capture(ByRef) -> !fir.ref {name = "var_b"} + ! dtype2 (dtype_b) should NOT have mapper - no mapper defined for dtype_b + ! CHECK: omp.map.info {{.*}} map_clauses(to) capture(ByRef) var_ptr_ptr({{.*}}) -> {{.*}} {name = ""} + ! CHECK-NOT: mapper(@ + ! CHECK: omp.map.info {{.*}} {name = "dtype2"} + ! CHECK: omp.target_enter_data + !$omp target enter data map(mapper(testing), to: dtype, var_a, var_b, dtype2) +end subroutine + +!--- omp-declare-mapper-12.f90 +! Test multiple types with different mappers - each type gets its appropriate mapper +subroutine declare_mapper_12 + type dtype_a + real, allocatable :: arr(:) + real, allocatable :: arr2(:) + end type dtype_a + + type dtype_b + real, allocatable :: arr(:) + real, allocatable :: arr2(:) + end type dtype_b + + type dtype_c + real, allocatable :: arr(:) + end type dtype_c + + type dtype_d + real, allocatable :: arr2(:) + end type dtype_d + + ! CHECK-DAG: omp.declare_mapper @[[MAPPER_TESTING:_QQFdeclare_mapper_12testing]] : !fir.type<_QFdeclare_mapper_12Tdtype_c + ! CHECK-DAG: omp.declare_mapper @[[MAPPER_B:_QQFdeclare_mapper_12dtype_b_omp_default_mapper]] : !fir.type<_QFdeclare_mapper_12Tdtype_b + ! CHECK-DAG: omp.declare_mapper @[[MAPPER_A:_QQFdeclare_mapper_12dtype_a_omp_default_mapper]] : !fir.type<_QFdeclare_mapper_12Tdtype_a + !$omp declare mapper(dtype_a :: ty) map(ty%arr, ty%arr2) + !$omp declare mapper(dtype_b :: ty) map(ty%arr, ty%arr2) + !$omp declare mapper(testing : dtype_c :: ty) map(ty%arr) + + type(dtype_a), pointer :: dtype + type(dtype_b), pointer :: dtype2 + type(dtype_c), pointer :: dtype3 + type(dtype_d), pointer :: dtype4 + + ! When mapper(testing) is specified: + ! - dtype (dtype_a): gets default mapper for dtype_a (testing is for dtype_c) + ! - dtype2 (dtype_b): gets default mapper for dtype_b + ! - dtype3 (dtype_c): gets "testing" mapper (exact match) + ! - dtype4 (dtype_d): no mapper (no mapper defined for dtype_d) + + ! dtype should get MAPPER_A (default for dtype_a) + ! CHECK: omp.map.info {{.*}} mapper(@[[MAPPER_A]]){{.*}}{name = ""} + ! CHECK: omp.map.info {{.*}} {name = "dtype"} + ! dtype2 should get MAPPER_B (default for dtype_b) + ! CHECK: omp.map.info {{.*}} mapper(@[[MAPPER_B]]){{.*}}{name = ""} + ! CHECK: omp.map.info {{.*}} {name = "dtype2"} + ! dtype3 should get MAPPER_TESTING (explicit match) + ! CHECK: omp.map.info {{.*}} mapper(@[[MAPPER_TESTING]]){{.*}}{name = ""} + ! CHECK: omp.map.info {{.*}} {name = "dtype3"} + ! dtype4 should NOT have any mapper + ! CHECK: omp.map.info {{.*}} map_clauses(to) capture(ByRef) var_ptr_ptr({{.*}}) -> {{.*}} {name = ""} + ! CHECK-NOT: mapper(@ + ! CHECK: omp.map.info {{.*}} {name = "dtype4"} + ! CHECK: omp.target_enter_data + !$omp target enter data map(mapper(testing), to: dtype, dtype2, dtype3, dtype4) +end subroutine diff --git a/flang/test/Lower/OpenMP/target-motion-skip-implicit-mapper.f90 b/flang/test/Lower/OpenMP/target-motion-skip-implicit-mapper.f90 deleted file mode 100644 index 067e7b06d2cf0..0000000000000 --- a/flang/test/Lower/OpenMP/target-motion-skip-implicit-mapper.f90 +++ /dev/null @@ -1,30 +0,0 @@ -! RUN: bbc -emit-hlfir -fopenmp -fopenmp-version=52 %s -o - | FileCheck %s - -module m - type :: t - integer :: a - end type - !$omp declare mapper(t :: v) map(v%a) -contains - subroutine test_motion(x) - type(t) :: x - - !$omp target enter data map(to: x) - !$omp target update to(x) - !$omp target update from(x) - !$omp target update to(mapper(default): x) - !$omp target exit data map(from: x) - end subroutine test_motion -end module m - -! CHECK-LABEL: func.func @_QMmPtest_motion( -! CHECK: %[[ENTER:.*]] = omp.map.info {{.*}} map_clauses(to) capture(ByRef) -> {{.*}} {name = "x"} -! CHECK: omp.target_enter_data map_entries(%[[ENTER]] -! CHECK: %[[UPTO:.*]] = omp.map.info {{.*}} map_clauses(to) capture(ByRef) -> {{.*}} {name = "x"} -! CHECK: omp.target_update map_entries(%[[UPTO]] -! CHECK: %[[UPFROM:.*]] = omp.map.info {{.*}} map_clauses(from) capture(ByRef) -> {{.*}} {name = "x"} -! CHECK: omp.target_update map_entries(%[[UPFROM]] -! CHECK: %[[UPDEFAULT:.*]] = omp.map.info {{.*}} map_clauses(to) capture(ByRef) mapper(@{{.*}}t_omp_default_mapper) -> {{.*}} {name = "x"} -! CHECK: omp.target_update map_entries(%[[UPDEFAULT]] -! CHECK: %[[EXIT:.*]] = omp.map.info {{.*}} map_clauses(from) capture(ByRef) -> {{.*}} {name = "x"} -! CHECK: omp.target_exit_data map_entries(%[[EXIT]]