diff mbox

[trans-mem] implement _ITM_dropReferences

Message ID 20100624145014.GA27783@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez June 24, 2010, 2:50 p.m. UTC 
On Wed, Jun 23, 2010 at 11:59:10AM -0700, Richard Henderson wrote:
> On 06/23/2010 10:12 AM, Aldy Hernandez wrote:
> > +void ITM_REGPARM
> > +_ITM_dropReferences (const void *ptr, size_t len)
> > +{
> > +  gtm_transaction *tx = gtm_tx();
> > +  tx->drop_references_local (ptr, len);
> > +  tx->drop_references_allocations (ptr);
> > +}
> 
> The primary thing missing here is dropping references inside
> the actual transactional memory backend, i.e. gtm_dispatch.

Ughhhhh... Ok, since I'm about to seriously botch this part of the
implementation as I fight with cachelines and masks, can I get a commit
of the rest of the patch, and continue on to the dispatch code?

> Why add the ignore field when you could simply alloc_actions.erase?

Cause my brain wasn't working?

> Similarly, it seems easy enough to free the one element and test instead
> in rollback_local for u == NULL.

Likewise.

Fixed below.  OK provided I work on the dispatch code next?

	* alloc_c.cc (_ITM_dropReferences): New.
	* libitm.map (_ITM_dropReferences): Add.
	* libitm.h (_ITM_dropReferences): Add transaction_pure attribute.
	* libitm_i.h (struct gtm_transaction): Declare
	drop_references_allocations and drop_references_local.
	* local.cc (rollback_local): Ignore memory when applicable.
	(drop_references_local): New.

Comments

Richard Henderson June 24, 2010, 4:47 p.m. UTC | #1 
On 06/24/2010 07:50 AM, Aldy Hernandez wrote:
> +	  gtm_local_undo *u = local_undo[i];
> +	  /* ?? Do we need such granularity, or can we get away with
> +	     just comparing PTR and LEN. ??  */
> +	  if ((const char *)u->addr >= (const char *)ptr
> +	      && ((const char *)u->addr + u->len <= (const char *)ptr + len))
> +	    this->local_undo = NULL;
> +	}

Um, no.

  free (u);
  local_undo[i] = NULL;

Ok with that fix.


r~
Aldy Hernandez June 24, 2010, 5:05 p.m. UTC | #2 
On Thu, Jun 24, 2010 at 09:47:28AM -0700, Richard Henderson wrote:
> On 06/24/2010 07:50 AM, Aldy Hernandez wrote:
> > +	  gtm_local_undo *u = local_undo[i];
> > +	  /* ?? Do we need such granularity, or can we get away with
> > +	     just comparing PTR and LEN. ??  */
> > +	  if ((const char *)u->addr >= (const char *)ptr
> > +	      && ((const char *)u->addr + u->len <= (const char *)ptr + len))
> > +	    this->local_undo = NULL;
> > +	}
> 
> Um, no.
> 
>   free (u);
>   local_undo[i] = NULL;

Whooops :).

OK, fixed and committing.
diff mbox

Patch

Index: alloc_c.cc
===================================================================
--- alloc_c.cc	(revision 161318)
+++ alloc_c.cc	(working copy)
@@ -57,4 +57,15 @@  _ITM_free (void *ptr)
     gtm_tx()->forget_allocation (ptr, free);
 }
 
+/* Forget any internal references to PTR.  */
+
+__attribute__((transaction_pure))
+void ITM_REGPARM
+_ITM_dropReferences (const void *ptr, size_t len)
+{
+  gtm_transaction *tx = gtm_tx();
+  tx->drop_references_local (ptr, len);
+  tx->drop_references_allocations (ptr);
+}
+
 } // extern "C"
Index: libitm.map
===================================================================
--- libitm.map	(revision 161318)
+++ libitm.map	(working copy)
@@ -167,6 +167,7 @@  LIBITM_1.0 {
 	_ITM_malloc;
 	_ITM_calloc;
 	_ITM_free;
+	_ITM_dropReferences;
 
 	_ZGTtnwm;
 	_ZGTtnam;
Index: libitm.h
===================================================================
--- libitm.h	(revision 161318)
+++ libitm.h	(working copy)
@@ -152,6 +152,7 @@  extern void _ITM_addUserUndoAction(_ITM_
 
 extern int _ITM_getThreadnum(void) ITM_REGPARM;
 
+__attribute__((transaction_pure))
 extern void _ITM_dropReferences (const void *, size_t) ITM_REGPARM;
 
 
Index: testsuite/libitm.c/dropref.c
===================================================================
--- testsuite/libitm.c/dropref.c	(revision 0)
+++ testsuite/libitm.c/dropref.c	(revision 0)
@@ -0,0 +1,11 @@ 
+#include <libitm.h>
+
+char *pp;
+
+int main()
+{
+  __transaction {
+    _ITM_dropReferences (pp, 555);
+  }
+  return 0;
+}
Index: testsuite/libitm.c++/dropref.C
===================================================================
--- testsuite/libitm.c++/dropref.C	(revision 0)
+++ testsuite/libitm.c++/dropref.C	(revision 0)
@@ -0,0 +1,11 @@ 
+#include <libitm.h>
+
+char *pp;
+
+int main()
+{
+  __transaction {
+    _ITM_dropReferences (pp, 555);
+  }
+  return 0;
+}
Index: libitm_i.h
===================================================================
--- libitm_i.h	(revision 161318)
+++ libitm_i.h	(working copy)
@@ -226,6 +226,10 @@  struct gtm_transaction
   void commit_allocations (bool);
   void record_allocation (void *, void (*)(void *));
   void forget_allocation (void *, void (*)(void *));
+  void drop_references_allocations (const void *ptr)
+  {
+    this->alloc_actions.erase((uintptr_t) ptr);
+  }
 
   // In beginend.cc
   void rollback ();
@@ -247,6 +251,7 @@  struct gtm_transaction
   // In local.cc
   void commit_local (void);
   void rollback_local (void);
+  void drop_references_local (const void *, size_t);
 
   // In retry.cc
   void decide_retry_strategy (gtm_restart_reason);
Index: local.cc
===================================================================
--- local.cc	(revision 161318)
+++ local.cc	(working copy)
@@ -65,13 +65,38 @@  gtm_transaction::rollback_local (void)
       for (i = n; i-- > 0; )
 	{
 	  gtm_local_undo *u = local_undo[i];
-	  memcpy (u->addr, u->saved, u->len);
-	  free (u);
+	  if (u)
+	    {
+	      memcpy (u->addr, u->saved, u->len);
+	      free (u);
+	    }
 	}
       this->n_local_undo = 0;
     }
 }
 
+/* Forget any references to PTR in the local log.  */
+
+void
+gtm_transaction::drop_references_local (const void *ptr, size_t len)
+{
+  gtm_local_undo **local_undo = this->local_undo;
+  size_t i, n = this->n_local_undo;
+
+  if (n > 0)
+    {
+      for (i = n; i > 0; i--)
+	{
+	  gtm_local_undo *u = local_undo[i];
+	  /* ?? Do we need such granularity, or can we get away with
+	     just comparing PTR and LEN. ??  */
+	  if ((const char *)u->addr >= (const char *)ptr
+	      && ((const char *)u->addr + u->len <= (const char *)ptr + len))
+	    this->local_undo = NULL;
+	}
+    }
+}
+
 void ITM_REGPARM
 GTM_LB (const void *ptr, size_t len)
 {