[openmp][libomptarget][nfc] Infer types instead of duplicate and cast void

[JonChesterfield]

Summary

Replaces handwritten typedefs and casts via void* with types inferred
from declarations in omptargetplugins.h. Leaves unchanged a function with the
wrong type and register/unregister which follow a different pattern.

Rename a couple of pointer variables to match the api (missing _target_
Minor change made to dlwrap.h to use the type inference from it.

[JonChesterfield]

There's more that can be done here in terms of generating the wrappers from the header but this gets rid of the handwritten typedefs without disturbing much else.

openmp/libomptarget/include/dlwrap.h

74 ↗

(On Diff #395196)

Without this or similar, including dlwrap.h from a file that doesn't use all the pieces errors about declared but missing static functions. I'm considering moving the non-dlwrap parts into a separate header but leaving that for a later patch.

openmp/libomptarget/include/omptargetplugin.h

17

Probably harmless, but would like to use <> or "" consistently when referring to this. rtl.h uses "". Don't really want this header to find an omptarget from the system if it's locally installed so "" seems safer

openmp/libomptarget/src/device.cpp

500–502

This (and three similar) pointers don't follow the pattern of plugin API with __tgt_rtl prefix removed, renamed them.

openmp/libomptarget/src/rtl.h

28

Whole patch is a reaction to discovering this sequence of typedefs in rtl.h. Way too easy to get this wrong and no need to type them out by hand.

[JonChesterfield]

openmp/libomptarget/include/dlwrap.h

74 ↗

(On Diff #395196)

I think the change to dlwrap is good in it's own right, we shouldn't have a header that is an error to include unless some macro in it is invoked and the INITIALIZE() call may be useful anyway. Got some other patches backing up on this so will land that separate to the libomptarget implementation change

[JonChesterfield]

• rebase

[tianshilei1992]

openmp/libomptarget/src/device.cpp

500–502

worth doing it in another patch

openmp/libomptarget/src/rtl.h

83

load_binary is left.

86

I think these changes also worth a separate patch.

openmp/libomptarget/include/omptargetplugin.h

 //===-- omptargetplugin.h - Target dependent OpenMP Plugin API --*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines an interface between target independent OpenMP offload
 // runtime library libomptarget and target dependent plugin.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _OMPTARGETPLUGIN_H_
 #define _OMPTARGETPLUGIN_H_
 
-#include <omptarget.h>
+#include "omptarget.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 // Return the number of available devices of the type supported by the
 // target RTL.
 int32_t __tgt_rtl_number_of_devices(void);

openmp/libomptarget/src/device.cpp

if (!AsyncInfo || !RTL->data_exchange_async || !RTL->synchronize) {
     return RTL->data_exchange_async(RTLDeviceID, SrcPtr, DstDev.RTLDeviceID,
                                     DstPtr, Size, AsyncInfo);
 }
 
 // Run region on device
 int32_t DeviceTy::runRegion(void *TgtEntryPtr, void **TgtVarsPtr,
                             ptrdiff_t *TgtOffsets, int32_t TgtVarsSize,
                             AsyncInfoTy &AsyncInfo) {
-  if (!RTL->run_region || !RTL->synchronize)
-    return RTL->run_region(RTLDeviceID, TgtEntryPtr, TgtVarsPtr, TgtOffsets,
-                           TgtVarsSize);
+  if (!RTL->run_target_region || !RTL->synchronize)
+    return RTL->run_target_region(RTLDeviceID, TgtEntryPtr, TgtVarsPtr,
+                                  TgtOffsets, TgtVarsSize);
   else
-    return RTL->run_region_async(RTLDeviceID, TgtEntryPtr, TgtVarsPtr,
+    return RTL->run_target_region_async(RTLDeviceID, TgtEntryPtr, TgtVarsPtr,
                                         TgtOffsets, TgtVarsSize, AsyncInfo);
 }
 
 // Run region on device
 bool DeviceTy::printDeviceInfo(int32_t RTLDevId) {
   if (!RTL->print_device_info)
     return false;
   RTL->print_device_info(RTLDevId);
   return true;
 }
 
 // Run team region on device.
 int32_t DeviceTy::runTeamRegion(void *TgtEntryPtr, void **TgtVarsPtr,
                                 ptrdiff_t *TgtOffsets, int32_t TgtVarsSize,
                                 int32_t NumTeams, int32_t ThreadLimit,
                                 uint64_t LoopTripCount,
                                 AsyncInfoTy &AsyncInfo) {
-  if (!RTL->run_team_region_async || !RTL->synchronize)
-    return RTL->run_team_region(RTLDeviceID, TgtEntryPtr, TgtVarsPtr,
-                                TgtOffsets, TgtVarsSize, NumTeams, ThreadLimit,
-                                LoopTripCount);
-  else
-    return RTL->run_team_region_async(RTLDeviceID, TgtEntryPtr, TgtVarsPtr,
+  if (!RTL->run_target_team_region_async || !RTL->synchronize)
+    return RTL->run_target_team_region(RTLDeviceID, TgtEntryPtr, TgtVarsPtr,
                                        TgtOffsets, TgtVarsSize, NumTeams,
-                                      ThreadLimit, LoopTripCount, AsyncInfo);
+                                       ThreadLimit, LoopTripCount);
+  else
+    return RTL->run_target_team_region_async(
+        RTLDeviceID, TgtEntryPtr, TgtVarsPtr, TgtOffsets, TgtVarsSize, NumTeams,
 }
 
 // Whether data can be copied to DstDevice directly
 bool DeviceTy::isDataExchangable(const DeviceTy &DstDevice) {
   if (RTL != DstDevice.RTL || !RTL->is_data_exchangable)
     return false;
 
   if (RTL->is_data_exchangable(RTLDeviceID, DstDevice.RTLDeviceID))

openmp/libomptarget/src/rtl.h

 //===------------ rtl.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
 //
 //===----------------------------------------------------------------------===//
 //
 // Declarations for handling RTL plugins.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _OMPTARGET_RTL_H
 #define _OMPTARGET_RTL_H
 
+#include "dlwrap.h"
 #include "omptarget.h"
+#include "omptargetplugin.h"
 #include <list>
 #include <map>
 #include <mutex>
 #include <string>
 #include <vector>
 
 // Forward declarations.
 struct DeviceTy;
 struct __tgt_bin_desc;
 
 struct RTLInfoTy {
-  typedef int32_t(is_valid_binary_ty)(void *);
-  typedef int32_t(is_data_exchangable_ty)(int32_t, int32_t);
-  typedef int32_t(number_of_devices_ty)();
-  typedef int32_t(init_device_ty)(int32_t);
+
+#ifdef RTL_MAKE_TYPEDEF
+#error "RTL_MAKE_TYPEDEF already defined"
+#endif
+#define RTL_MAKE_TYPEDEF(SYMBOL)                                               \
+  using SYMBOL##_ty = dlwrap::trait<decltype(&__tgt_rtl_##SYMBOL)>::FunctionType
+  RTL_MAKE_TYPEDEF(is_valid_binary);
+  RTL_MAKE_TYPEDEF(is_data_exchangable);
+  RTL_MAKE_TYPEDEF(number_of_devices);
+  RTL_MAKE_TYPEDEF(init_device);
   typedef __tgt_target_table *(load_binary_ty)(int32_t, void *);
-  typedef void *(data_alloc_ty)(int32_t, int64_t, void *, int32_t);
-  typedef int32_t(data_submit_ty)(int32_t, void *, void *, int64_t);
-  typedef int32_t(data_submit_async_ty)(int32_t, void *, void *, int64_t,
-                                        __tgt_async_info *);
-  typedef int32_t(data_retrieve_ty)(int32_t, void *, void *, int64_t);
-  typedef int32_t(data_retrieve_async_ty)(int32_t, void *, void *, int64_t,
-                                          __tgt_async_info *);
-  typedef int32_t(data_exchange_ty)(int32_t, void *, int32_t, void *, int64_t);
-  typedef int32_t(data_exchange_async_ty)(int32_t, void *, int32_t, void *,
-                                          int64_t, __tgt_async_info *);
-  typedef int32_t(data_delete_ty)(int32_t, void *);
-  typedef int32_t(run_region_ty)(int32_t, void *, void **, ptrdiff_t *,
-                                 int32_t);
-  typedef int32_t(run_region_async_ty)(int32_t, void *, void **, ptrdiff_t *,
-                                       int32_t, __tgt_async_info *);
-  typedef int32_t(run_team_region_ty)(int32_t, void *, void **, ptrdiff_t *,
-                                      int32_t, int32_t, int32_t, uint64_t);
-  typedef int32_t(run_team_region_async_ty)(int32_t, void *, void **,
-                                            ptrdiff_t *, int32_t, int32_t,
-                                            int32_t, uint64_t,
-                                            __tgt_async_info *);
-  typedef int64_t(init_requires_ty)(int64_t);
-  typedef int32_t(synchronize_ty)(int32_t, __tgt_async_info *);
+  RTL_MAKE_TYPEDEF(data_alloc);
+  RTL_MAKE_TYPEDEF(data_submit);
+  RTL_MAKE_TYPEDEF(data_submit_async);
+  RTL_MAKE_TYPEDEF(data_retrieve);
+  RTL_MAKE_TYPEDEF(data_retrieve_async);
+  RTL_MAKE_TYPEDEF(data_exchange);
+  RTL_MAKE_TYPEDEF(data_exchange_async);
+  RTL_MAKE_TYPEDEF(data_delete);
+  RTL_MAKE_TYPEDEF(run_target_region);
+  RTL_MAKE_TYPEDEF(run_target_region_async);
+  RTL_MAKE_TYPEDEF(run_target_team_region);
+  RTL_MAKE_TYPEDEF(run_target_team_region_async);
+  RTL_MAKE_TYPEDEF(init_requires);
+  RTL_MAKE_TYPEDEF(synchronize);
   typedef int32_t (*register_lib_ty)(__tgt_bin_desc *);
-  typedef int32_t(supports_empty_images_ty)();
-  typedef void(print_device_info_ty)(int32_t);
-  typedef void(set_info_flag_ty)(uint32_t);
-  typedef int32_t(create_event_ty)(int32_t, void **);
-  typedef int32_t(record_event_ty)(int32_t, void *, __tgt_async_info *);
-  typedef int32_t(wait_event_ty)(int32_t, void *, __tgt_async_info *);
-  typedef int32_t(sync_event_ty)(int32_t, void *);
-  typedef int32_t(destroy_event_ty)(int32_t, void *);
+  RTL_MAKE_TYPEDEF(supports_empty_images);
+  RTL_MAKE_TYPEDEF(print_device_info);
+  RTL_MAKE_TYPEDEF(set_info_flag);
+  RTL_MAKE_TYPEDEF(create_event);
+  RTL_MAKE_TYPEDEF(record_event);
+  RTL_MAKE_TYPEDEF(wait_event);
+  RTL_MAKE_TYPEDEF(sync_event);
+  RTL_MAKE_TYPEDEF(destroy_event);
+#undef RTL_MAKE_TYPEDEF
 
   int32_t Idx = -1;             // RTL index, index is the number of devices
                                 // of other RTLs that were registered before,
                                 // i.e. the OpenMP index of the first device
                                 // to be registered with this RTL.
   int32_t NumberOfDevices = -1; // Number of devices this RTL deals with.
 
   void *LibraryHandler = nullptr;
 
 #ifdef OMPTARGET_DEBUG
   std::string RTLName;
 #endif
 
   // Functions implemented in the RTL.
-  is_valid_binary_ty *is_valid_binary = nullptr;
-  is_data_exchangable_ty *is_data_exchangable = nullptr;
-  number_of_devices_ty *number_of_devices = nullptr;
-  init_device_ty *init_device = nullptr;
+  is_valid_binary_ty is_valid_binary = nullptr;
+  is_data_exchangable_ty is_data_exchangable = nullptr;
+  number_of_devices_ty number_of_devices = nullptr;
+  init_device_ty init_device = nullptr;
   load_binary_ty *load_binary = nullptr;
-  data_alloc_ty *data_alloc = nullptr;
-  data_submit_ty *data_submit = nullptr;
-  data_submit_async_ty *data_submit_async = nullptr;
-  data_retrieve_ty *data_retrieve = nullptr;
-  data_retrieve_async_ty *data_retrieve_async = nullptr;
-  data_exchange_ty *data_exchange = nullptr;
-  data_exchange_async_ty *data_exchange_async = nullptr;
-  data_delete_ty *data_delete = nullptr;
-  run_region_ty *run_region = nullptr;
-  run_region_async_ty *run_region_async = nullptr;
-  run_team_region_ty *run_team_region = nullptr;
-  run_team_region_async_ty *run_team_region_async = nullptr;
-  init_requires_ty *init_requires = nullptr;
-  synchronize_ty *synchronize = nullptr;
+  data_alloc_ty data_alloc = nullptr;
+  data_submit_ty data_submit = nullptr;
+  data_submit_async_ty data_submit_async = nullptr;
+  data_retrieve_ty data_retrieve = nullptr;
+  data_retrieve_async_ty data_retrieve_async = nullptr;
+  data_exchange_ty data_exchange = nullptr;
+  data_exchange_async_ty data_exchange_async = nullptr;
+  data_delete_ty data_delete = nullptr;
+  run_target_region_ty run_target_region = nullptr;
+  run_target_region_async_ty run_target_region_async = nullptr;
+  run_target_team_region_ty run_target_team_region = nullptr;
+  run_target_team_region_async_ty run_target_team_region_async = nullptr;
+  init_requires_ty init_requires = nullptr;
+  synchronize_ty synchronize = nullptr;
   register_lib_ty register_lib = nullptr;
   register_lib_ty unregister_lib = nullptr;
-  supports_empty_images_ty *supports_empty_images = nullptr;
-  set_info_flag_ty *set_info_flag = nullptr;
-  print_device_info_ty *print_device_info = nullptr;
-  create_event_ty *create_event = nullptr;
-  record_event_ty *record_event = nullptr;
-  wait_event_ty *wait_event = nullptr;
-  sync_event_ty *sync_event = nullptr;
-  destroy_event_ty *destroy_event = nullptr;
+  supports_empty_images_ty supports_empty_images = nullptr;
+  set_info_flag_ty set_info_flag = nullptr;
+  print_device_info_ty print_device_info = nullptr;
+  create_event_ty create_event = nullptr;
+  record_event_ty record_event = nullptr;
+  wait_event_ty wait_event = nullptr;
+  sync_event_ty sync_event = nullptr;
+  destroy_event_ty destroy_event = nullptr;
 
   // Are there images associated with this RTL.
   bool isUsed = false;
 
   // Mutex for thread-safety when calling RTL interface functions.
   // It is easier to enforce thread-safety at the libomptarget level,
   // so that developers of new RTLs do not have to worry about it.
   std::mutex Mtx;

openmp/libomptarget/src/rtl.cpp

if (ProfileTraceFile) {
     if (auto E = llvm::timeTraceProfilerWrite(ProfileTraceFile, "-"))
       fprintf(stderr, "Error writing out the time trace\n");
 
     llvm::timeTraceProfilerCleanup();
   }
 #endif
 }
 
+template <typename T> static T CastDlsym(void *Handle, const char *Symbol) {
+  // dlsym returns void*, which can be cast to a function pointer in posix
+  // but not in c++, thus static_cast is an error and we need reinterpret
+  // or (c-style)
+  return reinterpret_cast<T>(dlsym(Handle, Symbol));
+}
+#define TypedDlsym(HANDLE, SYMBOL)                                             \
+  CastDlsym<dlwrap::trait<decltype(&SYMBOL)>::FunctionType>(HANDLE, #SYMBOL)
+
 void RTLsTy::LoadRTLs() {
   // Parse environment variable OMP_TARGET_OFFLOAD (if set)
   PM->TargetOffloadPolicy =
       (kmp_target_offload_kind_t)__kmpc_get_target_offload();
   if (PM->TargetOffloadPolicy == tgt_disabled) {
     return;
   }
 
for (auto *Name : RTLNames) {
 
     AllRTLs.emplace_back();
 
     // Retrieve the RTL information from the runtime library.
     RTLInfoTy &R = AllRTLs.back();
 
     bool ValidPlugin = true;
 
-    if (!(*((void **)&R.is_valid_binary) =
-              dlsym(dynlib_handle, "__tgt_rtl_is_valid_binary")))
+    if (!(R.is_valid_binary =
+              TypedDlsym(dynlib_handle, __tgt_rtl_is_valid_binary)))
       ValidPlugin = false;
-    if (!(*((void **)&R.number_of_devices) =
-              dlsym(dynlib_handle, "__tgt_rtl_number_of_devices")))
+    if (!(R.number_of_devices =
+              TypedDlsym(dynlib_handle, __tgt_rtl_number_of_devices)))
       ValidPlugin = false;
-    if (!(*((void **)&R.init_device) =
-              dlsym(dynlib_handle, "__tgt_rtl_init_device")))
+    if (!(R.init_device = TypedDlsym(dynlib_handle, __tgt_rtl_init_device)))
       ValidPlugin = false;
     if (!(*((void **)&R.load_binary) =
               dlsym(dynlib_handle, "__tgt_rtl_load_binary")))
       ValidPlugin = false;
-    if (!(*((void **)&R.data_alloc) =
-              dlsym(dynlib_handle, "__tgt_rtl_data_alloc")))
+    if (!(R.data_alloc = TypedDlsym(dynlib_handle, __tgt_rtl_data_alloc)))
       ValidPlugin = false;
-    if (!(*((void **)&R.data_submit) =
-              dlsym(dynlib_handle, "__tgt_rtl_data_submit")))
+    if (!(R.data_submit = TypedDlsym(dynlib_handle, __tgt_rtl_data_submit)))
       ValidPlugin = false;
-    if (!(*((void **)&R.data_retrieve) =
-              dlsym(dynlib_handle, "__tgt_rtl_data_retrieve")))
+    if (!(R.data_retrieve = TypedDlsym(dynlib_handle, __tgt_rtl_data_retrieve)))
       ValidPlugin = false;
-    if (!(*((void **)&R.data_delete) =
-              dlsym(dynlib_handle, "__tgt_rtl_data_delete")))
+    if (!(R.data_delete = TypedDlsym(dynlib_handle, __tgt_rtl_data_delete)))
       ValidPlugin = false;
-    if (!(*((void **)&R.run_region) =
-              dlsym(dynlib_handle, "__tgt_rtl_run_target_region")))
+    if (!(R.run_target_region =
+              TypedDlsym(dynlib_handle, __tgt_rtl_run_target_region)))
       ValidPlugin = false;
-    if (!(*((void **)&R.run_team_region) =
-              dlsym(dynlib_handle, "__tgt_rtl_run_target_team_region")))
+    if (!(R.run_target_team_region =
+              TypedDlsym(dynlib_handle, __tgt_rtl_run_target_team_region)))
       ValidPlugin = false;
 
     // Invalid plugin
     if (!ValidPlugin) {
       DP("Invalid plugin as necessary interface is not found.\n");
       AllRTLs.pop_back();
       continue;
     }

 #ifdef OMPTARGET_DEBUG
     R.RTLName = Name;
 #endif
 
     DP("Registering RTL %s supporting %d devices!\n", R.RTLName.c_str(),
        R.NumberOfDevices);
 
     // Optional functions
-    *((void **)&R.init_requires) =
-        dlsym(dynlib_handle, "__tgt_rtl_init_requires");
-    *((void **)&R.data_submit_async) =
-        dlsym(dynlib_handle, "__tgt_rtl_data_submit_async");
-    *((void **)&R.data_retrieve_async) =
-        dlsym(dynlib_handle, "__tgt_rtl_data_retrieve_async");
-    *((void **)&R.run_region_async) =
-        dlsym(dynlib_handle, "__tgt_rtl_run_target_region_async");
-    *((void **)&R.run_team_region_async) =
-        dlsym(dynlib_handle, "__tgt_rtl_run_target_team_region_async");
-    *((void **)&R.synchronize) = dlsym(dynlib_handle, "__tgt_rtl_synchronize");
-    *((void **)&R.data_exchange) =
-        dlsym(dynlib_handle, "__tgt_rtl_data_exchange");
-    *((void **)&R.data_exchange_async) =
-        dlsym(dynlib_handle, "__tgt_rtl_data_exchange_async");
-    *((void **)&R.is_data_exchangable) =
-        dlsym(dynlib_handle, "__tgt_rtl_is_data_exchangable");
+    R.init_requires = TypedDlsym(dynlib_handle, __tgt_rtl_init_requires);
+    R.data_submit_async =
+        TypedDlsym(dynlib_handle, __tgt_rtl_data_submit_async);
+    R.data_retrieve_async =
+        TypedDlsym(dynlib_handle, __tgt_rtl_data_retrieve_async);
+    R.run_target_region_async =
+        TypedDlsym(dynlib_handle, __tgt_rtl_run_target_region_async);
+    R.run_target_team_region_async =
+        TypedDlsym(dynlib_handle, __tgt_rtl_run_target_team_region_async);
+    R.synchronize = TypedDlsym(dynlib_handle, __tgt_rtl_synchronize);
+    R.data_exchange = TypedDlsym(dynlib_handle, __tgt_rtl_data_exchange);
+    R.data_exchange_async =
+        TypedDlsym(dynlib_handle, __tgt_rtl_data_exchange_async);
+    R.is_data_exchangable =
+        TypedDlsym(dynlib_handle, __tgt_rtl_is_data_exchangable);
     *((void **)&R.register_lib) =
         dlsym(dynlib_handle, "__tgt_rtl_register_lib");
     *((void **)&R.unregister_lib) =
         dlsym(dynlib_handle, "__tgt_rtl_unregister_lib");
-    *((void **)&R.supports_empty_images) =
-        dlsym(dynlib_handle, "__tgt_rtl_supports_empty_images");
-    *((void **)&R.set_info_flag) =
-        dlsym(dynlib_handle, "__tgt_rtl_set_info_flag");
-    *((void **)&R.print_device_info) =
-        dlsym(dynlib_handle, "__tgt_rtl_print_device_info");
-    *((void **)&R.create_event) =
-        dlsym(dynlib_handle, "__tgt_rtl_create_event");
-    *((void **)&R.record_event) =
-        dlsym(dynlib_handle, "__tgt_rtl_record_event");
-    *((void **)&R.wait_event) = dlsym(dynlib_handle, "__tgt_rtl_wait_event");
-    *((void **)&R.sync_event) = dlsym(dynlib_handle, "__tgt_rtl_sync_event");
-    *((void **)&R.destroy_event) =
-        dlsym(dynlib_handle, "__tgt_rtl_destroy_event");
+
+    R.supports_empty_images =
+        TypedDlsym(dynlib_handle, __tgt_rtl_supports_empty_images);
+    R.set_info_flag = TypedDlsym(dynlib_handle, __tgt_rtl_set_info_flag);
+    R.print_device_info =
+        TypedDlsym(dynlib_handle, __tgt_rtl_print_device_info);
+    R.create_event = TypedDlsym(dynlib_handle, __tgt_rtl_create_event);
+    R.record_event = TypedDlsym(dynlib_handle, __tgt_rtl_record_event);
+    R.wait_event = TypedDlsym(dynlib_handle, __tgt_rtl_wait_event);
+    R.sync_event = TypedDlsym(dynlib_handle, __tgt_rtl_sync_event);
+    R.destroy_event = TypedDlsym(dynlib_handle, __tgt_rtl_destroy_event);
   }
 
   DP("RTLs loaded!\n");
 
   return;
 }
 
 ////////////////////////////////////////////////////////////////////////////////