[OpenMP][NFC] Pass a DeviceTy, not the device number to `target`

[jdoerfert]

Summary

This unifies the API of target relative to targetUpdateData and
such.

[tianshilei1992]

LG. Some nits.

openmp/libomptarget/src/omptarget.cpp

18

A bit of noise here

openmp/libomptarget/src/private.h

16

We could probably use forward declaration to avoid an include.

[jdoerfert]

Remove include

[JonChesterfield]

Thanks for this. A general trend towards passing structs instead of int and void* is a good thing.

openmp/libomptarget/src/omptarget.cpp

18

^looks like an overenthusiastic editor. stdint.h?

1244

Changes from int64 to int32 here. Either is presumably fine, probably good to use the same width across the library.

openmp/libomptarget/src/private.h

16

Yes, but let's optimise for the compiler finding our mistakes (via include) instead of for compile time for now.

[jdoerfert]

openmp/libomptarget/src/omptarget.cpp

18

already gone

1244

int32 is the type of the DeviceID member ...

[grokos]

openmp/libomptarget/src/omptarget.cpp

1244

Originally, the device ID argument of __tgt_* functions was a 32-bit integer. At some point it was extended to 64 bits in order to store more information like sub-device ID etc.. Internally, however, the library still uses the 32-bit representation. This is not an issue right now because none of the extra information is generated by clang or used by libomptarget yet, but certainly something to watch out for in the future. Nothing to be addressed in the scope of this patch though.

[jdoerfert]

openmp/libomptarget/src/omptarget.cpp

1244

We have quite a few of those mismatches we can clean up as we go. As u said, shouldn't break anything right now.

openmp/libomptarget/src/interface.cpp

#ifdef OMPTARGET_DEBUG
   for (int i = 0; i < arg_num; ++i) {
     DP("Entry %2d: Base=" DPxMOD ", Begin=" DPxMOD ", Size=%" PRId64
        ", Type=0x%" PRIx64 ", Name=%s\n",
        i, DPxPTR(args_base[i]), DPxPTR(args[i]), arg_sizes[i], arg_types[i],
        (arg_names) ? getNameFromMapping(arg_names[i]).c_str() : "unknown");
   }
 #endif
 
