diff mbox

[v2] libitm: Support sized delete.

Message ID 1447028699.7192.120.camel@localhost.localdomain
State New
Headers show

Commit Message

Torvald Riegel Nov. 9, 2015, 12:24 a.m. UTC
This patch supports the sized variants of operator delete.
Some change compare to v1.
Tested on x86_64-linux.

Comments

Richard Henderson Nov. 9, 2015, 9:30 a.m. UTC | #1
On 11/09/2015 01:24 AM, Torvald Riegel wrote:
> This patch supports the sized variants of operator delete.
> Some change compare to v1.
> Tested on x86_64-linux.
>

Looks good.


r~
diff mbox

Patch

commit df00a283f2e37bd3c69f37783fa81dde7ccf1f94
Author: Torvald Riegel <triegel@redhat.com>
Date:   Thu Oct 29 18:52:20 2015 +0100

    Support sized delete.
    
    This adds transactional clones of the sized version of operator delete.

diff --git a/libitm/alloc.cc b/libitm/alloc.cc
index bb292da..7b8786c 100644
--- a/libitm/alloc.cc
+++ b/libitm/alloc.cc
@@ -29,26 +29,38 @@  namespace GTM HIDDEN {
 void
 gtm_thread::record_allocation (void *ptr, void (*free_fn)(void *))
 {
-  uintptr_t iptr = (uintptr_t) ptr;
-
-  gtm_alloc_action *a = this->alloc_actions.find(iptr);
-  if (a == 0)
-    a = this->alloc_actions.insert(iptr);
+  // We do not deallocate before outermost commit, so we should never have
+  // an existing log entry for a new allocation.
+  gtm_alloc_action *a = this->alloc_actions.insert((uintptr_t) ptr);
 
   a->free_fn = free_fn;
+  a->free_fn_sz = 0;
   a->allocated = true;
 }
 
 void
 gtm_thread::forget_allocation (void *ptr, void (*free_fn)(void *))
 {
-  uintptr_t iptr = (uintptr_t) ptr;
-
-  gtm_alloc_action *a = this->alloc_actions.find(iptr);
-  if (a == 0)
-    a = this->alloc_actions.insert(iptr);
-
+  // We do not deallocate before outermost commit, so we should never have
+  // an existing log entry for a deallocation at the same address.  We may
+  // have an existing entry for a matching allocation, but this is handled
+  // correctly because both are complementary in that only one of these will
+  // cause an action at commit or abort.
+  gtm_alloc_action *a = this->alloc_actions.insert((uintptr_t) ptr);
   a->free_fn = free_fn;
+  a->free_fn_sz = 0;
+  a->allocated = false;
+}
+
+void
+gtm_thread::forget_allocation (void *ptr, size_t sz,
+			       void (*free_fn_sz)(void *, size_t))
+{
+  // Same as forget_allocation but with a size.
+  gtm_alloc_action *a = this->alloc_actions.insert((uintptr_t) ptr);
+  a->free_fn = 0;
+  a->free_fn_sz = free_fn_sz;
+  a->sz = sz;
   a->allocated = false;
 }
 
@@ -67,31 +79,27 @@  commit_allocations_2 (uintptr_t key, gtm_alloc_action *a, void *data)
 
   if (cb_data->revert_p)
     {
-      // Roll back nested allocations.
+      // Roll back nested allocations, discard deallocations.
       if (a->allocated)
-	a->free_fn (ptr);
+	{
+	  if (a->free_fn_sz != 0)
+	    a->free_fn_sz (ptr, a->sz);
+	  else
+	    a->free_fn (ptr);
+	}
     }
   else
     {
-      if (a->allocated)
-	{
-	  // Add nested allocations to parent transaction.
-	  gtm_alloc_action* a_parent = cb_data->parent->insert(key);
-	  *a_parent = *a;
-	}
-      else
-	{
-	  // ??? We could eliminate a parent allocation that matches this
-	  // memory release, if we had support for removing all accesses
-	  // to this allocation from the transaction's undo and redo logs
-	  // (otherwise, the parent transaction's undo or redo might write to
-	  // data that is already shared again because of calling free()).
-	  // We don't have this support currently, and the benefit of this
-	  // optimization is unknown, so just add it to the parent.
-	  gtm_alloc_action* a_parent;
-	  a_parent = cb_data->parent->insert(key);
-	  *a_parent = *a;
-	}
+      // Add allocations and deallocations to parent.
+      // ??? We could eliminate a (parent) allocation that matches this
+      // a deallocation, if we had support for removing all accesses
+      // to this allocation from the transaction's undo and redo logs
+      // (otherwise, the parent transaction's undo or redo might write to
+      // data that is already shared again because of calling free()).
+      // We don't have this support currently, and the benefit of this
+      // optimization is unknown, so just add it to the parent.
+      gtm_alloc_action* a_parent = cb_data->parent->insert(key);
+      *a_parent = *a;
     }
 }
 
