[Openmp-commits] [PATCH] D96429: [OpenMP][NFC] Pass a DeviceTy, not the device number to `target`

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

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



================
Comment at: openmp/libomptarget/src/omptarget.cpp:18
 
+#include <bits/stdint-intn.h>
 #include <cassert>
----------------
tianshilei1992 wrote:
> A bit of noise here
^looks like an overenthusiastic editor. stdint.h?


================
Comment at: openmp/libomptarget/src/omptarget.cpp:1245
            int32_t ThreadLimit, int IsTeamConstruct) {
-  DeviceTy &Device = PM->Devices[DeviceId];
+  int32_t DeviceId = Device.DeviceID;
 
----------------
Changes from int64 to int32 here. Either is presumably fine, probably good to use the same width across the library.


================
Comment at: openmp/libomptarget/src/private.h:16
 
+#include "device.h"
 #include <Debug.h>
----------------
tianshilei1992 wrote:
> We could probably use forward declaration to avoid an include.
Yes, but let's optimise for the compiler finding our mistakes (via include) instead of for compile time for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96429