Patchwork [trans-mem] PR45940: handle pure functions with inline asms

login
register
mail settings
Submitter Aldy Hernandez
Date Nov. 29, 2010, 3:35 p.m.
Message ID <20101129153556.GA14709@redhat.com>
Download mbox | patch
Permalink /patch/73435/
State New
Headers show

Comments

Aldy Hernandez - Nov. 29, 2010, 3:35 p.m.
The problem here is that we see an inline asm and mark the function as
irrevocable, even if the user said it was transaction_pure.  The patch
below makes the compiler believe the user.

The relevant part of the patch is actually this:

@@ -736,7 +742,7 @@ diagnose_tm_blocks (void)

   /* If we saw something other than a call that makes this function
      unsafe, remember it so that the IPA pass only needs to scan calls.
*/
-  if (d.saw_unsafe && !is_tm_safe (current_function_decl))
+  if (d.saw_unsafe && !is_tm_safe_or_pure (current_function_decl))
     cgraph_local_info (current_function_decl)->tm_may_enter_irr = 1;

The rest is syntactic sugar providing is_tm_safe_or_pure() since it
happens so often.  Plus, all the is_tm_* machinery is only used in
trans-mem.c, so I made them static.

OK for branch?

p.s. This only fixes one of the two bugs reported in this PR.  If you
run this testcase with -O1, you still get an unrelated ICE, which I'm
investigating.

        * tree.h: Remove prototypes for is_tm_pure, is_tm_safe,
        is_tm_callable, is_tm_irrevocable.
        * trans-mem.c (is_tm_pure): Make static.
        (is_tm_irrevocable): Same.
        (is_tm_callable): Same.
        (is_tm_safe): Same.
        (is_tm_safe_or_pure): New.
        (diagnose_tm_1): Use is_tm_safe_or_pure.
	(diagnose_tm_blocks): Same.
        (ipa_tm_note_irrevocable): Same.
        (ipa_tm_mayenterirr_function): Same.
        (ipa_tm_execute): Same.

Patch

Index: tree.h
===================================================================
--- tree.h	(revision 166496)
+++ tree.h	(working copy)
@@ -5388,10 +5388,6 @@  extern tree build_personality_function (
 
 /* In trans-mem.c.  */
 extern tree build_tm_abort_call (location_t, bool);
-extern bool is_tm_safe (tree);
-extern bool is_tm_pure (tree);
-extern bool is_tm_callable (tree);
-extern bool is_tm_irrevocable (tree);
 extern bool is_tm_may_cancel_outer (tree);
 extern void record_tm_replacement (tree, tree);
 extern void tm_malloc_replacement (tree);
Index: testsuite/g++.dg/pr45940.C
===================================================================
--- testsuite/g++.dg/pr45940.C	(revision 0)
+++ testsuite/g++.dg/pr45940.C	(revision 0)
@@ -0,0 +1,30 @@ 
+// { dg-do compile }
+// { dg-options "-fgnu-tm -O0" }
+
+__attribute__((transaction_pure))
+inline int atomic_exchange_and_add(int dv )
+{
+    int r;
+    __asm__ ("" : "=r"(r));
+    return r;
+}
+
+class sp_counted_base
+{
+public:   
+    __attribute__((transaction_safe))
+    void release()
+    {   
+        if( atomic_exchange_and_add(-1 ) == 1 )
+        {
+        }
+    }
+};
+
+sp_counted_base *base;
+
+void here(){
+  __transaction[[atomic]] {
+    base->release();
+  }
+}
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 167111)
+++ trans-mem.c	(working copy)
@@ -173,7 +173,7 @@  get_attrs_for (tree x)
 
 /* Return true if X has been marked TM_PURE.  */
 
