[libomptarget] Move HostDataToTargetTy states into StatesTy

[ye-luo]

Summary

Use unique_ptr to achieve the effect of mutable.

Remove mutable keyword of DynRefCount and HoldRefCount
Remove std::shared_ptr from UpdateMtx

[tianshilei1992]

LGTM. Thanks for the clean up.

[grokos]

openmp/libomptarget/src/device.h

60

Outdated comment, RefCount is no longer mutable.

[ye-luo]

openmp/libomptarget/src/device.h

60

Good catch. Updated.

[devnexen]

Devicety assignment operator now fails to compile due to HostDataToTargetTy members constified (clang/libc++)

[ye-luo]

In D109007#2983426, @devnexen wrote:

Devicety assignment operator now fails to compile due to HostDataToTargetTy members constified (clang/libc++)

D109276 should solve this issue.

openmp/libomptarget/src/device.h

enum kmp_target_offload_kind {
   tgt_disabled = 0,
   tgt_default = 1,
   tgt_mandatory = 2
 };
 typedef enum kmp_target_offload_kind kmp_target_offload_kind_t;
 
 /// Map between host data and target data.
 struct HostDataToTargetTy {
-  uintptr_t HstPtrBase; // host info.
-  uintptr_t HstPtrBegin;
-  uintptr_t HstPtrEnd;       // non-inclusive.
-  map_var_info_t HstPtrName; // Optional source name of mapped variable.
+  const uintptr_t HstPtrBase; // host info.
+  const uintptr_t HstPtrBegin;
+  const uintptr_t HstPtrEnd;       // non-inclusive.
+  const map_var_info_t HstPtrName; // Optional source name of mapped variable.
 
-  uintptr_t TgtPtrBegin; // target info.
+  const uintptr_t TgtPtrBegin; // target info.
 
 private:
-  /// use mutable to allow modification via std::set iterator which is const.
-  mutable uint64_t RefCount;
   static const uint64_t INFRefCount = ~(uint64_t)0;
+
+  struct StatesTy {
+    StatesTy(uint64_t RC) : RefCount(RC) {}
+    /// this copy constructor is added to make HostDataToTargetTy copiable
+    /// when it is used by std::set copy constructor
+    StatesTy(const StatesTy &S) : RefCount(S.RefCount) {}
+    /// use mutable to allow modification via std::set iterator which is const.
+    uint64_t RefCount;
     /// This mutex will be locked when data movement is issued. For targets that
     /// doesn't support async data movement, this mutex can guarantee that after
-  /// it is released, memory region on the target is update to date. For targets
-  /// that support async data movement, this can guarantee that data movement
-  /// has been issued. This mutex *must* be locked right before releasing the
-  /// mapping table lock.
-  std::shared_ptr<std::mutex> UpdateMtx;
+    /// it is released, memory region on the target is update to date. For
+    /// targets that support async data movement, this can guarantee that data
+    /// movement has been issued. This mutex *must* be locked right before
+    /// releasing the mapping table lock.
+    std::mutex UpdateMtx;
+  };
+  // When HostDataToTargetTy is used by std::set, std::set::iterator is const
+  // use unique_ptr to make States mutable.
+  const std::unique_ptr<StatesTy> States;
 
 public:
   HostDataToTargetTy(uintptr_t BP, uintptr_t B, uintptr_t E, uintptr_t TB,
                      map_var_info_t Name = nullptr, bool IsINF = false)
       : HstPtrBase(BP), HstPtrBegin(B), HstPtrEnd(E), HstPtrName(Name),
-        TgtPtrBegin(TB), RefCount(IsINF ? INFRefCount : 1),
-        UpdateMtx(std::make_shared<std::mutex>()) {}
+        TgtPtrBegin(TB),
+        States(std::make_unique<StatesTy>(IsINF ? INFRefCount : 1)) {}
+
+  HostDataToTargetTy(const HostDataToTargetTy &Entry)
+      : HstPtrBase(Entry.HstPtrBase), HstPtrBegin(Entry.HstPtrBegin),
+        HstPtrEnd(Entry.HstPtrEnd), HstPtrName(Entry.HstPtrName),
+        TgtPtrBegin(Entry.TgtPtrBegin),
+        States(std::make_unique<StatesTy>(*Entry.States)) {}
 
-  uint64_t getRefCount() const { return RefCount; }
+  uint64_t getRefCount() const { return States->RefCount; }
 
   uint64_t resetRefCount() const {
-    if (RefCount != INFRefCount)
-      RefCount = 1;
+    if (States->RefCount != INFRefCount)
+      States->RefCount = 1;
 
-    return RefCount;
+    return States->RefCount;
   }
 
   uint64_t incRefCount() const {
-    if (RefCount != INFRefCount) {
-      ++RefCount;
-      assert(RefCount < INFRefCount && "refcount overflow");
+    if (States->RefCount != INFRefCount) {
+      ++States->RefCount;
+      assert(States->RefCount < INFRefCount && "refcount overflow");
     }
 
-    return RefCount;
+    return States->RefCount;
   }
 
   uint64_t decRefCount() const {
-    if (RefCount != INFRefCount) {
-      assert(RefCount > 0 && "refcount underflow");
-      --RefCount;
+    if (States->RefCount != INFRefCount) {
+      assert(States->RefCount > 0 && "refcount underflow");
+      --States->RefCount;
     }
 
-    return RefCount;
+    return States->RefCount;
   }
 
-  bool isRefCountInf() const { return RefCount == INFRefCount; }
+  bool isRefCountInf() const { return States->RefCount == INFRefCount; }
 
   std::string refCountToStr() const {
     return isRefCountInf() ? "INF" : std::to_string(getRefCount());
   }
 
   /// Should one decrement of the reference count (after resetting it if
   /// \c AfterReset) remove this mapping?
   bool decShouldRemove(bool AfterReset = false) const {
     if (AfterReset)
       return !isRefCountInf();
     return getRefCount() == 1;
   }
 
-  void lock() const { UpdateMtx->lock(); }
+  void lock() const { States->UpdateMtx.lock(); }
 
-  void unlock() const { UpdateMtx->unlock(); }
+  void unlock() const { States->UpdateMtx.unlock(); }
 };
 
 typedef uintptr_t HstPtrBeginTy;
 inline bool operator<(const HostDataToTargetTy &lhs, const HstPtrBeginTy &rhs) {
   return lhs.HstPtrBegin < rhs;
 }
 inline bool operator<(const HstPtrBeginTy &lhs, const HostDataToTargetTy &rhs) {
   return lhs < rhs.HstPtrBegin;