Re: [trans-mem] lots of irrevocability fixes

[Aldy Hernandez <aldyh at redhat dot com>]

Ok, new patch with your suggestions.

> On 07/23/2009 10:37 AM, Aldy Hernandez wrote:
>> 4. An indirect call within a transaction may go irrevocable, so any time
>> we call the runtime on ipa_tm_insert_gettmclone_call() we should mark it
>> so.
>
> In addition, a direct call to a tm_callable function may
> go irrevocable.  Calls to tm_safe and tm_pure may not.
> I'm not sure what the situation is for tm_wrap functions;
> perhaps just consider them as tm_callable for now.

Fixed and fixed.  I've added a bit to cgraph to mark tm_wrap functions.

> We don't need to run IPA again, we just need to tag each function
> so that we remember whether or not it must/can go irrevocable.
>
> Perhaps some bits on cgraph_local_info would be appropriate?
> We can probably remove the DECL_IS_TM_CLONE tree bit at the
> same time.

Fixed comment and am working on the cgraph bits for irrevocability.
I'll submit that when I get this one in.

OK for branch?

	* cgraph.h (cgraph_local_info): Add tm_wrappee.
	* trans-mem.c (execute_tm_mark): Fix comment.
	(ipa_tm_transform_calls): Set irrevocability bit for tm_callable
	and tm_wrap functions.
	(is_tm_wrap): New.
	(record_tm_replacement): Set tm_wrappee.

Index: cgraph.h
===================================================================
--- cgraph.h	(revision 150023)
+++ cgraph.h	(working copy)
@@ -98,6 +98,10 @@ struct GTY(()) cgraph_local_info {
   /* True if the function is going to be emitted in some other translation
      unit, referenced from vtable.  */
   unsigned vtable_method : 1;
+
+  /* True if the function was created with the tm_wrap attribute, so
+     it is the function to use in the substitution.  */
+  unsigned tm_wrappee : 1;
 };
 
 /* Information about the function that needs to be computed globally
Index: testsuite/gcc.dg/tm/irrevocable-4.c
===================================================================
--- testsuite/gcc.dg/tm/irrevocable-4.c	(revision 0)
+++ testsuite/gcc.dg/tm/irrevocable-4.c	(revision 0)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa" } */
+
+extern void bar(void) __attribute__((tm_callable));
+void orig(void);
+void xyz(void) __attribute__((tm_wrap (orig)));
+
+
+foo()
+{
+	__tm_atomic orig();
+}
+
+/* { dg-final { scan-ipa-dump-times "GTMA_MAY_ENTER_IRREVOKABLE" 1 "tmipa" } } */
Index: testsuite/gcc.dg/tm/irrevocable-3.c
===================================================================
--- testsuite/gcc.dg/tm/irrevocable-3.c	(revision 0)
+++ testsuite/gcc.dg/tm/irrevocable-3.c	(revision 0)
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa" } */
+
+extern void bar(void) __attribute__((tm_callable));
+
+foo()
+{
+	__tm_atomic bar();
+}
+
+/* { dg-final { scan-ipa-dump-times "GTMA_MAY_ENTER_IRREVOKABLE" 1 "tmipa" } } */
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 150023)
+++ trans-mem.c	(working copy)
@@ -246,6 +246,15 @@ is_tm_callable (tree x)
   return false;
 }
 
+/* Return true if X is a wrappee for another function.  */
+
+bool
+is_tm_wrap (tree fndecl)
+{
+  struct cgraph_local_info *local = cgraph_local_info (fndecl);
+  return local->tm_wrappee;
+}
+
 /* Return true if STMT may alter control flow via a transactional edge.  */
 
 bool
@@ -317,6 +326,9 @@ void
 record_tm_replacement (tree from, tree to)
 {
   struct tree_map **slot, *h;
+  struct cgraph_local_info *local = cgraph_local_info (from);
+
+  local->tm_wrappee = true;
 
   if (tm_wrap_map == NULL)
     tm_wrap_map = htab_create_ggc (32, tree_map_hash, tree_map_eq, 0);
@@ -1188,11 +1200,17 @@ execute_tm_mark (void)
 
 	/* Collect a new SUBCODE set, now that optimizations are done...  */
 	gimple_tm_atomic_set_subcode (region->tm_atomic_stmt, 0);
-	/* ...but keep the bits that require IPA to collect.  Ideally
-	   we should do IPA again to make sure things like DCE didn't
-	   invalidate irrevocability, but we are certain not to
-	   introduce things that go irrevocable, so it's safe to keep
-	   the irrevocability bit.  */
+	/* ...but keep the irrevocability bit.  Ideally we should
+	   remember irrevocability on a per-function basis, to make
+	   sure things like DCE didn't invalidate it.  However, since
+	   we are certain not to introduce things that go irrevocable,
+	   it's safe to preserve the bit.
+
+	   ?? Tag each function so that we remember whether or not it
+	   must/can go irrevocable.  Perhaps some bits on
+	   cgraph_local_info.  Remove the DECL_IS_TM_CLONE tree bit
+	   simultaneously.
+	*/
 	gimple_tm_atomic_set_subcode (region->tm_atomic_stmt,
 				      subcode & GTMA_MAY_ENTER_IRREVOKABLE);
 
@@ -2155,6 +2173,10 @@ ipa_tm_transform_calls (struct cgraph_no
 	  continue;
 	}
 
+      if (is_tm_callable (fndecl)
+	  || is_tm_wrap (fndecl))
+	tm_atomic_subcode_ior (region, GTMA_MAY_ENTER_IRREVOKABLE);
+
       /* Don't scan past the end of the transaction.  */
       if (is_tm_ending_fndecl (fndecl))
 	continue;