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

Wed Jun 1 21:56:58 PDT 2022

JonChesterfield added a comment.

I think this is reasonable, assuming that's a feature we want from this tool. Not keen on the interleaving of HSA calls and printf as it makes it relatively difficult to read but the overall complexity is still low. Will leave it open for other comments but intend to accept.

================
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,
----------------
Several of these fields are unused, not sure it's good to add them

================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:555

+  void printDeviceInfo(int32_t device_id) {
+    auto agent = HSAAgents[device_id];
----------------
Can this be static or a free function? Lots of state in it but I'm hopeful it doesn't mutate anything

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126836