[Openmp-commits] [PATCH] D126836: [LIBOMPTARGET] Adding AMD to llvm-omp-device-info

[Jose Manuel Monsalve Diaz via Phabricator via Openmp-commits]

josemonsalve2 added a comment.

@saiislam I will look into that.

@JonChesterfield I look at the `rocm-info` code and they have different structs for all the different fields. I just didn't want to do all that, since it is not really that important for the runtime itself. However, the idea of the JSON file is good. Let's consider it and re-structure this.

One option could be to add a device info layer, with all the details, that can be access by the runtime, as well as output in text and JSON. But this should be an extension of this.



================
Comment at: openmp/libomptarget/plugins/amdgpu/dynamic_hsa/hsa_ext_amd.h:63
+  HSA_AMD_AGENT_INFO_DRIVER_NODE_ID = 0xA004,
+  HSA_AMD_AGENT_INFO_MAX_ADDRESS_WATCH_POINTS = 0xA005,
+  HSA_AMD_AGENT_INFO_BDFID = 0xA006,
----------------
JonChesterfield wrote:
> Several of these fields are unused, not sure it's good to add them
Ok, I will remove the ones that are not used


================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:555
 
+  void printDeviceInfo(int32_t device_id) {
+    auto agent = HSAAgents[device_id];
----------------
JonChesterfield wrote:
> Can this be static or a free function? Lots of state in it but I'm hopeful it doesn't mutate anything
That's reasonable. It should not mutate anything


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126836