[Openmp-commits] [PATCH] D94361: [OpenMP] Remove copy constructor of `RTLInfoTy`

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 created this revision.
Herald added subscribers: guansong, yaxunl.
tianshilei1992 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: openmp-commits, sstefan1.
Herald added a project: OpenMP.

Multiple `RTLInfoTy` objects are stored in a vector `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 vector, 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94361

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


Index: openmp/libomptarget/src/rtl.h
===================================================================
--- openmp/libomptarget/src/rtl.h
+++ openmp/libomptarget/src/rtl.h
@@ -94,39 +94,6 @@
   // 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.
Index: openmp/libomptarget/src/rtl.cpp
===================================================================
--- openmp/libomptarget/src/rtl.cpp
+++ openmp/libomptarget/src/rtl.cpp
@@ -88,11 +88,12 @@
 
     DP("Successfully loaded library '%s'!\n", Name);
 
+    AllRTLs.emplace_back();
+
     // Retrieve the RTL information from the runtime library.
-    RTLInfoTy R;
+    RTLInfoTy &R = AllRTLs.back();
 
     R.LibraryHandler = dynlib_handle;
-    R.isUsed = false;
 
 #ifdef OMPTARGET_DEBUG
     R.RTLName = Name;
@@ -150,15 +151,14 @@
 
     // 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;
     }
 
     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");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D94361.315602.patch
Type: text/x-patch
Size: 2591 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20210109/28f09caf/attachment.bin>