[Openmp-commits] [PATCH] D108784: [libomptarget][amdgpu] Drop env variables

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added inline comments.


================
Comment at: openmp/libomptarget/plugins/amdgpu/impl/rt.h:25
+} // namespace Runtime
+hsa_status_t RegisterModuleFromMemory(
+    std::map<std::string, atl_kernel_info_t> &KernelInfoTable,
----------------
This was previously forward declared in rtl.cpp, moving it to a header to catch divergence in prototype vs implementation


================
Comment at: openmp/libomptarget/plugins/amdgpu/impl/utils.cpp:31
-
-  var = GetEnv("ATMI_MAX_HSA_QUEUE_SIZE");
-  if (!var.empty())
----------------
Potentially contentious point is here. Previously we let stoi(ATMI_MAX_HSA_QUEUE_SIZE) override the maximum size HSA queue created. This patch leaves the 4096 limit in place (which seems low to me) but removes the environment control.

We might want a whole suite of environment variables for overriding things like queue size, but I don't see anything particularly special about this one. Suggest we drop it and if someone objects reinstate it (in rtl.cpp, near the other environment uses)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108784