-  int rc = target(loc, device_id, host_ptr, arg_num, args_base, args, arg_sizes,
+  DeviceTy &Device = PM->Devices[device_id];
+  int rc = target(loc, Device, host_ptr, arg_num, args_base, args, arg_sizes,
                   arg_types, arg_names, arg_mappers, 0, 0, false /*team*/);
   HandleTargetOutcome(rc == OFFLOAD_SUCCESS, loc);
   return rc;
 }
 
 EXTERN int __tgt_target_nowait_mapper(
     ident_t *loc, int64_t device_id, void *host_ptr, int32_t arg_num,
     void **args_base, void **args, int64_t *arg_sizes, int64_t *arg_types,
#ifdef OMPTARGET_DEBUG
   for (int i = 0; i < arg_num; ++i) {
     DP("Entry %2d: Base=" DPxMOD ", Begin=" DPxMOD ", Size=%" PRId64
        ", Type=0x%" PRIx64 ", Name=%s\n",
        i, DPxPTR(args_base[i]), DPxPTR(args[i]), arg_sizes[i], arg_types[i],
        (arg_names) ? getNameFromMapping(arg_names[i]).c_str() : "unknown");
   }
 #endif
 
-  int rc = target(loc, device_id, host_ptr, arg_num, args_base, args, arg_sizes,
+  DeviceTy &Device = PM->Devices[device_id];
+  int rc = target(loc, Device, host_ptr, arg_num, args_base, args, arg_sizes,
                   arg_types, arg_names, arg_mappers, team_num, thread_limit,
                   true /*team*/);
   HandleTargetOutcome(rc == OFFLOAD_SUCCESS, loc);
   return rc;
 }
 
 EXTERN int __tgt_target_teams_nowait_mapper(
     ident_t *loc, int64_t device_id, void *host_ptr, int32_t arg_num,

openmp/libomptarget/src/omptarget.cpp

 // target region.
 //
 //===----------------------------------------------------------------------===//
 
 #include "device.h"
 #include "private.h"
 #include "rtl.h"
 
 #include <cassert>
 #include <vector>
 
 /* All begin addresses for partially mapped structs must be 8-aligned in order
  * to ensure proper alignment of members. E.g.
  *
  * struct S {
  *   int a;   // 4-aligned
  *   int b;   // 4-aligned
static int InitLibrary(DeviceTy &Device) {
    */
   if (!Device.PendingCtorsDtors.empty()) {
     // Call all ctors for all libraries registered so far
     for (auto &lib : Device.PendingCtorsDtors) {
       if (!lib.second.PendingCtors.empty()) {
         DP("Has pending ctors... call now\n");
         for (auto &entry : lib.second.PendingCtors) {
           void *ctor = entry;
-          int rc =
-              target(nullptr, device_id, ctor, 0, nullptr, nullptr, nullptr,
+          int rc = target(nullptr, Device, ctor, 0, nullptr, nullptr, nullptr,
                           nullptr, nullptr, nullptr, 1, 1, true /*team*/);
           if (rc != OFFLOAD_SUCCESS) {
             REPORT("Running ctor " DPxMOD " failed.\n", DPxPTR(ctor));
             Device.PendingGlobalsMtx.unlock();
             return OFFLOAD_FAIL;
           }
         }
         // Clear the list to indicate that this device has been used
         lib.second.PendingCtors.clear();

 } // namespace
 
 /// performs the same actions as data_begin in case arg_num is
 /// non-zero and initiates run of the offloaded region on the target platform;
 /// if arg_num is non-zero after the region execution is done it also
 /// performs the same action as data_update and data_end above. This function
 /// returns 0 if it was able to transfer the execution to a target and an
 /// integer different from zero otherwise.
-int target(ident_t *loc, int64_t DeviceId, void *HostPtr, int32_t ArgNum,
+int target(ident_t *loc, DeviceTy &Device, void *HostPtr, int32_t ArgNum,
            void **ArgBases, void **Args, int64_t *ArgSizes, int64_t *ArgTypes,
            map_var_info_t *ArgNames, void **ArgMappers, int32_t TeamNum,
            int32_t ThreadLimit, int IsTeamConstruct) {
-  DeviceTy &Device = PM->Devices[DeviceId];
+  int32_t DeviceId = Device.DeviceID;
 
   TableMap *TM = getTableMap(HostPtr);
   // No map for this host pointer found!
   if (!TM) {
     REPORT("Host ptr " DPxMOD " does not have a matching target pointer.\n",
            DPxPTR(HostPtr));
     return OFFLOAD_FAIL;
   }

openmp/libomptarget/src/private.h

 //===---------- private.h - Target independent OpenMP target RTL ----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Private function declarations and helper macros for debugging output.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _OMPTARGET_PRIVATE_H
 #define _OMPTARGET_PRIVATE_H
 
+#include "device.h"
 #include <Debug.h>
 #include <SourceInfo.h>
 #include <omptarget.h>
 
 #include <cstdint>
 
 extern int targetDataBegin(ident_t *loc, DeviceTy &Device, int32_t arg_num,
                            void **args_base, void **args, int64_t *arg_sizes,
                            int64_t *arg_types, map_var_info_t *arg_names,
                            void **arg_mappers,
                            __tgt_async_info *async_info_ptr);
 
 extern int targetDataEnd(ident_t *loc, DeviceTy &Device, int32_t ArgNum,
                          void **ArgBases, void **Args, int64_t *ArgSizes,
                          int64_t *ArgTypes, map_var_info_t *arg_names,
                          void **ArgMappers, __tgt_async_info *AsyncInfo);
 
 extern int targetDataUpdate(ident_t *loc, DeviceTy &Device, int32_t arg_num,
                             void **args_base, void **args, int64_t *arg_sizes,
                             int64_t *arg_types, map_var_info_t *arg_names,
                             void **arg_mappers,
                             __tgt_async_info *async_info_ptr = nullptr);
 
-extern int target(ident_t *loc, int64_t DeviceId, void *HostPtr, int32_t ArgNum,
+extern int target(ident_t *loc, DeviceTy &Device, void *HostPtr, int32_t ArgNum,
                   void **ArgBases, void **Args, int64_t *ArgSizes,
                   int64_t *ArgTypes, map_var_info_t *arg_names,
                   void **ArgMappers, int32_t TeamNum, int32_t ThreadLimit,
                   int IsTeamConstruct);
 
 extern int CheckDeviceAndCtors(int64_t device_id);
 
 // This structure stores information of a mapped memory region.

openmp/libomptarget/src/rtl.cpp

for (auto *R : UsedRTLs) {
 
       // Execute dtors for static objects if the device has been used, i.e.
       // if its PendingCtors list has been emptied.
       for (int32_t i = 0; i < FoundRTL->NumberOfDevices; ++i) {
         DeviceTy &Device = PM->Devices[FoundRTL->Idx + i];
         Device.PendingGlobalsMtx.lock();
         if (Device.PendingCtorsDtors[desc].PendingCtors.empty()) {
           for (auto &dtor : Device.PendingCtorsDtors[desc].PendingDtors) {
-            int rc =
-                target(nullptr, Device.DeviceID, dtor, 0, nullptr, nullptr,
-                       nullptr, nullptr, nullptr, nullptr, 1, 1, true /*team*/);
+            int rc = target(nullptr, Device, dtor, 0, nullptr, nullptr, nullptr,
+                            nullptr, nullptr, nullptr, 1, 1, true /*team*/);
             if (rc != OFFLOAD_SUCCESS) {
               DP("Running destructor " DPxMOD " failed.\n", DPxPTR(dtor));
             }
           }
           // Remove this library's entry from PendingCtorsDtors
           Device.PendingCtorsDtors.erase(desc);
         }
         Device.PendingGlobalsMtx.unlock();