From d1fe42bb1250877b94e04873481346ccc3cce0dd Mon Sep 17 00:00:00 2001 From: Anchu Rajendran Date: Fri, 1 Mar 2024 18:58:57 -0600 Subject: [PATCH 1/4] [flang] Changes to correctly map variables in link clause of declare target As per the standard, "If a variable appears in a link clause on a declare target directive that does not have a device_type clause with the nohost device-type-description then it is treated as if it had appeared in a map clause with a map-type of tofrom" is an implicit mapping rule. Before this change, such variables were mapped as to by default. --- flang/lib/Lower/OpenMP/OpenMP.cpp | 16 +++++- .../OpenMP/declare-target-link-tarop-cap.f90 | 55 +++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 flang/test/Lower/OpenMP/declare-target-link-tarop-cap.f90 diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 185e0316870e9..d90e217af66c5 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -1120,7 +1120,21 @@ genTargetOp(Fortran::lower::AbstractConverter &converter, if (auto refType = baseOp.getType().dyn_cast()) eleType = refType.getElementType(); - if (fir::isa_trivial(eleType) || fir::isa_char(eleType)) { + // If a variable is specified in declare target link and if device + // type is nohost, it needs to be mapped tofrom + mlir::ModuleOp mod = converter.getFirOpBuilder().getModule(); + mlir::Operation *op = mod.lookupSymbol(converter.mangleName(sym)); + auto declareTargetOp = + llvm::dyn_cast_if_present(op); + if (declareTargetOp && declareTargetOp.isDeclareTarget()) { + if (declareTargetOp.getDeclareTargetCaptureClause() == + mlir::omp::DeclareTargetCaptureClause::link && + declareTargetOp.getDeclareTargetDeviceType() != + mlir::omp::DeclareTargetDeviceType::nohost) { + mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO; + mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM; + } + } else if (fir::isa_trivial(eleType) || fir::isa_char(eleType)) { captureKind = mlir::omp::VariableCaptureKind::ByCopy; } else if (!fir::isa_builtin_cptr_type(eleType)) { mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO; diff --git a/flang/test/Lower/OpenMP/declare-target-link-tarop-cap.f90 b/flang/test/Lower/OpenMP/declare-target-link-tarop-cap.f90 new file mode 100644 index 0000000000000..56f7db4d060d9 --- /dev/null +++ b/flang/test/Lower/OpenMP/declare-target-link-tarop-cap.f90 @@ -0,0 +1,55 @@ +!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s +!RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s +!RUN: bbc -emit-hlfir -fopenmp %s -o - | FileCheck %s +!RUN: bbc -emit-hlfir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s + +program test_link + +integer :: test_int = 1 +!$omp declare target link(test_int) + +integer :: test_array_1d(3) = (/1,2,3/) +!$omp declare target link(test_array_1d) + +integer, pointer :: test_ptr1 +!$omp declare target link(test_ptr1) + +integer, target :: test_target = 1 +!$omp declare target link(test_target) + +integer, pointer :: test_ptr2 +!$omp declare target link(test_ptr2) + +!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref, i32) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref {name = "test_int"} +!$omp target + test_int = test_int + 1 +!$omp end target + + +!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>, !fir.array<3xi32>) map_clauses(implicit, tofrom) capture(ByRef) bounds({{%.*}}) -> !fir.ref> {name = "test_array_1d"} +!$omp target + do i = 1,3 + test_array_1d(i) = i * 2 + end do +!$omp end target + +allocate(test_ptr1) +test_ptr1 = 1 +!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>>, !fir.box>) map_clauses(implicit, tofrom) capture(ByRef) members({{%.*}} : !fir.llvm_ptr>) -> !fir.ref>> {name = "test_ptr1"} +!$omp target + test_ptr1 = test_ptr1 + 1 +!$omp end target + +!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref, i32) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref {name = "test_target"} +!$omp target + test_target = test_target + 1 +!$omp end target + + +!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>>, !fir.box>) map_clauses(implicit, tofrom) capture(ByRef) members({{%.*}} : !fir.llvm_ptr>) -> !fir.ref>> {name = "test_ptr2"} +test_ptr2 => test_target +!$omp target + test_ptr2 = test_ptr2 + 1 +!$omp end target + +end From 1721b2b93ff4b3a067db64d5da71e7b572e4c3d7 Mon Sep 17 00:00:00 2001 From: Anchu Rajendran Date: Mon, 4 Mar 2024 11:18:09 -0600 Subject: [PATCH 2/4] Revision 2: adding runtime test, fixing comment --- flang/lib/Lower/OpenMP/OpenMP.cpp | 2 +- .../declare-target-array-in-target-region.f90 | 52 ++++++++++++++++--- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index d90e217af66c5..5cff95c7d125b 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -1121,7 +1121,7 @@ genTargetOp(Fortran::lower::AbstractConverter &converter, eleType = refType.getElementType(); // If a variable is specified in declare target link and if device - // type is nohost, it needs to be mapped tofrom + // type is not specified as `nohost`, it needs to be mapped tofrom mlir::ModuleOp mod = converter.getFirOpBuilder().getModule(); mlir::Operation *op = mod.lookupSymbol(converter.mangleName(sym)); auto declareTargetOp = diff --git a/openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90 b/openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90 index c09146198768b..bebdbb7f54de6 100644 --- a/openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90 +++ b/openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90 @@ -11,23 +11,59 @@ ! RUN: %libomptarget-compile-fortran-run-and-check-generic module test_0 implicit none - INTEGER :: sp(10) = (/0,0,0,0,0,0,0,0,0,0/) - !$omp declare target link(sp) + INTEGER :: arr1(10) = (/0,0,0,0,0,0,0,0,0,0/) + INTEGER :: arr2(10) = (/0,0,0,0,0,0,0,0,0,0/) + !$omp declare target link(arr1) enter(arr2) end module test_0 -program main +subroutine test_with_array_link_and_tofrom() use test_0 integer :: i = 1 integer :: j = 11 -!$omp target map(tofrom:sp, i, j) +!$omp target map(tofrom:arr1, i, j) do while (i <= j) - sp(i) = i; + arr1(i) = i; i = i + 1 end do !$omp end target -PRINT *, sp(:) +! CHECK: 1 2 3 4 5 6 7 8 9 10 +PRINT *, arr1(:) +end subroutine test_with_array_link_and_tofrom -end program +subroutine test_with_array_link_only() +use test_0 +integer :: i = 1 +integer :: j = 11 +!$omp target map(i, j) + do while (i <= j) + arr1(i) = i + 1; + i = i + 1 + end do +!$omp end target -! CHECK: 1 2 3 4 5 6 7 8 9 10 +! CHECK: 2 3 4 5 6 7 8 9 10 11 +PRINT *, arr1(:) +end subroutine test_with_array_link_only + +subroutine test_with_array_enter_only() +use test_0 +integer :: i = 1 +integer :: j = 11 +!$omp target map(i, j) + do while (i <= j) + arr2(i) = i + 1; + i = i + 1 + end do +!$omp end target + +! CHECK: 0 0 0 0 0 0 0 0 0 0 +PRINT *, arr2(:) +end subroutine test_with_array_enter_only + + +program main +call test_with_array_link_and_tofrom() +call test_with_array_link_only() +call test_with_array_enter_only() +end program From e7766011f6cfdf2a035ef059df76ae0589dac8e8 Mon Sep 17 00:00:00 2001 From: Anchu Rajendran Date: Wed, 6 Mar 2024 12:41:44 -0600 Subject: [PATCH 3/4] R3: Reformatting test cases, adding a scalar test --- .../OpenMP/declare-target-link-tarop-cap.f90 | 70 ++++++++-------- .../declare-target-array-in-target-region.f90 | 69 ---------------- .../declare-target-vars-in-target-region.f90 | 81 +++++++++++++++++++ 3 files changed, 116 insertions(+), 104 deletions(-) delete mode 100644 openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90 create mode 100644 openmp/libomptarget/test/offloading/fortran/declare-target-vars-in-target-region.f90 diff --git a/flang/test/Lower/OpenMP/declare-target-link-tarop-cap.f90 b/flang/test/Lower/OpenMP/declare-target-link-tarop-cap.f90 index 56f7db4d060d9..7cd0597161578 100644 --- a/flang/test/Lower/OpenMP/declare-target-link-tarop-cap.f90 +++ b/flang/test/Lower/OpenMP/declare-target-link-tarop-cap.f90 @@ -5,51 +5,51 @@ program test_link -integer :: test_int = 1 -!$omp declare target link(test_int) + integer :: test_int = 1 + !$omp declare target link(test_int) -integer :: test_array_1d(3) = (/1,2,3/) -!$omp declare target link(test_array_1d) + integer :: test_array_1d(3) = (/1,2,3/) + !$omp declare target link(test_array_1d) -integer, pointer :: test_ptr1 -!$omp declare target link(test_ptr1) + integer, pointer :: test_ptr1 + !$omp declare target link(test_ptr1) -integer, target :: test_target = 1 -!$omp declare target link(test_target) + integer, target :: test_target = 1 + !$omp declare target link(test_target) -integer, pointer :: test_ptr2 -!$omp declare target link(test_ptr2) + integer, pointer :: test_ptr2 + !$omp declare target link(test_ptr2) -!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref, i32) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref {name = "test_int"} -!$omp target - test_int = test_int + 1 -!$omp end target + !CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref, i32) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref {name = "test_int"} + !$omp target + test_int = test_int + 1 + !$omp end target -!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>, !fir.array<3xi32>) map_clauses(implicit, tofrom) capture(ByRef) bounds({{%.*}}) -> !fir.ref> {name = "test_array_1d"} -!$omp target - do i = 1,3 - test_array_1d(i) = i * 2 - end do -!$omp end target + !CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>, !fir.array<3xi32>) map_clauses(implicit, tofrom) capture(ByRef) bounds({{%.*}}) -> !fir.ref> {name = "test_array_1d"} + !$omp target + do i = 1,3 + test_array_1d(i) = i * 2 + end do + !$omp end target -allocate(test_ptr1) -test_ptr1 = 1 -!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>>, !fir.box>) map_clauses(implicit, tofrom) capture(ByRef) members({{%.*}} : !fir.llvm_ptr>) -> !fir.ref>> {name = "test_ptr1"} -!$omp target - test_ptr1 = test_ptr1 + 1 -!$omp end target + allocate(test_ptr1) + test_ptr1 = 1 + !CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>>, !fir.box>) map_clauses(implicit, tofrom) capture(ByRef) members({{%.*}} : !fir.llvm_ptr>) -> !fir.ref>> {name = "test_ptr1"} + !$omp target + test_ptr1 = test_ptr1 + 1 + !$omp end target -!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref, i32) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref {name = "test_target"} -!$omp target - test_target = test_target + 1 -!$omp end target + !CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref, i32) map_clauses(implicit, tofrom) capture(ByRef) -> !fir.ref {name = "test_target"} + !$omp target + test_target = test_target + 1 + !$omp end target -!CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>>, !fir.box>) map_clauses(implicit, tofrom) capture(ByRef) members({{%.*}} : !fir.llvm_ptr>) -> !fir.ref>> {name = "test_ptr2"} -test_ptr2 => test_target -!$omp target - test_ptr2 = test_ptr2 + 1 -!$omp end target + !CHECK-DAG: {{%.*}} = omp.map_info var_ptr({{%.*}} : !fir.ref>>, !fir.box>) map_clauses(implicit, tofrom) capture(ByRef) members({{%.*}} : !fir.llvm_ptr>) -> !fir.ref>> {name = "test_ptr2"} + test_ptr2 => test_target + !$omp target + test_ptr2 = test_ptr2 + 1 + !$omp end target end diff --git a/openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90 b/openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90 deleted file mode 100644 index bebdbb7f54de6..0000000000000 --- a/openmp/libomptarget/test/offloading/fortran/declare-target-array-in-target-region.f90 +++ /dev/null @@ -1,69 +0,0 @@ -! Offloading test with a target region mapping a declare target -! Fortran array writing some values to it and checking the host -! correctly receives the updates made on the device. -! REQUIRES: flang -! UNSUPPORTED: nvptx64-nvidia-cuda-LTO -! UNSUPPORTED: aarch64-unknown-linux-gnu -! UNSUPPORTED: aarch64-unknown-linux-gnu-LTO -! UNSUPPORTED: x86_64-pc-linux-gnu -! UNSUPPORTED: x86_64-pc-linux-gnu-LTO - -! RUN: %libomptarget-compile-fortran-run-and-check-generic -module test_0 - implicit none - INTEGER :: arr1(10) = (/0,0,0,0,0,0,0,0,0,0/) - INTEGER :: arr2(10) = (/0,0,0,0,0,0,0,0,0,0/) - !$omp declare target link(arr1) enter(arr2) -end module test_0 - -subroutine test_with_array_link_and_tofrom() - use test_0 - integer :: i = 1 - integer :: j = 11 -!$omp target map(tofrom:arr1, i, j) - do while (i <= j) - arr1(i) = i; - i = i + 1 - end do -!$omp end target - -! CHECK: 1 2 3 4 5 6 7 8 9 10 -PRINT *, arr1(:) -end subroutine test_with_array_link_and_tofrom - -subroutine test_with_array_link_only() -use test_0 -integer :: i = 1 -integer :: j = 11 -!$omp target map(i, j) - do while (i <= j) - arr1(i) = i + 1; - i = i + 1 - end do -!$omp end target - -! CHECK: 2 3 4 5 6 7 8 9 10 11 -PRINT *, arr1(:) -end subroutine test_with_array_link_only - -subroutine test_with_array_enter_only() -use test_0 -integer :: i = 1 -integer :: j = 11 -!$omp target map(i, j) - do while (i <= j) - arr2(i) = i + 1; - i = i + 1 - end do -!$omp end target - -! CHECK: 0 0 0 0 0 0 0 0 0 0 -PRINT *, arr2(:) -end subroutine test_with_array_enter_only - - -program main -call test_with_array_link_and_tofrom() -call test_with_array_link_only() -call test_with_array_enter_only() -end program diff --git a/openmp/libomptarget/test/offloading/fortran/declare-target-vars-in-target-region.f90 b/openmp/libomptarget/test/offloading/fortran/declare-target-vars-in-target-region.f90 new file mode 100644 index 0000000000000..24957b60c69a9 --- /dev/null +++ b/openmp/libomptarget/test/offloading/fortran/declare-target-vars-in-target-region.f90 @@ -0,0 +1,81 @@ +! Offloading test with a target region mapping a declare target +! Fortran array writing some values to it and checking the host +! correctly receives the updates made on the device. +! REQUIRES: flang +! UNSUPPORTED: nvptx64-nvidia-cuda-LTO +! UNSUPPORTED: aarch64-unknown-linux-gnu +! UNSUPPORTED: aarch64-unknown-linux-gnu-LTO +! UNSUPPORTED: x86_64-pc-linux-gnu +! UNSUPPORTED: x86_64-pc-linux-gnu-LTO + +! RUN: %libomptarget-compile-fortran-run-and-check-generic +module test_0 + implicit none + INTEGER :: arr1(10) = (/0,0,0,0,0,0,0,0,0,0/) + INTEGER :: arr2(10) = (/0,0,0,0,0,0,0,0,0,0/) + !$omp declare target link(arr1) enter(arr2) + INTEGER :: scalar = 1 + !$omp declare target link(scalar) +end module test_0 + +subroutine test_with_array_link_and_tofrom() + use test_0 + integer :: i = 1 + integer :: j = 11 + !$omp target map(tofrom:arr1, i, j) + do while (i <= j) + arr1(i) = i; + i = i + 1 + end do + !$omp end target + + ! CHECK: 1 2 3 4 5 6 7 8 9 10 + PRINT *, arr1(:) +end subroutine test_with_array_link_and_tofrom + +subroutine test_with_array_link_only() + use test_0 + integer :: i = 1 + integer :: j = 11 + !$omp target map(i, j) + do while (i <= j) + arr1(i) = i + 1; + i = i + 1 + end do + !$omp end target + + ! CHECK: 2 3 4 5 6 7 8 9 10 11 + PRINT *, arr1(:) +end subroutine test_with_array_link_only + +subroutine test_with_array_enter_only() + use test_0 + integer :: i = 1 + integer :: j = 11 + !$omp target map(i, j) + do while (i <= j) + arr2(i) = i + 1; + i = i + 1 + end do + !$omp end target + + ! CHECK: 0 0 0 0 0 0 0 0 0 0 + PRINT *, arr2(:) +end subroutine test_with_array_enter_only + +subroutine test_with_scalar_link_only() + use test_0 + !$omp target + scalar = 10 + !$omp end target + + ! CHECK: 10 + PRINT *, scalar +end subroutine test_with_scalar_link_only + +program main + call test_with_array_link_and_tofrom() + call test_with_array_link_only() + call test_with_array_enter_only() + call test_with_scalar_link_only() +end program From 43e31c3695c7bda8e60b8dcb9b9e18128c8d92c5 Mon Sep 17 00:00:00 2001 From: Anchu Rajendran Date: Wed, 6 Mar 2024 15:18:43 -0600 Subject: [PATCH 4/4] R4: Fixing minor formating issues in the test --- .../declare-target-vars-in-target-region.f90 | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/openmp/libomptarget/test/offloading/fortran/declare-target-vars-in-target-region.f90 b/openmp/libomptarget/test/offloading/fortran/declare-target-vars-in-target-region.f90 index 24957b60c69a9..f524deac3bcce 100644 --- a/openmp/libomptarget/test/offloading/fortran/declare-target-vars-in-target-region.f90 +++ b/openmp/libomptarget/test/offloading/fortran/declare-target-vars-in-target-region.f90 @@ -10,16 +10,16 @@ ! RUN: %libomptarget-compile-fortran-run-and-check-generic module test_0 - implicit none - INTEGER :: arr1(10) = (/0,0,0,0,0,0,0,0,0,0/) - INTEGER :: arr2(10) = (/0,0,0,0,0,0,0,0,0,0/) - !$omp declare target link(arr1) enter(arr2) - INTEGER :: scalar = 1 - !$omp declare target link(scalar) + implicit none + INTEGER :: arr1(10) = (/0,0,0,0,0,0,0,0,0,0/) + INTEGER :: arr2(10) = (/0,0,0,0,0,0,0,0,0,0/) + !$omp declare target link(arr1) enter(arr2) + INTEGER :: scalar = 1 + !$omp declare target link(scalar) end module test_0 subroutine test_with_array_link_and_tofrom() - use test_0 + use test_0 integer :: i = 1 integer :: j = 11 !$omp target map(tofrom:arr1, i, j)