@@ -99,10 +107,15 @@  static void
 commit_allocations_1 (uintptr_t key, gtm_alloc_action *a, void *cb_data)
 {
   void *ptr = (void *)key;
-  uintptr_t revert_p = (uintptr_t) cb_data;
+  bool revert_p = (bool) (uintptr_t) cb_data;
 
-  if (a->allocated == revert_p)
-    a->free_fn (ptr);
+  if (revert_p == a->allocated)
+    {
+      if (a->free_fn_sz != 0)
+	a->free_fn_sz (ptr, a->sz);
+      else
+	a->free_fn (ptr);
+    }
 }
 
 /* Permanently commit allocated memory during transaction.
diff --git a/libitm/alloc_cpp.cc b/libitm/alloc_cpp.cc
index 8514618..13185a7 100644
--- a/libitm/alloc_cpp.cc
+++ b/libitm/alloc_cpp.cc
@@ -35,41 +35,50 @@  using namespace GTM;
 
 #define _ZnwX			S(_Znw,MANGLE_SIZE_T)
 #define _ZnaX			S(_Zna,MANGLE_SIZE_T)
+#define _ZdlPvX			S(_ZdlPv,MANGLE_SIZE_T)
 #define _ZnwXRKSt9nothrow_t	S(S(_Znw,MANGLE_SIZE_T),RKSt9nothrow_t)
 #define _ZnaXRKSt9nothrow_t	S(S(_Zna,MANGLE_SIZE_T),RKSt9nothrow_t)
+#define _ZdlPvXRKSt9nothrow_t	S(S(_ZdlPv,MANGLE_SIZE_T),RKSt9nothrow_t)
 
 #define _ZGTtnwX		S(_ZGTtnw,MANGLE_SIZE_T)
 #define _ZGTtnaX		S(_ZGTtna,MANGLE_SIZE_T)
+#define _ZGTtdlPvX		S(_ZGTtdlPv,MANGLE_SIZE_T)
 #define _ZGTtnwXRKSt9nothrow_t	S(S(_ZGTtnw,MANGLE_SIZE_T),RKSt9nothrow_t)
 #define _ZGTtnaXRKSt9nothrow_t	S(S(_ZGTtna,MANGLE_SIZE_T),RKSt9nothrow_t)
+#define _ZGTtdlPvXRKSt9nothrow_t S(S(_ZGTtdlPv,MANGLE_SIZE_T),RKSt9nothrow_t)
 
 /* Everything from libstdc++ is weak, to avoid requiring that library
    to be linked into plain C applications using libitm.so.  */
 
 extern "C" {
 
-extern void *_ZnwX (size_t) __attribute__((weak));
-extern void _ZdlPv (void *) __attribute__((weak));
-extern void *_ZnaX (size_t) __attribute__((weak));
-extern void _ZdaPv (void *) __attribute__((weak));
+extern void *_ZnwX  (size_t) __attribute__((weak));
+extern void _ZdlPv  (void *) __attribute__((weak));
+extern void _ZdlPvX (void *, size_t) __attribute__((weak));
+extern void *_ZnaX  (size_t) __attribute__((weak));
+extern void _ZdaPv  (void *) __attribute__((weak));
 
 typedef const struct nothrow_t { } *c_nothrow_p;
 
 extern void *_ZnwXRKSt9nothrow_t (size_t, c_nothrow_p) __attribute__((weak));
 extern void _ZdlPvRKSt9nothrow_t (void *, c_nothrow_p) __attribute__((weak));
+extern void _ZdlPvXRKSt9nothrow_t
+(void *, size_t, c_nothrow_p) __attribute__((weak));
 extern void *_ZnaXRKSt9nothrow_t (size_t, c_nothrow_p) __attribute__((weak));
 extern void _ZdaPvRKSt9nothrow_t (void *, c_nothrow_p) __attribute__((weak));
 
 #if !defined (HAVE_ELF_STYLE_WEAKREF) 
-void *_ZnwX (size_t) { return NULL; }
-void _ZdlPv (void *) { return; }
-void *_ZnaX (size_t) { return NULL; }
-void _ZdaPv (void *) { return; }
-
-void *_ZnwXRKSt9nothrow_t (size_t, c_nothrow_p) { return NULL; }
-void _ZdlPvRKSt9nothrow_t (void *, c_nothrow_p) { return; }
-void *_ZnaXRKSt9nothrow_t (size_t, c_nothrow_p) { return NULL; }
-void _ZdaPvRKSt9nothrow_t (void *, c_nothrow_p) { return; }
+void *_ZnwX  (size_t) { return NULL; }
+void _ZdlPv  (void *) { return; }
+void _ZdlPvX (void *, size_t) { return; }
+void *_ZnaX  (size_t) { return NULL; }
+void _ZdaPv  (void *) { return; }
+
+void *_ZnwXRKSt9nothrow_t  (size_t, c_nothrow_p) { return NULL; }
+void _ZdlPvRKSt9nothrow_t  (void *, c_nothrow_p) { return; }
+void _ZdlPvXRKSt9nothrow_t (void *, size_t, c_nothrow_p) { return; }
+void *_ZnaXRKSt9nothrow_t  (size_t, c_nothrow_p) { return NULL; }
+void _ZdaPvRKSt9nothrow_t  (void *, c_nothrow_p) { return; }
 #endif /* HAVE_ELF_STYLE_WEAKREF */
 
 /* Wrap the delete nothrow symbols for usage with a single argument.
@@ -89,6 +98,12 @@  del_opvnt (void *ptr)
   _ZdaPvRKSt9nothrow_t (ptr, NULL);
 }
 
+static void
+delsz_opnt (void *ptr, size_t sz)
+{
+  _ZdlPvXRKSt9nothrow_t (ptr, sz, NULL);
+}
+
 /* Wrap: operator new (std::size_t sz)  */
 void *
 _ZGTtnwX (size_t sz)
@@ -161,4 +176,20 @@  _ZGTtdaPvRKSt9nothrow_t (void *ptr, c_nothrow_p nt UNUSED)
     gtm_thr()->forget_allocation (ptr, del_opvnt);
 }
 
