diff mbox

[trans-mem] optimize exception object memory

Message ID 20100719212255.GA12644@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez July 19, 2010, 9:22 p.m. UTC 
Hi.

Currently, when we throw an exception, any access to the exception
object gets instrumented, but we can avoid this instrumentation
because by definition, the exception object is local to the current
thread.  For that matter, we don't have to record anything at all,
because an exception restart will call revert_cpp_exceptions() which
will call __cxa_tm_cleanup() in the C++ runtime.  The
function__cxa_tm_cleanup in turn, will restore the newly allocated
exception object, thus making it unecessary to instrument anything.

For "throw 1", we currently generate:

  D.2200_6 = _ITM_cxa_allocate_exception (4);
  D.2201_7 = (int *) D.2200_6;
  D.2203_9 = 1;
  __builtin__ITM_WU4 (D.2201_7, D.2203_9);	// <-- unecessary

With the patch below we avoid the call to _ITM_WU4().

(Similarly for _ITM_cxa_begin_catch).

OK for branch?

	* trans-mem.c (thread_private_new_memory): Do not instrument
	_ITM_cxa_allocate_exception or _ITM_cxa_begin_catch.

Comments

Richard Henderson July 19, 2010, 10:25 p.m. UTC | #1 
On 07/19/2010 02:22 PM, Aldy Hernandez wrote:
> +      /* If the address escapes the function, it also escapes the thread.  */
> +      retval = mem_non_local;
> +
> +      /* Unlesss... it's one of our special thread private calls.  */
> +      if (stmt && is_gimple_call (stmt)
> +	  && (fndecl = gimple_call_fndecl (stmt)))
> +	{
> +	  const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
> +	  if (!strcmp (name, "_ITM_cxa_allocate_exception")
> +	      || !strcmp (name, "_ITM_cxa_begin_catch"))

Wouldn't it be better to invent a new attribute that users could put
on their own functions as well?  I'm not sure what the best name might
be, but perhaps just "thread_local"?

The commentary would then be

  /* If the address has been declared to be thread-local, believe it.  */
...
  /* Otherwise, we have to assume that any address that escapes the
     function also escapes the thread.  */

Bonus points if we can figure out a way to apply this idea to 
function arguments, as opposed to just function returns as above.


r~
diff mbox

Patch

Index: testsuite/g++.dg/tm/throw.C
===================================================================
--- testsuite/g++.dg/tm/throw.C	(revision 0)
+++ testsuite/g++.dg/tm/throw.C	(revision 0)
@@ -0,0 +1,18 @@ 
+// { dg-do compile }
+// { dg-options "-fgnu-tm -O -fdump-tree-optimized" }
+
+// Test that we don't instrument the result from
+// _ITM_cxa_allocate_exception().
+
+static void throwit() {
+        throw 1; 
+}
+
+void tranfunc() {
+        __transaction {
+                throwit();
+        }
+}
+
+// { dg-final { scan-tree-dump-times "_ITM_WU" 0 "optimized" } }
+// { dg-final { cleanup-tree-dump "optimized" } }
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 161318)
+++ trans-mem.c	(working copy)
@@ -1258,6 +1258,7 @@  thread_private_new_memory (basic_block e
   tm_new_mem_map_t elt, *elt_p;
   tree val = x;
   enum thread_memory_type retval = mem_transaction_local;
+  bool escapes_function_p = false;
 
   if (!entry_block
       || TREE_CODE (x) != SSA_NAME
@@ -1283,17 +1284,14 @@  thread_private_new_memory (basic_block e
   do
     {
       if (ptr_deref_may_alias_global_p (x))
-	{
-	  /* Address escapes.  This is not thread-private.  */
-	  retval = mem_non_local;
-	  goto new_memory_ret;
-	}
+	escapes_function_p = true;
 
       stmt = SSA_NAME_DEF_STMT (x);
 
       /* If the malloc call is outside the transaction, this is
 	 thread-local.  */
       if (retval != mem_thread_local
+	  && gimple_code (stmt) != GIMPLE_NOP
 	  && !dominated_by_p (CDI_DOMINATORS, gimple_bb (stmt), entry_block))
 	retval = mem_thread_local;
 
@@ -1357,6 +1355,33 @@  thread_private_new_memory (basic_block e
     retval = mem_non_local;
 
  new_memory_ret:
+  if (escapes_function_p)
+    {
+      tree fndecl;
+
+      /* If the address escapes the function, it also escapes the thread.  */
+      retval = mem_non_local;
+
+      /* Unlesss... it's one of our special thread private calls.  */
+      if (stmt && is_gimple_call (stmt)
+	  && (fndecl = gimple_call_fndecl (stmt)))
+	{
+	  const char *name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
+	  if (!strcmp (name, "_ITM_cxa_allocate_exception")
+	      || !strcmp (name, "_ITM_cxa_begin_catch"))
+	    {
+	      retval = mem_transaction_local;
+	      /* Set MEM_TRANSACTION_LOCAL which will get no
+		 instrumentation for this address.
+
+		 Even though we're technically thread private, treat
+		 as transaction local, because a transaction restart
+		 will call __cxa_tm_cleanup() which will cleanup any
+		 (exception) memory allocated.  */
+	    }
+	}
+    }
+
   elt_p->local_new_memory = retval;
   return retval;
 }