Skip to content

[flang][OpenMP] Support custom mappers in target update to/from clauses #169673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KrxGu wants to merge 6 commits into
base: main
Choose a base branch
from
+196 −87
Open

[flang][OpenMP] Support custom mappers in target update to/from clauses #169673

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b2a07c5
[flang][OpenMP] Support mappers in target update to/from
Nov 26, 2025
f378223
[flang][OpenMP] Implement mapper lowering for target update
Nov 26, 2025
d17b0ca
[flang][OpenMP] Refactor mapper resolution per review
Nov 27, 2025
52992b7
[flang] Fix clang-format issues
Nov 27, 2025
6b6ce4e
[flang] Fix test expectations for program-scoped mapper names
Nov 27, 2025
c620237
[flang] Extract mapper resolution into helper function
Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions flang/lib/Lower/OpenMP/ClauseProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1548,21 +1548,34 @@ bool ClauseProcessor::processMotionClauses(lower::StatementContext &stmtCtx,
mlir::Location clauseLocation = converter.genLocation(source);
const auto &[expectation, mapper, iterator, objects] = clause.t;

// TODO Support motion modifiers: mapper, iterator.
if (mapper) {
TODO(clauseLocation, "Mapper modifier is not supported yet");
} else if (iterator) {
TODO(clauseLocation, "Iterator modifier is not supported yet");
}

mlir::omp::ClauseMapFlags mapTypeBits =
std::is_same_v<llvm::remove_cvref_t<decltype(clause)>, omp::clause::To>
? mlir::omp::ClauseMapFlags::to
: mlir::omp::ClauseMapFlags::from;
if (expectation && *expectation == omp::clause::To::Expectation::Present)
mapTypeBits |= mlir::omp::ClauseMapFlags::present;

// Support motion modifiers: mapper, iterator.
std::string mapperIdName = "__implicit_mapper";
if (mapper) {
// Only one mapper is allowed by the parser here.
assert(mapper->size() == 1 && "more than one mapper");
const semantics::Symbol *mapperSym = mapper->front().v.id().symbol;
mapperIdName = mapperSym->name().ToString();
if (mapperIdName != "default") {
// Mangle with the ultimate owner so that use-associated mapper
// identifiers resolve to the same symbol as their defining scope.
const semantics::Symbol &ultimate = mapperSym->GetUltimate();
mapperIdName = converter.mangleName(mapperIdName, ultimate.owner());
}
}
if (iterator) {
TODO(clauseLocation, "Iterator modifier is not supported yet");
}

processMapObjects(stmtCtx, clauseLocation, objects, mapTypeBits,
parentMemberIndices, result.mapVars, mapSymbols);
parentMemberIndices, result.mapVars, mapSymbols,
mapperIdName);
};

bool clauseFound = findRepeatableClause<omp::clause::To>(callbackFn);
Expand Down
60 changes: 60 additions & 0 deletions flang/lib/Semantics/resolve-names.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1631,6 +1631,8 @@ class OmpVisitor : public virtual DeclarationVisitor {
return true;
}
bool Pre(const parser::OmpMapClause &);
bool Pre(const parser::OmpClause::To &);
bool Pre(const parser::OmpClause::From &);

