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

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added inline comments.


================
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,
----------------
josemonsalve2 wrote:
> 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
I guess that was unclear, I meant for all the enums. E.g. a brief manual search suggests HSA_REGION_INFO_RUNTIME_ALLOC_ALLOWED is dead. Would suggest commenting out all of the new ones then recompiling as a crude but quick way to produce the minimal set.

I could probably be persuaded we can have dead code here that happens to match the downstream hsa.h as it's supposed to be a stable interface but I have vague fears of the numbers diverging between the two and confusion resulting. Perhaps the right thing to do is create a test case that checks this file against a hsa.h and errors if they diverge.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126836