[llvm-branch-commits] [openmp] 175c336 - [OpenMP] Remove copy constructor of `RTLInfoTy`

Shilei Tian via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Sat Jan 9 10:05:07 PST 2021


Author: Shilei Tian
Date: 2021-01-09T13:01:01-05:00
New Revision: 175c336a1c5a7d4cf2e24ec5188c370cd6093ddb

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

LOG: [OpenMP] Remove copy constructor of `RTLInfoTy`

Multiple `RTLInfoTy` objects are stored in a list `AllRTLs`. Since
`RTLInfoTy` contains a `std::mutex`, it is by default not a copyable object.
In order to support `AllRTLs.push_back(...)` which is currently used, a customized
copy constructor is provided. Every time we need to add a new data member into
`RTLInfoTy`, we should keep in mind not forgetting to add corresponding assignment
in the copy constructor. In fact, the only use of the copy constructor is to push
the object into the list, we can of course write it in a way that first emplace
a new object back, and then use the reference to the last element. In this way we
don't need the copy constructor anymore. If the element is invalid, we just need
to pop it, and that's what this patch does.

Reviewed By: JonChesterfield

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

Added: 
    

Modified: 
    openmp/libomptarget/src/rtl.cpp
    openmp/libomptarget/src/rtl.h

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/src/rtl.cpp b/openmp/libomptarget/src/rtl.cpp
index d8ed5d2da1c6..443359a62401 100644
--- a/openmp/libomptarget/src/rtl.cpp
+++ b/openmp/libomptarget/src/rtl.cpp
@@ -88,46 +88,67 @@ void RTLsTy::LoadRTLs() {
 
     DP("Successfully loaded library '%s'!\n", Name);
 
-    // Retrieve the RTL information from the runtime library.
-    RTLInfoTy R;
+    AllRTLs.emplace_back();
 
-    R.LibraryHandler = dynlib_handle;
-    R.isUsed = false;
+    // Retrieve the RTL information from the runtime library.
+    RTLInfoTy &R = AllRTLs.back();
 
-#ifdef OMPTARGET_DEBUG
-    R.RTLName = Name;
-#endif
+    bool ValidPlugin = true;
 
     if (!(*((void **)&R.is_valid_binary) =
               dlsym(dynlib_handle, "__tgt_rtl_is_valid_binary")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.number_of_devices) =
               dlsym(dynlib_handle, "__tgt_rtl_number_of_devices")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.init_device) =
               dlsym(dynlib_handle, "__tgt_rtl_init_device")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.load_binary) =
               dlsym(dynlib_handle, "__tgt_rtl_load_binary")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.data_alloc) =
               dlsym(dynlib_handle, "__tgt_rtl_data_alloc")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.data_submit) =
               dlsym(dynlib_handle, "__tgt_rtl_data_submit")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.data_retrieve) =
               dlsym(dynlib_handle, "__tgt_rtl_data_retrieve")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.data_delete) =
               dlsym(dynlib_handle, "__tgt_rtl_data_delete")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.run_region) =
               dlsym(dynlib_handle, "__tgt_rtl_run_target_region")))
-      continue;
+      ValidPlugin = false;
     if (!(*((void **)&R.run_team_region) =
               dlsym(dynlib_handle, "__tgt_rtl_run_target_team_region")))
+      ValidPlugin = false;
+
+    // Invalid plugin
+    if (!ValidPlugin) {
+      DP("Invalid plugin as necessary interface is not found.\n");
+      AllRTLs.pop_back();
+      continue;
+    }
+
+    // No devices are supported by this RTL?
+    if (!(R.NumberOfDevices = R.number_of_devices())) {
+      // The RTL is invalid! Will pop the object from the RTLs list.
+      DP("No devices supported in this RTL\n");
+      AllRTLs.pop_back();
       continue;
+    }
+
+    R.LibraryHandler = dynlib_handle;
+
+#ifdef OMPTARGET_DEBUG
+    R.RTLName = Name;
+#endif
+
+    DP("Registering RTL %s supporting %d devices!\n", R.RTLName.c_str(),
+       R.NumberOfDevices);
 
     // Optional functions
     *((void **)&R.init_requires) =
@@ -147,18 +168,6 @@ void RTLsTy::LoadRTLs() {
         dlsym(dynlib_handle, "__tgt_rtl_data_exchange_async");
     *((void **)&R.is_data_exchangable) =
         dlsym(dynlib_handle, "__tgt_rtl_is_data_exchangable");
-
-    // No devices are supported by this RTL?
-    if (!(R.NumberOfDevices = R.number_of_devices())) {
-      DP("No devices supported in this RTL\n");
-      continue;
-    }
-
-    DP("Registering RTL %s supporting %d devices!\n", R.RTLName.c_str(),
-       R.NumberOfDevices);
-
-    // The RTL is valid! Will save the information in the RTLs list.
-    AllRTLs.push_back(R);
   }
 
   DP("RTLs loaded!\n");

diff  --git a/openmp/libomptarget/src/rtl.h b/openmp/libomptarget/src/rtl.h
index 4667936a944d..b9ead48cd7ec 100644
--- a/openmp/libomptarget/src/rtl.h
+++ b/openmp/libomptarget/src/rtl.h
@@ -94,39 +94,6 @@ struct RTLInfoTy {
   // It is easier to enforce thread-safety at the libomptarget level,
   // so that developers of new RTLs do not have to worry about it.
   std::mutex Mtx;
-
-  // The existence of the mutex above makes RTLInfoTy non-copyable.
-  // We need to provide a copy constructor explicitly.
-  RTLInfoTy() = default;
-
-  RTLInfoTy(const RTLInfoTy &r) {
-    Idx = r.Idx;
-    NumberOfDevices = r.NumberOfDevices;
-    LibraryHandler = r.LibraryHandler;
-#ifdef OMPTARGET_DEBUG
-    RTLName = r.RTLName;
-#endif
-    is_valid_binary = r.is_valid_binary;
-    is_data_exchangable = r.is_data_exchangable;
-    number_of_devices = r.number_of_devices;
-    init_device = r.init_device;
-    load_binary = r.load_binary;
-    data_alloc = r.data_alloc;
-    data_submit = r.data_submit;
-    data_submit_async = r.data_submit_async;
-    data_retrieve = r.data_retrieve;
-    data_retrieve_async = r.data_retrieve_async;
-    data_exchange = r.data_exchange;
-    data_exchange_async = r.data_exchange_async;
-    data_delete = r.data_delete;
-    run_region = r.run_region;
-    run_region_async = r.run_region_async;
-    run_team_region = r.run_team_region;
-    run_team_region_async = r.run_team_region_async;
-    init_requires = r.init_requires;
-    isUsed = r.isUsed;
-    synchronize = r.synchronize;
-  }
 };
 
 /// RTLs identified in the system.


        


More information about the llvm-branch-commits mailing list