[Openmp-commits] [openmp] dc70c56 - [libomptarget][amdgpu][nfc] Update comments

[Jon Chesterfield via Openmp-commits]

Author: Jon Chesterfield
Date: 2021-01-23T22:53:58Z
New Revision: dc70c56be5922b874b1408edc1315fcda40680ba

URL: https://github.com/llvm/llvm-project/commit/dc70c56be5922b874b1408edc1315fcda40680ba
DIFF: https://github.com/llvm/llvm-project/commit/dc70c56be5922b874b1408edc1315fcda40680ba.diff

LOG: [libomptarget][amdgpu][nfc] Update comments

[libomptarget][amdgpu][nfc] Update comments

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D95295

Added: 
    

Modified: 
    openmp/libomptarget/plugins/amdgpu/impl/data.cpp
    openmp/libomptarget/plugins/amdgpu/impl/machine.h
    openmp/libomptarget/plugins/amdgpu/impl/rt.h
    openmp/libomptarget/plugins/amdgpu/impl/system.cpp
    openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/plugins/amdgpu/impl/data.cpp b/openmp/libomptarget/plugins/amdgpu/impl/data.cpp
index 39546fbae4b3..0d98d5c51ce1 100644
--- a/openmp/libomptarget/plugins/amdgpu/impl/data.cpp
+++ b/openmp/libomptarget/plugins/amdgpu/impl/data.cpp
@@ -51,7 +51,6 @@ hsa_amd_memory_pool_t get_memory_pool_by_mem_place(atmi_mem_place_t place) {
 void register_allocation(void *ptr, size_t size, atmi_mem_place_t place) {
   if (place.dev_type == ATMI_DEVTYPE_CPU)
     allow_access_to_all_gpu_agents(ptr);
-  // TODO(ashwinma): what if one GPU wants to access another GPU?
 }
 
 atmi_status_t Runtime::Malloc(void **ptr, size_t size, atmi_mem_place_t place) {

diff  --git a/openmp/libomptarget/plugins/amdgpu/impl/machine.h b/openmp/libomptarget/plugins/amdgpu/impl/machine.h
index 93169ed4eafb..9250c3b7c663 100644
--- a/openmp/libomptarget/plugins/amdgpu/impl/machine.h
+++ b/openmp/libomptarget/plugins/amdgpu/impl/machine.h
@@ -22,9 +22,6 @@ class ATLProcessor {
   }
   void addMemory(const ATLMemory &p);
   hsa_agent_t agent() const { return agent_; }
-  // TODO(ashwinma): Do we need this or are we building the machine structure
-  // just once in the program?
-  // void removeMemory(ATLMemory &p);
   const std::vector<ATLMemory> &memories() const;
   atmi_devtype_t type() const { return type_; }
 
@@ -86,7 +83,7 @@ template <typename T> T &get_processor(atmi_place_t place) {
   int dev_id = place.device_id;
   if (dev_id == -1) {
     // user is asking runtime to pick a device
-    // TODO(ashwinma): best device of this type? pick 0 for now
+    // best device of this type? pick 0 for now
     dev_id = 0;
   }
   return g_atl_machine.processors<T>()[dev_id];

diff  --git a/openmp/libomptarget/plugins/amdgpu/impl/rt.h b/openmp/libomptarget/plugins/amdgpu/impl/rt.h
index 757919eb3a45..a857861307c6 100644
--- a/openmp/libomptarget/plugins/amdgpu/impl/rt.h
+++ b/openmp/libomptarget/plugins/amdgpu/impl/rt.h
@@ -26,10 +26,7 @@ class Environment {
   void GetEnvAll();
 
   int getMaxQueueSize() const { return max_queue_size_; }
-
-  // TODO(ashwinma): int may change to enum if we have more debug modes
   int getDebugMode() const { return debug_mode_; }
-  // TODO(ashwinma): int may change to enum if we have more profile modes
 
 private:
   std::string GetEnv(const char *name) {
@@ -69,10 +66,7 @@ class Runtime final {
   static atmi_status_t Memfree(void *);
   static atmi_status_t Malloc(void **, size_t, atmi_mem_place_t);
 
-  // environment variables
   int getMaxQueueSize() const { return env_.getMaxQueueSize(); }
-
-  // TODO(ashwinma): int may change to enum if we have more debug modes
   int getDebugMode() const { return env_.getDebugMode(); }
 
 protected:

diff  --git a/openmp/libomptarget/plugins/amdgpu/impl/system.cpp b/openmp/libomptarget/plugins/amdgpu/impl/system.cpp
index 913dc91b298d..1a126a186ff2 100644
--- a/openmp/libomptarget/plugins/amdgpu/impl/system.cpp
+++ b/openmp/libomptarget/plugins/amdgpu/impl/system.cpp
@@ -133,7 +133,7 @@ static const std::map<std::string, KernelArgMD::ValueKind> ArgValueKind = {
     {"hidden_hostcall_buffer", KernelArgMD::ValueKind::HiddenHostcallBuffer},
 };
 
-// public variables -- TODO(ashwinma) move these to a runtime object?
+// global variables. TODO: Get rid of these
 atmi_machine_t g_atmi_machine;
 ATLMachine g_atl_machine;
 
@@ -210,8 +210,6 @@ atmi_status_t Runtime::Initialize() {
 }
 
 atmi_status_t Runtime::Finalize() {
-  // TODO(ashwinma): Finalize all processors, queues, signals, kernarg memory
-  // regions
   hsa_status_t err;
 
   for (uint32_t i = 0; i < g_executables.size(); i++) {
@@ -874,8 +872,6 @@ static hsa_status_t get_code_object_custom_metadata(void *binary,
         msgpackErrorCheck(iterate args map in kernel args metadata,
                           msgpack_errors);
 
-        // TODO(ashwinma): should the below population actions be done only for
-        // non-implicit args?
         // populate info with sizes and offsets
         info.arg_sizes.push_back(lcArg.size_);
         // v3 has offset field and not align field

diff  --git a/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp b/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
index 9453171e1378..f42722d4a770 100644
--- a/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
+++ b/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
@@ -1254,7 +1254,6 @@ __tgt_target_table *__tgt_rtl_load_binary_locked(int32_t device_id,
     }
   }
 
-  // TODO: Check with Guansong to understand the below comment more thoroughly.
   // Here, we take advantage of the data that is appended after img_end to get
   // the symbols' name we need to load. This data consist of the host entries
   // begin and end as well as the target name (see the offloading linker script