[OpenMP] Fix Slice Duplicate in Profiler

[jarmusch]

Summary

Using LIBOMPTARGET_PROFILER, duplicates are created from timing both Kernel functions and Data update functions.
I commented out the duplicate timescope and left them in the targetkernel and the targetdataupdate functions. This
way the timescope calls will be closer to the launching of the kernel and the data moving.

[tianshilei1992]

This doesn't look the right way to handle it. The initial design is to profile all functions in the call stack.

[jarmusch]

Thanks, @tianshilei1992 for your input.

I adjusted the timescope calls to address a problem with duplicate profiling slices. You can see them on chrome://tracing. The duplication occurred when both kernel functions and all data-moving functions were timed separately. Focusing the timescope calls on 'targetKernel' and 'targetdataupdate' functions provides a solution to the duplicate slices, and more accurate profiling data, while keeping the important profiling.

My intention was to strike a balance between resolving the duplication issue and keeping the important profiling. I understand the importance of comprehensive profiling and I'm open to further discussion.

asmooth.json161 KBDownload

(Edited - change of file - the first file was after the fix - now you see the duplicate slices)

[jdoerfert]

openmp/libomptarget/src/interface.cpp

85

Keep this, put the RegionTypeMsg into the details field of the profile entry.

131

Remove.

147–148

Remove.

161–162

Remove.

177–178

Remove.

188–189

Remove.

204–205

Remove.

247

Keep this.

297

Remove, above is better / more true to the time spend "offloading"

322–323

Remove.

openmp/libomptarget/src/omptarget.cpp

533–534

Remove, but verify all callees go through targetDataMapper<...>

575

This is not tracked anymore.

1006–1007

Remove, but verify all callees go through targetDataMapper<...>

1099–1100

Remove, but verify all callees go through targetDataMapper<...>

1144

Remove, but verify all callees go through targetDataMapper<...>

[tianshilei1992]

In D157725#4585647, @jarmusch wrote:

Thanks, @tianshilei1992 for your input.

I adjusted the timescope calls to address a problem with duplicate profiling slices. You can see them on chrome://tracing. The duplication occurred when both kernel functions and all data-moving functions were timed separately. Focusing the timescope calls on 'targetKernel' and 'targetdataupdate' functions provides a solution to the duplicate slices, and more accurate profiling data, while keeping the important profiling.

My intention was to strike a balance between resolving the duplication issue and keeping the important profiling. I understand the importance of comprehensive profiling and I'm open to further discussion.

asmooth.json161 KBDownload

(Edited - change of file - the first file was after the fix - now you see the duplicate slices)

Okay. Then don't keep commented out code.

[protze.joachim]

From my perspective, the issue is not in the measurement. Having nested time regions is a normal thing. Using the same Ident also doesn't seem to be the problem. For most nested measured regions, the visualization shows the nesting correctly. I think, the problem is in the visualization. Without verifying the regions where the nesting fails and the slices are printed into the same line, I think that chrome://tracing has an issue, if nested regions start at the same timestamp due to time resolution. An example is

{"pid":2590370,"tid":2590370,"ph":"X","ts":2394356,"dur":22893,"name":"targetKernel","args":{"detail":";main.cpp;main;60;7;;"}},
{"pid":2590370,"tid":2590370,"ph":"X","ts":2394356,"dur":22894,"name":"__tgt_target_kernel","args":{"detail":";main.cpp;main;60;7;;"}}

This should be fixed in the visualization, not in the measurement.

[jarmusch]

I addressed @jdoerfert's recommendations while the discussion is ongoing.

@protze.joachim, from your description, I understand that the issue might not lie in the
measurement itself but rather in the visualization process. That was my initial thought as
well. Moreover, based on my brief understanding of the code, I noticed that __tgt_target_kernel
always calls targetKernel, resulting in the timescope being invoked twice at practically the
same time. Is this measurement necessary, or does it create a duplication that requires
chrome://tracing to remove one for proper visualization?

[jdoerfert]

Could you share how the json looks now?

