[Openmp-commits] [openmp] 4756f76 - [libomptarget][amdgcn] Tolerate deadstripped env variable

[Jon Chesterfield via Openmp-commits]

Author: Jon Chesterfield
Date: 2021-02-09T11:58:37Z
New Revision: 4756f76bce7835dd233c82637bf60082469f9320

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

LOG: [libomptarget][amdgcn] Tolerate deadstripped env variable

[libomptarget][amdgcn] Tolerate deadstripped env variable

Discovered by Pushpinder. If the device_environment variable is unused
it can be deadstripped, in which case we should not abort due to it
missing. This change is safe in that a missing symbol which is actually
used can be reported by both linker and loader, and a missing unused
symbol is better deadstripped than left in the image.

Reviewed By: pdhaliwal

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

Added: 
    

Modified: 
    openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp b/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
index f42722d4a770..0d41bde1e62b 100644
--- a/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
+++ b/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
@@ -1025,6 +1025,9 @@ struct device_environment {
   //  - under review in trunk is debug_level, device_num
   //  - rocmcc matches aomp, patch to swap num_devices and device_num
 
+  // The symbol may also have been deadstripped because the device side
+  // accessors were unused.
+
   // If the symbol is in .data (aomp, rocm) it can be written directly.
   // If it is in .bss, we must wait for it to be allocated space on the
   // gpu (trunk) and initialize after loading.
@@ -1069,39 +1072,43 @@ struct device_environment {
   bool in_image() { return si.sh_type != SHT_NOBITS; }
 
   atmi_status_t before_loading(void *data, size_t size) {
-    assert(valid);
-    if (in_image()) {
-      DP("Setting global device environment before load (%u bytes)\n", si.size);
-      uint64_t offset = (char *)si.addr - (char *)image->ImageStart;
-      void *pos = (char *)data + offset;
-      memcpy(pos, &host_device_env, si.size);
+    if (valid) {
+      if (in_image()) {
+        DP("Setting global device environment before load (%u bytes)\n",
+           si.size);
+        uint64_t offset = (char *)si.addr - (char *)image->ImageStart;
+        void *pos = (char *)data + offset;
+        memcpy(pos, &host_device_env, si.size);
+      }
     }
     return ATMI_STATUS_SUCCESS;
   }
 
   atmi_status_t after_loading() {
-    assert(valid);
-    if (!in_image()) {
-      DP("Setting global device environment after load (%u bytes)\n", si.size);
-      int device_id = host_device_env.device_num;
+    if (valid) {
+      if (!in_image()) {
+        DP("Setting global device environment after load (%u bytes)\n",
+           si.size);
+        int device_id = host_device_env.device_num;
 
-      void *state_ptr;
-      uint32_t state_ptr_size;
-      atmi_status_t err = atmi_interop_hsa_get_symbol_info(
-          get_gpu_mem_place(device_id), sym(), &state_ptr, &state_ptr_size);
-      if (err != ATMI_STATUS_SUCCESS) {
-        DP("failed to find %s in loaded image\n", sym());
-        return err;
-      }
+        void *state_ptr;
+        uint32_t state_ptr_size;
+        atmi_status_t err = atmi_interop_hsa_get_symbol_info(
+            get_gpu_mem_place(device_id), sym(), &state_ptr, &state_ptr_size);
+        if (err != ATMI_STATUS_SUCCESS) {
+          DP("failed to find %s in loaded image\n", sym());
+          return err;
+        }
 
-      if (state_ptr_size != si.size) {
-        DP("Symbol had size %u before loading, %u after\n", state_ptr_size,
-           si.size);
-        return ATMI_STATUS_ERROR;
-      }
+        if (state_ptr_size != si.size) {
+          DP("Symbol had size %u before loading, %u after\n", state_ptr_size,
+             si.size);
+          return ATMI_STATUS_ERROR;
+        }
 
-      return DeviceInfo.freesignalpool_memcpy_h2d(state_ptr, &host_device_env,
-                                                  state_ptr_size, device_id);
+        return DeviceInfo.freesignalpool_memcpy_h2d(state_ptr, &host_device_env,
+                                                    state_ptr_size, device_id);
+      }
     }
     return ATMI_STATUS_SUCCESS;
   }
@@ -1165,9 +1172,6 @@ __tgt_target_table *__tgt_rtl_load_binary_locked(int32_t device_id,
   {
     auto env = device_environment(device_id, DeviceInfo.NumberOfDevices, image,
                                   img_size);
-    if (!env.valid) {
-      return NULL;
-    }
 
     atmi_status_t err = module_register_from_memory_to_place(
         (void *)image->ImageStart, img_size, get_gpu_place(device_id),