From c1c79c1fd82bcc13f647fff076fa9979785b4ddd Mon Sep 17 00:00:00 2001 From: Jack Styles Date: Wed, 18 Jun 2025 15:08:08 +0100 Subject: [PATCH 1/3] [Flang][OpenMP] Update default MapType for Map Clauses and OpenMP 5.2 In OpenMP 5.2, the `target enter data` and `target exit data` constructs now have default map types if the user does not define them in the Map clause. For `target enter data`, this is `to` and `target exit data` this is `false`. This behaviour is now enabled when OpenMP 5.2 or greater is used when compiling. To enable this, the default value is now set in the `processMap` clause, with any previous behaviour being maintained for either older versions of OpenMP or other directives. --- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 17 ++++++++--- flang/lib/Lower/OpenMP/ClauseProcessor.h | 1 + flang/lib/Lower/OpenMP/Clauses.cpp | 2 +- flang/lib/Lower/OpenMP/OpenMP.cpp | 5 ++-- .../target-enter-data-default-openmp52.f90 | 28 +++++++++++++++++++ 5 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index b5c8de8c2ce8b..c093ecf4664a9 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -1243,7 +1243,7 @@ void ClauseProcessor::processMapObjects( bool ClauseProcessor::processMap( mlir::Location currentLocation, lower::StatementContext &stmtCtx, - mlir::omp::MapClauseOps &result, + mlir::omp::MapClauseOps &result, llvm::omp::Directive directive, llvm::SmallVectorImpl *mapSyms) const { // We always require tracking of symbols, even if the caller does not, // so we create an optionally used local set of symbols when the mapSyms @@ -1261,9 +1261,18 @@ bool ClauseProcessor::processMap( llvm::omp::OpenMPOffloadMappingFlags mapTypeBits = llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE; std::string mapperIdName = "__implicit_mapper"; - // If the map type is specified, then process it else Tofrom is the - // default. - Map::MapType type = mapType.value_or(Map::MapType::Tofrom); + // If the map type is specified, then process it else set the appropriate + // default value + Map::MapType type; + if (directive == llvm::omp::Directive::OMPD_target_enter_data && + semaCtx.langOptions().OpenMPVersion >= 52) + type = mapType.value_or(Map::MapType::To); + else if (directive == llvm::omp::Directive::OMPD_target_exit_data && + semaCtx.langOptions().OpenMPVersion >= 52) + type = mapType.value_or(Map::MapType::From); + else + type = mapType.value_or(Map::MapType::Tofrom); + switch (type) { case Map::MapType::To: mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO; diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h index c957a94d387e9..3d8c4a337a4a4 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.h +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h @@ -139,6 +139,7 @@ class ClauseProcessor { bool processMap(mlir::Location currentLocation, lower::StatementContext &stmtCtx, mlir::omp::MapClauseOps &result, + llvm::omp::Directive directive = llvm::omp::OMPD_unknown, llvm::SmallVectorImpl *mapSyms = nullptr) const; bool processMotionClauses(lower::StatementContext &stmtCtx, diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index c0c57d1832d4e..b599d69a36272 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -1043,7 +1043,7 @@ Map make(const parser::OmpClause::Map &inp, auto type = [&]() -> std::optional { if (t3) return convert1(t3->v); - return Map::MapType::Tofrom; + return std::nullopt; }(); Map::MapTypeModifiers typeMods; diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 3e865a1ee7185..e89ed653aa0b4 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -1831,7 +1831,8 @@ static void genTargetClauses( } cp.processIf(llvm::omp::Directive::OMPD_target, clauseOps); cp.processIsDevicePtr(clauseOps, isDevicePtrSyms); - cp.processMap(loc, stmtCtx, clauseOps, &mapSyms); + cp.processMap(loc, stmtCtx, clauseOps, llvm::omp::Directive::OMPD_unknown, + &mapSyms); cp.processNowait(clauseOps); cp.processThreadLimit(stmtCtx, clauseOps); @@ -1884,7 +1885,7 @@ static void genTargetEnterExitUpdateDataClauses( if (directive == llvm::omp::Directive::OMPD_target_update) cp.processMotionClauses(stmtCtx, clauseOps); else - cp.processMap(loc, stmtCtx, clauseOps); + cp.processMap(loc, stmtCtx, clauseOps, directive); cp.processNowait(clauseOps); } diff --git a/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 b/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 new file mode 100644 index 0000000000000..0f44b7f944abe --- /dev/null +++ b/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 @@ -0,0 +1,28 @@ +! This test checks the lowering and application of default map types for the target enter/exit data constructs and map clauses + +!RUN: %flang -fc1 -emit-fir -fopenmp -fopenmp-version=52 -o - %s | FileCheck %s --check-prefix=CHECK-52 +!RUN: not %flang -fc1 -emit-fir -fopenmp -fopenmp-version=51 -o - %s 2<&1| FileCheck %s --check-prefix=CHECK-51 + +module test + use omp_lib + real, allocatable :: A + +contains + subroutine initialize() + allocate(A) + !$omp target enter data map(A) + !CHECK-52: %276 = omp.map.info var_ptr(%2 : !fir.ref>>, f32) map_clauses(to) capture(ByRef) var_ptr_ptr(%275 : !fir.llvm_ptr>) -> !fir.llvm_ptr> {name = ""} + !CHECK-52: %277 = omp.map.info var_ptr(%2 : !fir.ref>>, !fir.box>) map_clauses(to) capture(ByRef) members(%276 : [0] : !fir.llvm_ptr>) -> !fir.ref>> {name = "a"} + !CHECK-51: to and alloc map types are permitted + + end subroutine initialize + + subroutine finalize() + !$omp target exit data map(A) + !CHECK-52: %274 = omp.map.info var_ptr(%2 : !fir.ref>>, f32) map_clauses(from) capture(ByRef) var_ptr_ptr(%273 : !fir.llvm_ptr>) -> !fir.llvm_ptr> {name = ""} + !CHECK-52: %275 = omp.map.info var_ptr(%2 : !fir.ref>>, !fir.box>) map_clauses(from) capture(ByRef) members(%274 : [0] : !fir.llvm_ptr>) -> !fir.ref>> {name = "a"} + !CHECK-51: from, release and delete map types are permitted + deallocate(A) + + end subroutine finalize +end module test From 64d8a708369d50de1c30a99cbb702471b7b9f20d Mon Sep 17 00:00:00 2001 From: Jack Styles Date: Wed, 18 Jun 2025 16:09:57 +0100 Subject: [PATCH 2/3] Fix failing test --- .../Lower/OpenMP/target-enter-data-default-openmp52.f90 | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 b/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 index 0f44b7f944abe..3b81de9a9c459 100644 --- a/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 +++ b/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 @@ -4,23 +4,22 @@ !RUN: not %flang -fc1 -emit-fir -fopenmp -fopenmp-version=51 -o - %s 2<&1| FileCheck %s --check-prefix=CHECK-51 module test - use omp_lib real, allocatable :: A contains subroutine initialize() allocate(A) !$omp target enter data map(A) - !CHECK-52: %276 = omp.map.info var_ptr(%2 : !fir.ref>>, f32) map_clauses(to) capture(ByRef) var_ptr_ptr(%275 : !fir.llvm_ptr>) -> !fir.llvm_ptr> {name = ""} - !CHECK-52: %277 = omp.map.info var_ptr(%2 : !fir.ref>>, !fir.box>) map_clauses(to) capture(ByRef) members(%276 : [0] : !fir.llvm_ptr>) -> !fir.ref>> {name = "a"} + !CHECK-52: omp.map.info var_ptr(%2 : !fir.ref>>, f32) map_clauses(to) capture(ByRef) var_ptr_ptr(%5 : !fir.llvm_ptr>) -> !fir.llvm_ptr> {name = ""} + !CHECK-52: omp.map.info var_ptr(%2 : !fir.ref>>, !fir.box>) map_clauses(to) capture(ByRef) members(%6 : [0] : !fir.llvm_ptr>) -> !fir.ref>> {name = "a"} !CHECK-51: to and alloc map types are permitted end subroutine initialize subroutine finalize() !$omp target exit data map(A) - !CHECK-52: %274 = omp.map.info var_ptr(%2 : !fir.ref>>, f32) map_clauses(from) capture(ByRef) var_ptr_ptr(%273 : !fir.llvm_ptr>) -> !fir.llvm_ptr> {name = ""} - !CHECK-52: %275 = omp.map.info var_ptr(%2 : !fir.ref>>, !fir.box>) map_clauses(from) capture(ByRef) members(%274 : [0] : !fir.llvm_ptr>) -> !fir.ref>> {name = "a"} + !CHECK-52: omp.map.info var_ptr(%2 : !fir.ref>>, f32) map_clauses(from) capture(ByRef) var_ptr_ptr(%3 : !fir.llvm_ptr>) -> !fir.llvm_ptr> {name = ""} + !CHECK-52: omp.map.info var_ptr(%2 : !fir.ref>>, !fir.box>) map_clauses(from) capture(ByRef) members(%4 : [0] : !fir.llvm_ptr>) -> !fir.ref>> {name = "a"} !CHECK-51: from, release and delete map types are permitted deallocate(A) From 63c5f836d68a49d5452136a8a0bef2515ab225cf Mon Sep 17 00:00:00 2001 From: Jack Styles Date: Wed, 18 Jun 2025 17:16:37 +0100 Subject: [PATCH 3/3] Fix notation in run line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was causing a test failure on Windows§ --- flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 b/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 index 3b81de9a9c459..0d4fd964b71ec 100644 --- a/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 +++ b/flang/test/Lower/OpenMP/target-enter-data-default-openmp52.f90 @@ -1,7 +1,7 @@ ! This test checks the lowering and application of default map types for the target enter/exit data constructs and map clauses !RUN: %flang -fc1 -emit-fir -fopenmp -fopenmp-version=52 -o - %s | FileCheck %s --check-prefix=CHECK-52 -!RUN: not %flang -fc1 -emit-fir -fopenmp -fopenmp-version=51 -o - %s 2<&1| FileCheck %s --check-prefix=CHECK-51 +!RUN: not %flang -fc1 -emit-fir -fopenmp -fopenmp-version=51 -o - %s 2>&1| FileCheck %s --check-prefix=CHECK-51 module test real, allocatable :: A