[Flang][OpenMP] Update default MapType for Map Clauses and OpenMP 5.2 #144715
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
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.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Jack Styles (Stylie777) ChangesIn OpenMP 5.2, the Full diff: https://github.com/llvm/llvm-project/pull/144715.diff 5 Files Affected:
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<const semantics::Symbol *> *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<const semantics::Symbol *> *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<Map::MapType> {
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<!fir.box<!fir.heap<f32>>>, f32) map_clauses(to) capture(ByRef) var_ptr_ptr(%275 : !fir.llvm_ptr<!fir.ref<f32>>) -> !fir.llvm_ptr<!fir.ref<f32>> {name = ""}
+ !CHECK-52: %277 = omp.map.info var_ptr(%2 : !fir.ref<!fir.box<!fir.heap<f32>>>, !fir.box<!fir.heap<f32>>) map_clauses(to) capture(ByRef) members(%276 : [0] : !fir.llvm_ptr<!fir.ref<f32>>) -> !fir.ref<!fir.box<!fir.heap<f32>>> {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<!fir.box<!fir.heap<f32>>>, f32) map_clauses(from) capture(ByRef) var_ptr_ptr(%273 : !fir.llvm_ptr<!fir.ref<f32>>) -> !fir.llvm_ptr<!fir.ref<f32>> {name = ""}
+ !CHECK-52: %275 = omp.map.info var_ptr(%2 : !fir.ref<!fir.box<!fir.heap<f32>>>, !fir.box<!fir.heap<f32>>) map_clauses(from) capture(ByRef) members(%274 : [0] : !fir.llvm_ptr<!fir.ref<f32>>) -> !fir.ref<!fir.box<!fir.heap<f32>>> {name = "a"}
+ !CHECK-51: from, release and delete map types are permitted
+ deallocate(A)
+
+ end subroutine finalize
+end module test
|
|
@llvm/pr-subscribers-flang-openmp Author: Jack Styles (Stylie777) ChangesIn OpenMP 5.2, the Full diff: https://github.com/llvm/llvm-project/pull/144715.diff 5 Files Affected:
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<const semantics::Symbol *> *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<const semantics::Symbol *> *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<Map::MapType> {
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<!fir.box<!fir.heap<f32>>>, f32) map_clauses(to) capture(ByRef) var_ptr_ptr(%275 : !fir.llvm_ptr<!fir.ref<f32>>) -> !fir.llvm_ptr<!fir.ref<f32>> {name = ""}
+ !CHECK-52: %277 = omp.map.info var_ptr(%2 : !fir.ref<!fir.box<!fir.heap<f32>>>, !fir.box<!fir.heap<f32>>) map_clauses(to) capture(ByRef) members(%276 : [0] : !fir.llvm_ptr<!fir.ref<f32>>) -> !fir.ref<!fir.box<!fir.heap<f32>>> {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<!fir.box<!fir.heap<f32>>>, f32) map_clauses(from) capture(ByRef) var_ptr_ptr(%273 : !fir.llvm_ptr<!fir.ref<f32>>) -> !fir.llvm_ptr<!fir.ref<f32>> {name = ""}
+ !CHECK-52: %275 = omp.map.info var_ptr(%2 : !fir.ref<!fir.box<!fir.heap<f32>>>, !fir.box<!fir.heap<f32>>) map_clauses(from) capture(ByRef) members(%274 : [0] : !fir.llvm_ptr<!fir.ref<f32>>) -> !fir.ref<!fir.box<!fir.heap<f32>>> {name = "a"}
+ !CHECK-51: from, release and delete map types are permitted
+ deallocate(A)
+
+ end subroutine finalize
+end module test
|
tblah
approved these changes
Jun 18, 2025
| !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 |
There was a problem hiding this comment.
I don't think this line is needed, and it leads to the test failing on builders not configured to build the openmp runtime.
If this does turn out to be needed you need to add
! REQUIRES: openmp_runtime
so that the test gets skipped on machines that do not build the runtime.
There was a problem hiding this comment.
Yes, this is only needed if the tests uses some constants or functions from omp_lib.
There was a problem hiding this comment.
I have removed use omp_lib.
|
Typo in the commit message: |
|
@kparzysz Apologies, I have updated the commit message. |
kparzysz
approved these changes
Jun 18, 2025
This was causing a test failure on Windows§
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.
In OpenMP 5.2, the
target enter dataandtarget exit dataconstructs now have default map types if the user does not define them in the Map clause. Fortarget enter data, this istoandtarget exit datathis isfrom. 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 theprocessMapclause, with any previous behaviour being maintained for either older versions of OpenMP or other directives.See also #110008