+/* Wrap: operator delete(void* ptr, std::size_t sz)  */
+void
+_ZGTtdlPvX (void *ptr, size_t sz)
+{
+  if (ptr)
+    gtm_thr()->forget_allocation (ptr, sz, _ZdlPvX);
+}
+
+/* Wrap: operator delete (void *ptr, std::size_t sz, const std::nothrow_t&)  */
+void
+_ZGTtdlPvXRKSt9nothrow_t (void *ptr, size_t sz, c_nothrow_p nt UNUSED)
+{
+  if (ptr)
+    gtm_thr()->forget_allocation (ptr, sz, delsz_opnt);
+}
+
 } // extern "C"
diff --git a/libitm/libitm.map b/libitm/libitm.map
index 21bcfdf..ac371de 100644
--- a/libitm/libitm.map
+++ b/libitm/libitm.map
@@ -182,3 +182,8 @@  LIBITM_1.0 {
   local:
 	*;
 };
+LIBITM_1.1 {
+  global:
+	_ZGTtdlPv?;
+	_ZGTtdlPv?RKSt9nothrow_t;
+} LIBITM_1.0;
diff --git a/libitm/libitm_i.h b/libitm/libitm_i.h
index 0eda01b..bf8d4d1 100644
--- a/libitm/libitm_i.h
+++ b/libitm/libitm_i.h
@@ -97,11 +97,25 @@  enum gtm_restart_reason
 
 namespace GTM HIDDEN {
 
+// A log of (de)allocation actions.  We defer handling of some actions until
+// a commit of the outermost transaction.  We also rely on potentially having
+// both an allocation and a deallocation for the same piece of memory in the
+// log; the order in which such entries are processed does not matter because
+// the actions are not in conflict (see below).
 // This type is private to alloc.c, but needs to be defined so that
 // the template used inside gtm_thread can instantiate.
 struct gtm_alloc_action
 {
-  void (*free_fn)(void *);
+  // Iff free_fn_sz is nonzero, it must be used instead of free_fn.
+  union
+  {
+    void (*free_fn)(void *);
+    void (*free_fn_sz)(void *, size_t);
+  };
+  size_t sz;
+  // If true, this is an allocation; we discard the log entry on outermost
+  // commit, and deallocate on abort.  If false, this is a deallocation and
+  // we deallocate on outermost commit and discard the log entry on abort.
   bool allocated;
 };
 
@@ -269,6 +283,7 @@  struct gtm_thread
   void commit_allocations (bool, aa_tree<uintptr_t, gtm_alloc_action>*);
   void record_allocation (void *, void (*)(void *));
   void forget_allocation (void *, void (*)(void *));
+  void forget_allocation (void *, size_t, void (*)(void *, size_t));
   void drop_references_allocations (const void *ptr)
   {
     this->alloc_actions.erase((uintptr_t) ptr);
diff --git a/libitm/testsuite/libitm.c++/newdelete.C b/libitm/testsuite/libitm.c++/newdelete.C
new file mode 100644
index 0000000..10eba4f
--- /dev/null
+++ b/libitm/testsuite/libitm.c++/newdelete.C
@@ -0,0 +1,12 @@ 
+// { dg-do run }
+
+int main ()
+{
+  atomic_commit {
+    int* data = new int;
+    delete data;
+    data = new int[10];
+    delete[] data;
+  }
+  return 0;
+}