-bool
+static bool
 is_tm_pure (tree x)
 {
   tree attrs = get_attrs_for (x);
@@ -184,7 +184,7 @@  is_tm_pure (tree x)
 
 /* Return true if X has been marked TM_IRREVOCABLE.  */
 
-bool
+static bool
 is_tm_irrevocable (tree x)
 {
   tree attrs = get_attrs_for (x);
@@ -206,7 +206,7 @@  is_tm_irrevocable (tree x)
 
 /* Return true if X has been marked TM_SAFE.  */
 
-bool
+static bool
 is_tm_safe (tree x)
 {
   tree attrs = get_attrs_for (x);
@@ -222,6 +222,12 @@  is_tm_safe (tree x)
   return false;
 }
 
+static inline bool
+is_tm_safe_or_pure (tree x)
+{
+  return is_tm_safe (x) || is_tm_pure (x);
+}
+
 /* Return true if CALL is const, or tm_pure.  */
 
 static bool
@@ -247,7 +253,7 @@  is_tm_pure_call (gimple call)
 
 /* Return true if X has been marked TM_CALLABLE.  */
 
-bool
+static bool
 is_tm_callable (tree x)
 {
   tree attrs = get_attrs_for (x);
@@ -587,7 +593,7 @@  diagnose_tm_1 (gimple_stmt_iterator *gsi
 		replacement = NULL_TREE;
 	      }
 
-	    if (is_tm_safe (fn) || is_tm_pure (fn))
+	    if (is_tm_safe_or_pure (fn))
 	      is_safe = true;
 	    else if (is_tm_callable (fn) || is_tm_irrevocable (fn))
 	      {
@@ -736,7 +742,7 @@  diagnose_tm_blocks (void)
 
   /* If we saw something other than a call that makes this function
      unsafe, remember it so that the IPA pass only needs to scan calls.  */
-  if (d.saw_unsafe && !is_tm_safe (current_function_decl))
+  if (d.saw_unsafe && !is_tm_safe_or_pure (current_function_decl))
     cgraph_local_info (current_function_decl)->tm_may_enter_irr = 1;
 
   return 0;
@@ -3495,8 +3501,7 @@  ipa_tm_note_irrevocable (struct cgraph_n
 	continue;
       /* Even if we think we can go irrevocable, believe the user
 	 above all.  */
-      if (is_tm_safe (e->caller->decl)
-	  || is_tm_pure (e->caller->decl))
+      if (is_tm_safe_or_pure (e->caller->decl))
 	continue;
       if (gimple_call_in_transaction_p (e->call_stmt))
 	d->want_irr_scan_normal = true;
@@ -3792,9 +3797,9 @@  ipa_tm_mayenterirr_function (struct cgra
   tree decl = node->decl;
 
   /* Filter out all functions that are marked.  */
-  if (is_tm_safe (decl))
+  if (is_tm_safe_or_pure (decl))
     return false;
-  if (is_tm_pure (decl) || (flags_from_decl_or_type (decl) & ECF_CONST) != 0)
+  if ((flags_from_decl_or_type (decl) & ECF_CONST) != 0)
     return false;
   if (is_tm_irrevocable (decl))
     return true;
@@ -4443,8 +4448,7 @@  ipa_tm_execute (void)
       if (is_tm_irrevocable (node->decl))
 	ipa_tm_note_irrevocable (node, &worklist);
       else if (a <= AVAIL_NOT_AVAILABLE
-	       && !is_tm_safe (node->decl)
-	       && !is_tm_pure (node->decl))
+	       && !is_tm_safe_or_pure (node->decl))
 	ipa_tm_note_irrevocable (node, &worklist);
       else if (a >= AVAIL_OVERWRITABLE)
 	{
@@ -4512,8 +4516,7 @@  ipa_tm_execute (void)
       node->local.tm_may_enter_irr = true;
 
       for (e = node->callers; e ; e = e->next_caller)
-	if (!is_tm_safe (e->caller->decl)
-	    && !is_tm_pure (e->caller->decl)
+	if (!is_tm_safe_or_pure (e->caller->decl)
 	    && !e->caller->local.tm_may_enter_irr)
 	  maybe_push_queue (e->caller, &worklist, &d->in_worklist);
     }