[Openmp-commits] [PATCH] D115966: [openmp][libomptarget][nfc Infer types instead of duplicate and cast void

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

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.



================
Comment at: openmp/libomptarget/include/dlwrap.h:74
 // so that the function pointer call can be wrapped in library-specific code
+//
+// DLWRAP_INITIALIZE() declares static functions:
----------------
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.


================
Comment at: openmp/libomptarget/include/omptargetplugin.h:17
 
-#include <omptarget.h>
+#include "omptarget.h"
 
----------------
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


================
Comment at: openmp/libomptarget/src/device.cpp:500
                             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,
----------------
This (and three similar) pointers don't follow the pattern of plugin API with __tgt_rtl prefix removed, renamed them.


================
Comment at: openmp/libomptarget/src/rtl.h:28
 struct RTLInfoTy {
-  typedef int32_t(is_valid_binary_ty)(void *);
-  typedef int32_t(is_data_exchangable_ty)(int32_t, int32_t);
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115966/new/

https://reviews.llvm.org/D115966