[Openmp-commits] [PATCH] D69424: [NFC][libomptarget] move remaining device specific code out of omptarget-nvptx.h

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield marked an inline comment as done.
JonChesterfield added inline comments.


================
Comment at: openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h:34-37
+/// Device environment data
+struct omptarget_device_environmentTy {
+  int32_t debug_level;
+};
----------------
ABataev wrote:
> JonChesterfield wrote:
> > ABataev wrote:
> > > JonChesterfield wrote:
> > > > ABataev wrote:
> > > > > Is this really target specific thing?
> > > > This one is a driver/plugin specific thing. The hsa struct has another couple of fields.
> > > Why do you have different fields for hsa? Maybe we can use the same fields for NVPTX if they are useful?
> > The struct definition looks likely to be useful for nvptx, though the fields should probably then be wired up. Defined for gcn/hsa as:
> > 
> > ```
> > struct omptarget_device_environmentTy {
> >   int32_t debug_level;   // gets value of envvar LIBOMPTARGET_DEVICE_RTL_DEBUG
> >                          // only useful for Debug build of deviceRTLs 
> >   int32_t num_devices;   // gets number of active offload devices 
> >   int32_t device_num;    // gets a value 0 to num_devices-1
> > };
> > ```
> > 
> > where the fields are used by additional functions in libcall, e.g.
> > 
> > ```EXTERN int omp_get_num_devices(void) {
> >   PRINT(LD_IO, "call omp_get_num_devices() returns device_size %d\n",
> >         omptarget_device_environment.num_devices);
> >   return omptarget_device_environment.num_devices;
> > }
> > ```
> > 
> > Would you like omp_get_num_devices / omp_get_device_num added to the generic interface? If so I'll write the patch, and reduce this diff to exclude the struct move.
> Are they required by the standard?
I don't know. I'll find out before creating a diff which proposes said functions - in the meantime I've dropped it from this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69424