Skip to content

Fixing the location attribute added to mapInfoOp #90764

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.

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

Merged
merged 2 commits into from
May 23, 2024

Conversation

anchuraj
Copy link
Contributor

@anchuraj anchuraj commented May 1, 2024

Named location attribute added to tgt_offload_entry shall be used by runtime calls like ompx_dump_mapping_tables to print the information of variables that are mapped to the device. ompx_dump_mapping_tables was printing the wrong location information and this change fixes it.

A sample execution of example before the change:

omptarget device 0 info: OpenMP Host-Device pointer mappings after block at libomptarget:0:0:

omptarget device 0 info: Host Ptr           Target Ptr         Size (B) DynRefCount HoldRefCount Declaration

omptarget device 0 info: 0x0000000000206df0 0x00007f02cdc00000 20000000 1           0            <program-file-loc> at unknown:18:35

The change replaces unknown to the mapped symbol and location to the declaration location.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir offload labels May 1, 2024
@llvmbot
Copy link
Member

llvmbot commented May 1, 2024

@llvm/pr-subscribers-offload

@llvm/pr-subscribers-flang-fir-hlfir

Author: Anchu Rajendran S (anchuraj)

Changes

Named location attribute added to tgt_offload_entry shall be used by runtime calls like ompx_dump_mapping_tables to print the information of variables that are mapped to the device. ompx_dump_mapping_tables was printing the wrong location information and this change fixes it.

A sample execution of example before the change:

omptarget device 0 info: OpenMP Host-Device pointer mappings after block at libomptarget:0:0:

omptarget device 0 info: Host Ptr           Target Ptr         Size (B) DynRefCount HoldRefCount Declaration

omptarget device 0 info: 0x0000000000206df0 0x00007f02cdc00000 20000000 1           0            &lt;program-file-loc&gt; at unknown:18:35

The change replaces unknown to the mapped symbol and location to the declaration location.


Full diff: https://github.com/llvm/llvm-project/pull/90764.diff

3 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+3-1)
  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+4-2)
  • (added) offload/test/offloading/fortran/dump_map_tables.f90 (+38)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 4c51b61f6bf029..1af954f3927b57 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -903,8 +903,10 @@ bool ClauseProcessor::processMap(
           // Explicit map captures are captured ByRef by default,
           // optimisation passes may alter this to ByCopy or other capture
           // types to optimise
+          auto context = converter.getFirOpBuilder().getContext();
+          auto location = mlir::NameLoc::get(mlir::StringAttr::get(context, asFortran.str()), symAddr.getLoc());
           mlir::Value mapOp = createMapInfoOp(
-              firOpBuilder, clauseLocation, symAddr, mlir::Value{},
+              firOpBuilder, location, symAddr, mlir::Value{},
               asFortran.str(), bounds, {},
               static_cast<
                   std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index e932f7c284bca8..00d9eca19d3cc5 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1645,9 +1645,11 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
           mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
           mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
         }
-
+    ;
+        auto context = converter.getFirOpBuilder().getContext();
+        auto location = mlir::NameLoc::get(mlir::StringAttr::get(context, sym.name().ToString()), baseOp.getLoc());
         mlir::Value mapOp = createMapInfoOp(
-            firOpBuilder, baseOp.getLoc(), baseOp, mlir::Value{}, name.str(),
+            firOpBuilder, location, baseOp, mlir::Value{}, name.str(),
             bounds, {},
             static_cast<
                 std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
diff --git a/offload/test/offloading/fortran/dump_map_tables.f90 b/offload/test/offloading/fortran/dump_map_tables.f90
new file mode 100644
index 00000000000000..93de49205b32ec
--- /dev/null
+++ b/offload/test/offloading/fortran/dump_map_tables.f90
@@ -0,0 +1,38 @@
+! 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
+
+program map_dump_example
+  INTERFACE
+    SUBROUTINE ompx_dump_mapping_tables() BIND(C)
+    END SUBROUTINE ompx_dump_mapping_tables
+  END INTERFACE
+
+  integer i,j,k,N
+  integer async_q(4)
+  real :: A(5000000),B(5000000),C(5000000)
+  N=5000000
+  do i=1, N
+    A(i)=0
+  enddo
+! clang-format off
+! CHECK: omptarget device 0 info: OpenMP Host-Device pointer mappings after block
+! CHECK-NEXT: omptarget device 0 info: Host Ptr Target Ptr Size (B) DynRefCount HoldRefCount Declaration
+! CHECK-NEXT: omptarget device 0 info: {{(0x[0-9a-f]{16})}} {{(0x[0-9a-f]{16})}}  20000000 1 0 {{.*}} at a(:n):21:11
+! clang-format on
+!$omp target enter data map(to:A(:N))
+  call ompx_dump_mapping_tables()
+!$omp target parallel do
+  do i=1, N
+    A(i)=B(i)*C(i)
+  enddo
+!$omp target exit data map(from:A)
+end program

@anchuraj anchuraj requested review from agozillon and skatrak May 1, 2024 19:19
Copy link

github-actions bot commented May 1, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@anchuraj anchuraj force-pushed the fix-ompx-dump-mapping-tables branch from b15058e to 7d80a3f Compare May 1, 2024 19:25
@anchuraj anchuraj assigned raghavendhra and unassigned raghavendhra May 1, 2024
@anchuraj anchuraj requested a review from raghavendhra May 1, 2024 19:31
Copy link
Contributor

@agozillon agozillon left a comment

Choose a reason for hiding this comment

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

Other than the small nit, this PR LGTM! Thank you for the patch :-)

@anchuraj anchuraj force-pushed the fix-ompx-dump-mapping-tables branch from 7d80a3f to 31cae6f Compare May 2, 2024 14:53
Copy link
Member

@skatrak skatrak left a comment

Choose a reason for hiding this comment

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

Thank you Anchu, this LGTM as well!

Named location attribute added tgt_info shall be used by runtime calls
like ompx_dump_mapping_tables to print the information of variables that
are mapped to the device. Before this change, ompx_dump_mapping_tables
was printing the wrong location information.
@anchuraj anchuraj force-pushed the fix-ompx-dump-mapping-tables branch from 31cae6f to cf8ebf7 Compare May 23, 2024 06:52
@anchuraj anchuraj merged commit 1a2a0c0 into llvm:main May 23, 2024
3 checks passed
@anchuraj anchuraj deleted the fix-ompx-dump-mapping-tables branch July 24, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category offload
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants