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

Wed Feb 10 16:42:52 PST 2021

jdoerfert added inline comments.

================
Comment at: openmp/libomptarget/src/omptarget.cpp:1245
            int32_t ThreadLimit, int IsTeamConstruct) {
-  DeviceTy &Device = PM->Devices[DeviceId];
+  int32_t DeviceId = Device.DeviceID;

----------------
grokos wrote:
> jdoerfert wrote:
> > JonChesterfield wrote:
> > > Changes from int64 to int32 here. Either is presumably fine, probably good to use the same width across the library.
> > int32 is the type of the `DeviceID` member ...
> 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.
We have quite a few of those mismatches we can clean up as we go. As u said, shouldn't break anything right now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96429