bool Pre(const parser::OpenMPSectionsConstruct &) {
PushScope(Scope::Kind::OtherConstruct, nullptr);
Expand Down Expand Up @@ -1876,6 +1878,64 @@ bool OmpVisitor::Pre(const parser::OmpMapClause &x) {
return true;
}

bool OmpVisitor::Pre(const parser::OmpClause::To &x) {
// Resolve mapper names in "to" clauses (e.g., for target update)
auto &mods{OmpGetModifiers(x.v)};
if (auto *mapper{OmpGetUniqueModifier<parser::OmpMapper>(mods)}) {
if (auto *symbol{FindSymbol(currScope(), mapper->v)}) {
auto &ultimate{symbol->GetUltimate()};
auto *misc{ultimate.detailsIf<MiscDetails>()};
auto *md{ultimate.detailsIf<MapperDetails>()};
if (!md && (!misc || misc->kind() != MiscDetails::Kind::ConstructName))
context().Say(mapper->v.source,
"Name '%s' should be a mapper name"_err_en_US, mapper->v.source);
else
mapper->v.symbol = symbol;
} else {
// Allow the special 'default' mapper identifier without prior
// declaration so lowering can recognize and handle it. Emit an
// error for any other missing mapper identifier.
if (mapper->v.source.ToString() == "default") {
mapper->v.symbol = &MakeSymbol(
mapper->v, MiscDetails{MiscDetails::Kind::ConstructName});
} else {
context().Say(
mapper->v.source, "'%s' not declared"_err_en_US, mapper->v.source);
}
}
}
return true;
}

bool OmpVisitor::Pre(const parser::OmpClause::From &x) {
// Resolve mapper names in "from" clauses (e.g., for target update)
auto &mods{OmpGetModifiers(x.v)};
if (auto *mapper{OmpGetUniqueModifier<parser::OmpMapper>(mods)}) {
if (auto *symbol{FindSymbol(currScope(), mapper->v)}) {
auto &ultimate{symbol->GetUltimate()};
auto *misc{ultimate.detailsIf<MiscDetails>()};
auto *md{ultimate.detailsIf<MapperDetails>()};
if (!md && (!misc || misc->kind() != MiscDetails::Kind::ConstructName))
context().Say(mapper->v.source,
"Name '%s' should be a mapper name"_err_en_US, mapper->v.source);
else
mapper->v.symbol = symbol;
} else {
// Allow the special 'default' mapper identifier without prior
// declaration so lowering can recognize and handle it. Emit an
// error for any other missing mapper identifier.
if (mapper->v.source.ToString() == "default") {
mapper->v.symbol = &MakeSymbol(
mapper->v, MiscDetails{MiscDetails::Kind::ConstructName});
} else {
context().Say(
mapper->v.source, "'%s' not declared"_err_en_US, mapper->v.source);
}
}
}
return true;
}

void OmpVisitor::ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec,
const parser::OmpClauseList &clauses) {
// This "manually" walks the tree of the construct, because we need
Expand Down
51 changes: 51 additions & 0 deletions flang/test/Lower/OpenMP/target-update-mapper.f90
View file
Open in desktop
Copy link
Member

@TIFitis TIFitis Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the lowering test to flang/test/Lower/OpenMP/declare-mapper.f90.

Also add an offloading test in offload/test/offloading/fortran/.

Copy link
Contributor Author

@KrxGu KrxGu Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add an offloading test in offload/test/offloading/fortran/target-update-custom-mapper.f90, however I don't have access to GPU hardware(GPU poor 🙃) and can only test on CPU.

I've prepared the test and verified to the best of my capability:

  • Correct Fortran syntax (no syntax errors)
  • Compiles successfully to MLIR
  • Generated IR contains proper omp.declare_mapper declarations for both custom and default mappers
  • omp.target_update operations correctly reference the mappers (mapper(@_QQFcustom) and mapper(@_QQFmy_type_omp_default_mapper))

The test follows the same pattern as existing offloading tests like target-custom-mapper.f90. It will require GPU hardware to actually execute (REQUIRES: flang, amdgpu).

Here's the test code:

! Test custom mappers with target update to/from clauses
! REQUIRES: flang, amdgpu

! RUN: %libomptarget-compile-fortran-run-and-check-generic

program target_update_mapper_test
   implicit none
   integer, parameter :: n = 100
   
   type :: my_type
      integer :: a(n)
      integer :: b(n)
   end type my_type

   ! Declare custom mapper that only maps field 'a'
   !$omp declare mapper(custom : my_type :: t) map(t%a)
   
   ! Declare default mapper that maps both fields
   !$omp declare mapper(my_type :: t) map(t%a, t%b)

   type(my_type) :: obj
   integer :: i, sum_a, sum_b

   ! Initialize data on host
   do i = 1, n
      obj%a(i) = i
      obj%b(i) = i * 2
   end do

   ! Allocate space on device using custom mapper (only maps 'a')
   !$omp target enter data map(mapper(custom), alloc: obj)

   ! Update field 'a' to device using custom mapper
   obj%a = 10
   !$omp target update to(mapper(custom): obj)

   ! Read back field 'a' from device (should be 10)
   obj%a = 0
   !$omp target update from(mapper(custom): obj)
   
   sum_a = sum(obj%a)
   sum_b = sum(obj%b)

   ! CHECK: Sum of a after update from device: 1000
   print *, "Sum of a after update from device:", sum_a
   
   ! Field 'b' was never mapped, so should still have original values
   ! CHECK: Sum of b (never mapped): 10100
   print *, "Sum of b (never mapped):", sum_b

   ! Now test with default mapper (maps both fields)
   obj%a = 20
   obj%b = 30
   
   !$omp target update to(mapper(default): obj)
   
   obj%a = 0
   obj%b = 0
   
   !$omp target update from(mapper(default): obj)
   
   sum_a = sum(obj%a)
   sum_b = sum(obj%b)
   
   ! CHECK: Sum of a with default mapper: 2000
   print *, "Sum of a with default mapper:", sum_a
   
   ! CHECK: Sum of b with default mapper: 3000
   print *, "Sum of b with default mapper:", sum_b

   !$omp target exit data map(delete: obj)

   ! CHECK: Test passed!
   print *, "Test passed!"

end program target_update_mapper_test

Let me know if you'd like me to include this in the PR, or if you prefer to validate it on GPU hardware first.

Copy link
Member

@TIFitis TIFitis Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

# .---command stderr------------
# | /home/akash/Documents/llvm-work/offload/test/offloading/fortran/target-declare-mapper-to-from.f90:68:11: error: CHECK: expected string not found in input
# |  ! CHECK: Sum of b with default mapper: 3000
# |           ^
# | <stdin>:3:36: note: scanning from here
# |  Sum of a with default mapper: 2000
# |                                    ^
# | <stdin>:4:2: note: possible intended match here
# |  Sum of b with default mapper: 0
# |  ^
# | 
# | Input file: <stdin>
# | Check file: /home/akash/Documents/llvm-work/offload/test/offloading/fortran/target-declare-mapper-to-from.f90
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |             1:  Sum of a after update from device: 1000 
# |             2:  Sum of b (never mapped): 10100 
# |             3:  Sum of a with default mapper: 2000 
# | check:68'0                                        X error: no match found
# |             4:  Sum of b with default mapper: 0 
# | check:68'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# | check:68'1      ?                                possible intended match
# |             5:  Test passed! 
# | check:68'0     ~~~~~~~~~~~~~~
# | >>>>>>
# `-----------------------------

This test does not pass on AMDGPU in it's current state c620237

Copy link
Contributor Author

@KrxGu KrxGu Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TIFitis Thanks for testing this! You're absolutely right - I found the logic flaw in my test.

The issue is that I'm mixing mappers inconsistently:

  1. Line 37: I allocate device memory using custom mapper (which only maps field a)
  2. Line 58: I try to update using default mapper (which maps both a and b)

Since field b was never allocated on the device, the update fails and reads back as 0.

Here's a corrected version that properly separates the two test scenarios:

! Test custom mappers with target update to/from clauses
! REQUIRES: flang, amdgpu

! RUN: %libomptarget-compile-fortran-run-and-check-generic

program target_update_mapper_test
   implicit none
   integer, parameter :: n = 100
   
   type :: my_type
      integer :: a(n)
      integer :: b(n)
   end type my_type

   ! Declare custom mapper that only maps field 'a'
   !$omp declare mapper(custom : my_type :: t) map(t%a)
   
   ! Declare default mapper that maps both fields
   !$omp declare mapper(my_type :: t) map(t%a, t%b)

   type(my_type) :: obj
   integer :: i, sum_a, sum_b

   ! ========== Test 1: Custom mapper (field 'a' only) ==========
   
   ! Initialize data on host
   do i = 1, n
      obj%a(i) = i
      obj%b(i) = i * 2
   end do

   ! Allocate and update using custom mapper (only 'a')
   !$omp target enter data map(mapper(custom), alloc: obj)

   obj%a = 10
   !$omp target update to(mapper(custom): obj)

   obj%a = 0
   !$omp target update from(mapper(custom): obj)
   
   sum_a = sum(obj%a)
   sum_b = sum(obj%b)

   ! CHECK: Sum of a (custom mapper): 1000
   print *, "Sum of a (custom mapper):", sum_a
   
   ! Field 'b' was never mapped with custom mapper
   ! CHECK: Sum of b (never mapped): 10100
   print *, "Sum of b (never mapped):", sum_b

   !$omp target exit data map(mapper(custom), delete: obj)

   ! ========== Test 2: Default mapper (both fields) ==========
   
   ! Re-initialize
   do i = 1, n
      obj%a(i) = 20
      obj%b(i) = 30
   end do
   
   ! Allocate and update using default mapper (both 'a' and 'b')
   !$omp target enter data map(mapper(default), alloc: obj)
   
   !$omp target update to(mapper(default): obj)
   
   obj%a = 0
   obj%b = 0
   
   !$omp target update from(mapper(default): obj)
   
   sum_a = sum(obj%a)
   sum_b = sum(obj%b)
   
   ! CHECK: Sum of a (default mapper): 2000
   print *, "Sum of a (default mapper):", sum_a
   
   ! CHECK: Sum of b (default mapper): 3000
   print *, "Sum of b (default mapper):", sum_b

   !$omp target exit data map(mapper(default), delete: obj)

   ! CHECK: Test passed!
   print *, "Test passed!"

end program target_update_mapper_test

Would you prefer:

Option 1: I add this corrected test to the PR now (you can validate on GPU hardware through CI)
Option 2: We handle this as a follow-up PR or maybe someone who has GPU access can add it .🙃

Let me know what works best for you!

Copy link
Member

@TIFitis TIFitis Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test is still not passing

flang -fopenmp -fopenmp-version=50 -fopenmp-targets=amdgcn-amd-amdhsa --offload-arch=gfx1100 -O3 test.f90 -o a.out && ./a.out 
warning: OpenMP support for version 50 in flang is still incomplete
warning: OpenMP support for version 50 in flang is still incomplete
warning: OpenMP support for version 50 in flang is still incomplete
 Sum of a (custom mapper): 1000
 Sum of b (never mapped): 10100
omptarget message: explicit extension not allowed: host address specified is 0x000064cc7ba9a660 (800 bytes), but device allocation maps to host at 0x000064cc7ba9a660 (400 bytes)
omptarget error: Call to getTargetPointer returned null pointer (device failure or illegal mapping).
omptarget error: Call to targetDataBegin via targetDataMapper for custom mapper failed.
omptarget error: Consult https://openmp.llvm.org/design/Runtimes.html for debugging options.
omptarget error: Source location information not present. Compile with -g or -gline-tables-only.
omptarget fatal error 1: failure of target construct while offloading is mandatory
Aborted (core dumped)

We can't land a patch without a test, even more so when the test is failing.

Unfortunately, I'm going on vacation until next year. Hopefully you can find someone else who can help you with the offloading test.

Cheers

Copy link
Contributor Author

@KrxGu KrxGu Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test is still not passing

flang -fopenmp -fopenmp-version=50 -fopenmp-targets=amdgcn-amd-amdhsa --offload-arch=gfx1100 -O3 test.f90 -o a.out && ./a.out 
warning: OpenMP support for version 50 in flang is still incomplete
warning: OpenMP support for version 50 in flang is still incomplete
warning: OpenMP support for version 50 in flang is still incomplete
 Sum of a (custom mapper): 1000
 Sum of b (never mapped): 10100
omptarget message: explicit extension not allowed: host address specified is 0x000064cc7ba9a660 (800 bytes), but device allocation maps to host at 0x000064cc7ba9a660 (400 bytes)
omptarget error: Call to getTargetPointer returned null pointer (device failure or illegal mapping).
omptarget error: Call to targetDataBegin via targetDataMapper for custom mapper failed.
omptarget error: Consult https://openmp.llvm.org/design/Runtimes.html for debugging options.
omptarget error: Source location information not present. Compile with -g or -gline-tables-only.
omptarget fatal error 1: failure of target construct while offloading is mandatory
Aborted (core dumped)

We can't land a patch without a test, even more so when the test is failing.

Unfortunately, I'm going on vacation until next year. Hopefully you can find someone else who can help you with the offloading test.

Cheers

@tblah , @kparzysz can someone please help me with this, I really want this PR to get merged.

Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 %s -o - | FileCheck %s

! Test mapper usage in target update to/from clauses

program target_update_mapper
implicit none

integer, parameter :: n = 4

type :: typ
integer, allocatable :: a(:)
integer, allocatable :: b(:)
end type typ

!$omp declare mapper(custom: typ :: t) map(t%a)

! CHECK-LABEL: omp.declare_mapper @_QQFcustom : !fir.type<_QFTtyp{a:!fir.box<!fir.heap<!fir.array<?xi32>>>,b:!fir.box<!fir.heap<!fir.array<?xi32>>>}>

type(typ) :: t

allocate(t%a(n), source=1)
allocate(t%b(n), source=2)

!$omp target enter data map(alloc: t)

! Test target update to with custom mapper
! CHECK: %[[MAP_INFO:.*]] = omp.map.info var_ptr(%{{.*}} : {{.*}}, {{.*}}) map_clauses(to) capture(ByRef) mapper(@_QQFcustom) -> {{.*}}
! CHECK: omp.target_update map_entries(%[[MAP_INFO]] : {{.*}})
t%a = 42
!$omp target update to(mapper(custom): t)

!$omp target
t%a(:) = t%a(:) / 2
t%b(:) = -1
!$omp end target

! Test target update from with custom mapper
! CHECK: %[[MAP_INFO2:.*]] = omp.map.info var_ptr(%{{.*}} : {{.*}}, {{.*}}) map_clauses(from) capture(ByRef) mapper(@_QQFcustom) -> {{.*}}
! CHECK: omp.target_update map_entries(%[[MAP_INFO2]] : {{.*}})
!$omp target update from(mapper(custom): t)

! Test target update to with default mapper
! CHECK: %[[MAP_INFO3:.*]] = omp.map.info var_ptr(%{{.*}} : {{.*}}, {{.*}}) map_clauses(to) capture(ByRef) mapper(@_QQFtyp_omp_default_mapper) -> {{.*}}
! CHECK: omp.target_update map_entries(%[[MAP_INFO3]] : {{.*}})
!$omp target update to(mapper(default): t)

!$omp target exit data map(delete: t)
deallocate(t%a)
deallocate(t%b)

end program target_update_mapper
54 changes: 54 additions & 0 deletions flang/test/Semantics/OpenMP/target-update-mapper.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=52

! Test mapper usage in target update to/from clauses

program target_update_mapper
implicit none

integer, parameter :: n = 4

type :: typ
integer, allocatable :: a(:)
integer, allocatable :: b(:)
end type typ

!$omp declare mapper(custom: typ :: t) map(t%a)

type(typ) :: t
integer :: not_a_mapper
allocate(t%a(n), source=1)
allocate(t%b(n), source=2)

!$omp target enter data map(alloc: t)

! Valid: using custom mapper with target update to
t%a = 42
!$omp target update to(mapper(custom): t)

!$omp target
t%a(:) = t%a(:) / 2
t%b(:) = -1
!$omp end target

! Valid: using custom mapper with target update from
!$omp target update from(mapper(custom): t)

! Valid: using default mapper explicitly
!$omp target update to(mapper(default): t)

print*, t%a
print*, t%b

!$omp target exit data map(delete: t)
deallocate(t%a)
deallocate(t%b)

! Test error case: undefined mapper
!ERROR: 'undefined_mapper' not declared
!$omp target update to(mapper(undefined_mapper): t)

! Test error case: wrong kind of symbol
!ERROR: Name 'not_a_mapper' should be a mapper name
!$omp target update from(mapper(not_a_mapper): t)

end program target_update_mapper