Having nested time regions is a normal thing.

Yes, but our nesting did never provide real benefit. We did not have good reason for the placement. Let's go back to minimal placement and then see where it makes sense to add nested timers.

[jarmusch]

here you go @jdoerfert

asmooth-with-profilerfix.json65 KBDownload

- from the same benchmark, asmooth-omp, with the profiler fix

[jdoerfert]

In D157725#4592558, @jarmusch wrote:

here you go @jdoerfert

asmooth-with-profilerfix.json65 KBDownload

- from the same benchmark, asmooth-omp, with the profiler fix

Looks good to me now. @tianshilei1992 @protze.joachim wdyt?

[protze.joachim]

If the region doesn't add value, removing this duplicate seems reasonable. I just commented on the technical tracing aspect.

[tianshilei1992]

LG now

[jarmusch]

edited function messages with more information for profiler

[jarmusch]

Please, land the patch for me. Thanks

[jplehr]

This seems to haven broken the AMPGPU buildbot
https://lab.llvm.org/buildbot/#/builders/193/builds/37266

FAILED: openmp/libomptarget/src/CMakeFiles/omptarget.dir/interface.cpp.o 
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang++ --target=x86_64-unknown-linux-gnu -DGTEST_HAS_RTTI=0 -DOMPT_SUPPORT=1 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LIBCPP_ENABLE_HARDENED_MODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -Wcast-qual -Wformat-pedantic -Wimplicit-fallthrough -Wsign-compare -Wno-enum-constexpr-conversion -Wno-extra -Wno-pedantic -O3 -DNDEBUG -fPIC  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT openmp/libomptarget/src/CMakeFiles/omptarget.dir/interface.cpp.o -MF openmp/libomptarget/src/CMakeFiles/omptarget.dir/interface.cpp.o.d -o openmp/libomptarget/src/CMakeFiles/omptarget.dir/interface.cpp.o -c /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:85:3: error: invalid operands to binary expression ('const char *' and 'const char *')
   85 |   TIMESCOPE_WITH_RTM_AND_IDENT(RegionTypeMsg, Loc);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/private.h:434:72: note: expanded from macro 'TIMESCOPE_WITH_RTM_AND_IDENT'
  434 |   llvm::TimeTraceScope TimeScope(__FUNCTION__, SI.getProfileLocation() + RegionTypeMsg)
      |                                                ~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:196:7: error: unknown type name 'Loc'
  196 |       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
      |       ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:196:12: error: unknown type name 'DeviceId'; did you mean 'DeviceTy'?
  196 |       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
      |            ^~~~~~~~
      |            DeviceTy
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/include/device.h:398:8: note: 'DeviceTy' declared here
  398 | struct DeviceTy {
      |        ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:196:22: error: unknown type name 'ArgNum'
  196 |       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
      |                      ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:196:30: error: unknown type name 'ArgsBase'
  196 |       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
      |                              ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:196:40: error: unknown type name 'Args'
  196 |       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
      |                                        ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:196:46: error: unknown type name 'ArgSizes'
  196 |       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
      |                                              ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:196:56: error: unknown type name 'ArgTypes'
  196 |       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
      |                                                        ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:196:66: error: unknown type name 'ArgNames'
  196 |       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
      |                                                                  ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:197:7: error: unknown type name 'ArgMappers'
  197 |       ArgMappers, targetDataUpdate, "Updating data within the OpenMP data region with update_mapper", "update");
      |       ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:197:19: error: unknown type name 'targetDataUpdate'
  197 |       ArgMappers, targetDataUpdate, "Updating data within the OpenMP data region with update_mapper", "update");
      |                   ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:197:37: error: expected parameter declarator
  197 |       ArgMappers, targetDataUpdate, "Updating data within the OpenMP data region with update_mapper", "update");
      |                                     ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:197:37: error: expected ')'
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:195:32: note: to match this '('
  195 |   targetDataMapper<AsyncInfoTy>(
      |                                ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:195:3: error: parameter name cannot have template arguments
  195 |   targetDataMapper<AsyncInfoTy>(
      |   ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:195:3: error: a type specifier is required for all declarations
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:197:112: error: expected ')'
  197 |       ArgMappers, targetDataUpdate, "Updating data within the OpenMP data region with update_mapper", "update");
      |                                                                                                                ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:189:44: note: to match this '('
  189 | EXTERN void __tgt_target_data_update_mapper(ident_t *Loc, int64_t DeviceId,
      |                                            ^
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/libomptarget/src/interface.cpp:198:1: error: extraneous closing brace ('}')
  198 | }
      | ^
17 errors generated.

[jarmusch]

@jplehr thanks, I fixed the broken commit

fixed with this commit - 1ff0bdb86dbfb7c70bbbabcae3affff59a0e0806

[jplehr]

Thanks for the timely fix! Build is now working.
Now there seem to be three test failures: https://lab.llvm.org/buildbot/#/builders/193/builds/37270

[jarmusch]

it will take some time to look into the errors, lyk as soon as possible

[jplehr]

Depending on how much time you need, it may be better to revert the patch in the meantime.

Let me know if I can assist for the failing AMD case.

[jdoerfert]

I'll fix it.

openmp/libomptarget/src/interface.cpp

 targetDataMapper(ident_t *Loc, int64_t DeviceId, int32_t ArgNum,
                  void **ArgsBase, void **Args, int64_t *ArgSizes,
                  int64_t *ArgTypes, map_var_info_t *ArgNames, void **ArgMappers,
                  TargetDataFuncPtrTy TargetDataFunction,
                  const char *RegionTypeMsg, const char *RegionName) {
   static_assert(std::is_convertible_v<TargetAsyncInfoTy, AsyncInfoTy>,
                 "TargetAsyncInfoTy must be convertible to AsyncInfoTy.");
 
-  // TIMESCOPE_WITH_IDENT(Loc);
+  TIMESCOPE_WITH_IDENT(Loc);
 
   DP("Entering data %s region for device %" PRId64 " with %d mappings\n",
      RegionName, DeviceId, ArgNum);
 
   if (checkDeviceAndCtors(DeviceId, Loc)) {
     DP("Not offloading to device %" PRId64 "\n", DeviceId);
     return;
   }

 /// libomptarget.so internal structure (an entry in a stack of data maps)
 /// and passes the data to the device.
 EXTERN void __tgt_target_data_begin_mapper(ident_t *Loc, int64_t DeviceId,
                                            int32_t ArgNum, void **ArgsBase,
                                            void **Args, int64_t *ArgSizes,
                                            int64_t *ArgTypes,
                                            map_var_info_t *ArgNames,
                                            void **ArgMappers) {
   // TIMESCOPE_WITH_IDENT(Loc);
   /// RAII to establish tool anchors before and after data begin
   OMPT_IF_BUILT(InterfaceRAII TargetDataEnterRAII(
                     RegionInterface.getCallbacks<ompt_target_enter_data>(),
                     DeviceId,
                     /* CodePtr */ OMPT_GET_RETURN_ADDRESS(0));)
 
   targetDataMapper<AsyncInfoTy>(Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes,
                                 ArgTypes, ArgNames, ArgMappers, targetDataBegin,
                                 "Entering OpenMP data region", "begin");
 }
 
 EXTERN void __tgt_target_data_begin_nowait_mapper(
     ident_t *Loc, int64_t DeviceId, int32_t ArgNum, void **ArgsBase,
     void **Args, int64_t *ArgSizes, int64_t *ArgTypes, map_var_info_t *ArgNames,
     void **ArgMappers, int32_t DepNum, void *DepList, int32_t NoAliasDepNum,
     void *NoAliasDepList) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   targetDataMapper<TaskAsyncInfoWrapperTy>(
       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
       ArgMappers, targetDataBegin, "Entering OpenMP data region", "begin");
 }
 
 /// passes data from the target, releases target memory and destroys
 /// the host-target mapping (top entry from the stack of data maps)
 /// created by the last __tgt_target_data_begin.
 EXTERN void __tgt_target_data_end_mapper(ident_t *Loc, int64_t DeviceId,
                                          int32_t ArgNum, void **ArgsBase,
                                          void **Args, int64_t *ArgSizes,
                                          int64_t *ArgTypes,
                                          map_var_info_t *ArgNames,
                                          void **ArgMappers) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   /// RAII to establish tool anchors before and after data end
   OMPT_IF_BUILT(InterfaceRAII TargetDataExitRAII(
                     RegionInterface.getCallbacks<ompt_target_exit_data>(),
                     DeviceId,
                     /* CodePtr */ OMPT_GET_RETURN_ADDRESS(0));)
 
   targetDataMapper<AsyncInfoTy>(Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes,
                                 ArgTypes, ArgNames, ArgMappers, targetDataEnd,
                                 "Exiting OpenMP data region", "end");
 }
 
 EXTERN void __tgt_target_data_end_nowait_mapper(
     ident_t *Loc, int64_t DeviceId, int32_t ArgNum, void **ArgsBase,
     void **Args, int64_t *ArgSizes, int64_t *ArgTypes, map_var_info_t *ArgNames,
     void **ArgMappers, int32_t DepNum, void *DepList, int32_t NoAliasDepNum,
     void *NoAliasDepList) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   targetDataMapper<TaskAsyncInfoWrapperTy>(
       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
       ArgMappers, targetDataEnd, "Exiting OpenMP data region", "end");
 }
 
 EXTERN void __tgt_target_data_update_mapper(ident_t *Loc, int64_t DeviceId,
                                             int32_t ArgNum, void **ArgsBase,
                                             void **Args, int64_t *ArgSizes,
                                             int64_t *ArgTypes,
                                             map_var_info_t *ArgNames,
                                             void **ArgMappers) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   /// RAII to establish tool anchors before and after data update
   OMPT_IF_BUILT(InterfaceRAII TargetDataUpdateRAII(
                     RegionInterface.getCallbacks<ompt_target_update>(),
                     DeviceId,
                     /* CodePtr */ OMPT_GET_RETURN_ADDRESS(0));)
 
   targetDataMapper<AsyncInfoTy>(
       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
       ArgMappers, targetDataUpdate, "Updating OpenMP data", "update");
 }
 
 EXTERN void __tgt_target_data_update_nowait_mapper(
     ident_t *Loc, int64_t DeviceId, int32_t ArgNum, void **ArgsBase,
     void **Args, int64_t *ArgSizes, int64_t *ArgTypes, map_var_info_t *ArgNames,
     void **ArgMappers, int32_t DepNum, void *DepList, int32_t NoAliasDepNum,
     void *NoAliasDepList) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   targetDataMapper<TaskAsyncInfoWrapperTy>(
       Loc, DeviceId, ArgNum, ArgsBase, Args, ArgSizes, ArgTypes, ArgNames,
       ArgMappers, targetDataUpdate, "Updating OpenMP data", "update");
 }
 
 static KernelArgsTy *upgradeKernelArgs(KernelArgsTy *KernelArgs,
                                        KernelArgsTy &LocalKernelArgs,
                                        int32_t NumTeams, int32_t ThreadLimit) {
   if (KernelArgs->Version > 2)

 
 template <typename TargetAsyncInfoTy>
 static inline int targetKernel(ident_t *Loc, int64_t DeviceId, int32_t NumTeams,
                                int32_t ThreadLimit, void *HostPtr,
                                KernelArgsTy *KernelArgs) {
   static_assert(std::is_convertible_v<TargetAsyncInfoTy, AsyncInfoTy>,
                 "Target AsyncInfoTy must be convertible to AsyncInfoTy.");
 
-  // TIMESCOPE_WITH_IDENT(Loc);
+  TIMESCOPE_WITH_IDENT(Loc);
 
   DP("Entering target region for device %" PRId64 " with entry point " DPxMOD
      "\n",
      DeviceId, DPxPTR(HostPtr));
 
   if (checkDeviceAndCtors(DeviceId, Loc)) {
     DP("Not offloading to device %" PRId64 "\n", DeviceId);
     return OMP_TGT_FAIL;
#endif
 
   DeviceTy &Device = *PM->Devices[DeviceId];
   TargetAsyncInfoTy TargetAsyncInfo(Device);
   AsyncInfoTy &AsyncInfo = TargetAsyncInfo;
   OMPT_IF_BUILT(InterfaceRAII TargetDataAllocRAII(
                     RegionInterface.getCallbacks<ompt_target>(), DeviceId,
                     /* CodePtr */ OMPT_GET_RETURN_ADDRESS(0));)
 
-  TIMESCOPE_WITH_IDENT(Loc);
-
   int Rc = OFFLOAD_SUCCESS;
   Rc = target(Loc, Device, HostPtr, *KernelArgs, AsyncInfo);
 
   if (Rc == OFFLOAD_SUCCESS)
     Rc = AsyncInfo.synchronize();
 
   handleTargetOutcome(Rc == OFFLOAD_SUCCESS, Loc);
   assert(Rc == OFFLOAD_SUCCESS && "__tgt_target_kernel unexpected failure!");
 
   return OMP_TGT_SUCCESS;
 }
 
 /// Implements a kernel entry that executes the target region on the specified
 /// device.
 ///
 /// \param Loc Source location associated with this target region.
 /// \param DeviceId The device to execute this region, -1 indicated the default.
 /// \param NumTeams Number of teams to launch the region with, -1 indicates a
 ///                 non-teams region and 0 indicates it was unspecified.
 /// \param ThreadLimit Limit to the number of threads to use in the kernel
 ///                    launch, 0 indicates it was unspecified.
 /// \param HostPtr  The pointer to the host function registered with the kernel.
 /// \param Args     All arguments to this kernel launch (see struct definition).
 EXTERN int __tgt_target_kernel(ident_t *Loc, int64_t DeviceId, int32_t NumTeams,
                                int32_t ThreadLimit, void *HostPtr,
                                KernelArgsTy *KernelArgs) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   if (KernelArgs->Flags.NoWait)
     return targetKernel<TaskAsyncInfoWrapperTy>(
         Loc, DeviceId, NumTeams, ThreadLimit, HostPtr, KernelArgs);
   else
     return targetKernel<AsyncInfoTy>(Loc, DeviceId, NumTeams, ThreadLimit,
                                      HostPtr, KernelArgs);
 }
 
 /// Activates the record replay mechanism.

openmp/libomptarget/src/omptarget.cpp

void targetUnlockExplicit(void *HostPtr, int DeviceNum, const char *Name) {
   DP("%s returns\n", Name);
 }
 
 /// Call the user-defined mapper function followed by the appropriate
 // targetData* function (targetData{Begin,End,Update}).
 int targetDataMapper(ident_t *Loc, DeviceTy &Device, void *ArgBase, void *Arg,
                      int64_t ArgSize, int64_t ArgType, map_var_info_t ArgNames,
                      void *ArgMapper, AsyncInfoTy &AsyncInfo,
                      TargetDataFuncPtrTy TargetDataFunction) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   DP("Calling the mapper function " DPxMOD "\n", DPxPTR(ArgMapper));
 
   // The mapper function fills up Components.
   MapperComponentsTy MapperComponents;
   MapperFuncPtrTy MapperFuncPtr = (MapperFuncPtrTy)(ArgMapper);
   (*MapperFuncPtr)((void *)&MapperComponents, ArgBase, Arg, ArgSize, ArgType,
                    ArgNames);
 
   // Construct new arrays for args_base, args, arg_sizes and arg_types
int targetDataMapper(ident_t *Loc, DeviceTy &Device, void *ArgBase, void *Arg,
   return Rc;
 }
 
 /// Internal function to do the mapping and transfer the data to the device
 int targetDataBegin(ident_t *Loc, DeviceTy &Device, int32_t ArgNum,
                     void **ArgsBase, void **Args, int64_t *ArgSizes,
                     int64_t *ArgTypes, map_var_info_t *ArgNames,
                     void **ArgMappers, AsyncInfoTy &AsyncInfo,
                     bool FromMapper) {
+  
+  TIMESCOPE_WITH_IDENT(Loc);
+
   // process each input.
   for (int32_t I = 0; I < ArgNum; ++I) {
     // Ignore private variables and arrays - there is no mapping for them.
     if ((ArgTypes[I] & OMP_TGT_MAPTYPE_LITERAL) ||
         (ArgTypes[I] & OMP_TGT_MAPTYPE_PRIVATE))
       continue;
 
     if (ArgMappers && ArgMappers[I]) {
AsyncInfo.addPostProcessingFunction([=, Device = &Device]() mutable -> int {
     return postProcessingTargetDataEnd(Device, *PostProcessingPtrs);
   });
 
   return Ret;
 }
 
 static int targetDataContiguous(ident_t *Loc, DeviceTy &Device, void *ArgsBase,
                                 void *HstPtrBegin, int64_t ArgSize,
                                 int64_t ArgType, AsyncInfoTy &AsyncInfo) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   TargetPointerResultTy TPR =
       Device.getTgtPtrBegin(HstPtrBegin, ArgSize, /*UpdateRefCount=*/false,
                             /*UseHoldRefCount=*/false, /*MustContain=*/true);
   void *TgtPtrBegin = TPR.TargetPointer;
   if (!TPR.isPresent()) {
     DP("hst data:" DPxMOD " not found, becomes a noop\n", DPxPTR(HstPtrBegin));
     if (ArgType & OMP_TGT_MAPTYPE_PRESENT) {
       MESSAGE("device mapping required by 'present' motion modifier does not "
               "exist for host address " DPxMOD " (%" PRId64 " bytes)",
static int targetDataContiguous(ident_t *Loc, DeviceTy &Device, void *ArgsBase,
   return OFFLOAD_SUCCESS;
 }
 
 static int targetDataNonContiguous(ident_t *Loc, DeviceTy &Device,
                                    void *ArgsBase,
                                    __tgt_target_non_contig *NonContig,
                                    uint64_t Size, int64_t ArgType,
                                    int CurrentDim, int DimSize, uint64_t Offset,
                                    AsyncInfoTy &AsyncInfo) {
-  // TIMESCOPE_WITH_IDENT(Loc);
   int Ret = OFFLOAD_SUCCESS;
   if (CurrentDim < DimSize) {
     for (unsigned int I = 0; I < NonContig[CurrentDim].Count; ++I) {
       uint64_t CurOffset =
           (NonContig[CurrentDim].Offset + I) * NonContig[CurrentDim].Stride;
       // we only need to transfer the first element for the last dimension
       // since we've already got a contiguous piece.
       if (CurrentDim != DimSize - 1 || I == 0) {
         Ret = targetDataNonContiguous(Loc, Device, ArgsBase, NonContig, Size,
static int getNonContigMergedDimension(__tgt_target_non_contig *NonContig,
   return RemovedDim;
 }
 
 /// Internal function to pass data to/from the target.
 int targetDataUpdate(ident_t *Loc, DeviceTy &Device, int32_t ArgNum,
                      void **ArgsBase, void **Args, int64_t *ArgSizes,
                      int64_t *ArgTypes, map_var_info_t *ArgNames,
                      void **ArgMappers, AsyncInfoTy &AsyncInfo, bool) {
-  TIMESCOPE_WITH_IDENT(Loc);
   // process each input.
   for (int32_t I = 0; I < ArgNum; ++I) {
     if ((ArgTypes[I] & OMP_TGT_MAPTYPE_LITERAL) ||
         (ArgTypes[I] & OMP_TGT_MAPTYPE_PRIVATE))
       continue;
 
     if (ArgMappers && ArgMappers[I]) {
       // Instead of executing the regular path of targetDataUpdate, call the
       // targetDataMapper variant which will call targetDataUpdate again
       // with new arguments.