This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[trans-mem] lots of irrevocability fixes


Hi Richard.

There are numerous fixes here.  I'm too lazy to submit individual
patches, but they're all somewhat related to irrevocability.  If you
mind terribly, I can submit them separately.

1. The bits we collect on transactions get cleared on execute_tm_mark (as
expected).  This means that irrevocability gets cleared and is hard to
collect again, unless we run IPA again.  Ideally we should do some sort
of IPA again to make sure that things like DCE didn't remove
irrevocability.  However, since we are certain not to introduce things
that do go irrevocable, it's safe to keep the irrevocability bit when we
clear the bits.  I have done so.

2. In doing #1, we can see that HAVE_CALL_IRREVOKABLE is more like
GTMA_MAY_ENTER_IRREVOKABLE.  I have renamed it throughout.

3. GIMPLE_CALLs may have a LHS to capture the return value.  If so, we
need to set GTMA_HAVE_STORE if we require a barrier.  This is also
fixed.

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.

5. A call to __tm_abort if -fgnu-tm is not used should not trigger a
segfault from the parser.  I have fixed this.

6. A direct call to __builtin__ITM_changeTransactionMode()
(BUILT_IN_TM_IRREVOKABLE) should by definition be irrevocable.  Duh.  I
have easily fixed this in is_tm_irrevokable().

7. Minor typographical cleanups.

I have added testcases for everything.

Regtested on x86-64 Linux for C/C++.

OK for branch?

p.s. On retrospect, maybe I should've submitted various patches.  But
this is easier :).

	* gimple-pretty-print.c (dump_gimple_tm_atomic_subcode): Add space
	after every GTMA_*.
	* trans-mem.c (is_tm_irrevokable): Return true for direct calls to
	the irrevocable builtin.
	(lower_sequence_tm): Rename GTMA_HAVE_CALL_IRREVOKABLE into
	GTMA_MAY_ENTER_IRREVOKABLE.
	(ipa_tm_insert_irr_call): Same.
	(expand_call_tm): Early return when is_tm_abort .
	Mark as GTMA_HAVE_STORE if the LHS of a call requires a barrier.
	(execute_tm_mark): Do not clear the GTMA_MAY_ENTER_IRREVOKABLE.
	(ipa_tm_insert_gettmclone_call): Set GTMA_MAY_ENTER_IRREVOKABLE
	unless safe.
	* gimple.h: Rename GTMA_HAVE_CALL_IRREVOKABLE into
	GTMA_MAY_ENTER_IRREVOKABLE.
	Add comments for GTMA_*IRREV*.
	* c-parser.c (c_parser_tm_abort): Complain if using tm_abort
	without TM enabled, and return a NOP_EXPR.

Index: testsuite/gcc.dg/tm/props-2.c
===================================================================
--- testsuite/gcc.dg/tm/props-2.c	(revision 0)
+++ testsuite/gcc.dg/tm/props-2.c	(revision 0)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa" } */
+
+/* Test that irrevocability gets set for the obvious case.  */
+
+int global;
+int george;
+
+extern crap() __attribute__((tm_irrevokable));
+
+foo(){
+    __tm_atomic {
+        global++;
+        crap();
+        george++;
+    }
+}
+
+/* { dg-final { scan-ipa-dump-times "GTMA_MAY_ENTER_IRREVOKABLE" 1 "tmipa" } } */
Index: testsuite/gcc.dg/tm/props-3.c
===================================================================
--- testsuite/gcc.dg/tm/props-3.c	(revision 0)
+++ testsuite/gcc.dg/tm/props-3.c	(revision 0)
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa" } */
+
+/* Test that indirect calls set the irrevocable bit.  */
+
+void (indirect)(void);
+
+extern crap() __attribute__((tm_irrevokable));
+
+foo(){
+    __tm_atomic {
+      (indirect)();
+    }
+}
+
+/* { dg-final { scan-ipa-dump-times "GTMA_MAY_ENTER_IRREVOKABLE" 1 "tmipa" } } */
Index: testsuite/gcc.dg/tm/abort-1.c
===================================================================
--- testsuite/gcc.dg/tm/abort-1.c	(revision 149958)
+++ testsuite/gcc.dg/tm/abort-1.c	(working copy)
@@ -3,4 +3,5 @@
 void f(void)
 {
   __tm_abort;		/* { dg-error "not within" } */
+  /* { dg-error "without trans" "" { target *-*-* } 5 } */
 }
Index: testsuite/gcc.dg/tm/abort-3.c
===================================================================
--- testsuite/gcc.dg/tm/abort-3.c	(revision 0)
+++ testsuite/gcc.dg/tm/abort-3.c	(revision 0)
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+
+void f(void)
+{
+  __tm_atomic { /* { dg-error "__tm_atomic. without" } */
+    __tm_abort; /* { dg-error "__tm_abort. without" } */
+  }
+}
Index: testsuite/gcc.dg/tm/irrevocable-2.c
===================================================================
--- testsuite/gcc.dg/tm/irrevocable-2.c	(revision 0)
+++ testsuite/gcc.dg/tm/irrevocable-2.c	(revision 0)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa" } */
+
+/* Test that a direct call to __builtin__ITM_changeTransactionMode()
+   sets the irrevocable bit.  */
+
+int global;
+int george;
+
+foo()
+{
+	__tm_atomic {
+		global++;
+		__builtin__ITM_changeTransactionMode ();
+		george++;
+	}
+}
+
+/* { dg-final { scan-ipa-dump-times "GTMA_MAY_ENTER_IRREVOKABLE" 1 "tmipa" } } */
Index: gimple-pretty-print.c
===================================================================
--- gimple-pretty-print.c	(revision 149958)
+++ gimple-pretty-print.c	(working copy)
@@ -1129,16 +1129,16 @@ dump_gimple_tm_atomic_subcode (pretty_pr
   if (subcode & GTMA_HAVE_LOAD)
     pp_string (buffer, "GTMA_HAVE_LOAD ");
   if (subcode & GTMA_HAVE_STORE)
-    pp_string (buffer, "GTMA_HAVE_STORE");
+    pp_string (buffer, "GTMA_HAVE_STORE ");
   if (subcode & GTMA_HAVE_CALL_TM)
-    pp_string (buffer, "GTMA_HAVE_CALL_TM");
-  if (subcode & GTMA_HAVE_CALL_IRREVOKABLE)
-    pp_string (buffer, "GTMA_HAVE_CALL_IRREVOKABLE");
+    pp_string (buffer, "GTMA_HAVE_CALL_TM ");
+  if (subcode & GTMA_MAY_ENTER_IRREVOKABLE)
+    pp_string (buffer, "GTMA_MAY_ENTER_IRREVOKABLE ");
   if (subcode & GTMA_MUST_CALL_IRREVOKABLE)
-    pp_string (buffer, "GTMA_MUST_CALL_IRREVOKABLE");
+    pp_string (buffer, "GTMA_MUST_CALL_IRREVOKABLE ");
   if (subcode & GTMA_HAVE_UNCOMMITTED_THROW)
-    pp_string (buffer, "GTMA_HAVE_UNCOMMITTED_THROW");
-  pp_string (buffer, " ]");
+    pp_string (buffer, "GTMA_HAVE_UNCOMMITTED_THROW ");
+  pp_string (buffer, "]");
 }
 
 /* Dump a GIMPLE_TM_ATOMIC tuple on the pretty_printer BUFFER.  */
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 149959)
+++ trans-mem.c	(working copy)
@@ -180,6 +180,16 @@ is_tm_irrevokable (tree x)
 
   if (attrs)
     return lookup_attribute ("tm_irrevokable", attrs) != NULL;
+
+  /* A call to the irrevocable builtin is by definition,
+     irrevocable.  */
+  if (TREE_CODE (x) == ADDR_EXPR)
+    x = TREE_OPERAND (x, 0);
+  if (TREE_CODE (x) == FUNCTION_DECL
+      && DECL_BUILT_IN_CLASS (x) == BUILT_IN_NORMAL
+      && DECL_FUNCTION_CODE (x) == BUILT_IN_TM_IRREVOKABLE)
+    return true;
+
   return false;
 }
 
@@ -606,7 +616,7 @@ lower_sequence_tm (gimple_stmt_iterator 
       break;
 
     case GIMPLE_ASM:
-      *state |= GTMA_HAVE_CALL_IRREVOKABLE;
+      *state |= GTMA_MAY_ENTER_IRREVOKABLE;
       break;
 
     case GIMPLE_TM_ATOMIC:
@@ -1091,6 +1101,7 @@ expand_call_tm (struct tm_region *region
 		gimple_stmt_iterator *gsi)
 {
   gimple stmt = gsi_stmt (*gsi);
+  tree lhs = gimple_call_lhs (stmt);
   tree fn_decl;
 
   if (is_tm_pure_call (stmt))
@@ -1098,13 +1109,19 @@ expand_call_tm (struct tm_region *region
 
   fn_decl = gimple_call_fndecl (stmt);
 
-  if (is_tm_abort (fn_decl))
-    tm_atomic_subcode_ior (region, GTMA_HAVE_ABORT);
-
   /* For indirect calls, we already generated a call into the runtime.  */
   if (!fn_decl)
     return false;
 
+  if (is_tm_abort (fn_decl))
+    {
+      tm_atomic_subcode_ior (region, GTMA_HAVE_ABORT);
+      return true;
+    }
+
+  if (lhs && requires_barrier (lhs))
+    tm_atomic_subcode_ior (region, GTMA_HAVE_STORE);
+
   return is_tm_ending_fndecl (fn_decl);
 }
 
@@ -1166,8 +1183,18 @@ execute_tm_mark (void)
   for (region = all_tm_regions; region ; region = region->next)
     if (region->exit_blocks)
       {
-	/* Collect a new SUBCODE set, now that optimizations are done.  */
+	unsigned int subcode
+	  = gimple_tm_atomic_subcode (region->tm_atomic_stmt);
+
+	/* 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.  */
+	gimple_tm_atomic_set_subcode (region->tm_atomic_stmt,
+				      subcode & GTMA_MAY_ENTER_IRREVOKABLE);
 
 	VEC_quick_push (basic_block, queue, region->entry_block);
 	do
@@ -1924,7 +1951,7 @@ ipa_tm_region_init (struct cgraph_node *
   return regions;
 }
 
-/* Create a copy the function (possibly declaration only) of OLD_NODE,
+/* Create a copy of the function (possibly declaration only) of OLD_NODE,
    appropriate for the transactional clone.  */
 
 static void
@@ -2011,7 +2038,7 @@ ipa_tm_insert_irr_call (struct cgraph_no
   gimple g;
   edge e;
 
-  tm_atomic_subcode_ior (region, GTMA_HAVE_CALL_IRREVOKABLE);
+  tm_atomic_subcode_ior (region, GTMA_MAY_ENTER_IRREVOKABLE);
 
   g = gimple_build_call (built_in_decls[BUILT_IN_TM_IRREVOKABLE], 0);
   add_stmt_to_tm_region (region, g);
@@ -2044,6 +2071,9 @@ ipa_tm_insert_gettmclone_call (struct cg
   ret = create_tmp_var (TREE_TYPE (old_fn), NULL);
   add_referenced_var (ret);
 
+  if (!safe)
+    tm_atomic_subcode_ior (region, GTMA_MAY_ENTER_IRREVOKABLE);
+
   /* Discard OBJ_TYPE_REF, since we weren't able to fold it.  */
   if (TREE_CODE (old_fn) == OBJ_TYPE_REF)
     old_fn = OBJ_TYPE_REF_EXPR (old_fn);
Index: gimple.h
===================================================================
--- gimple.h	(revision 149958)
+++ gimple.h	(working copy)
@@ -725,7 +725,14 @@ struct GTY(()) gimple_statement_omp_atom
 #define GTMA_HAVE_LOAD			(1u << 1)
 #define GTMA_HAVE_STORE			(1u << 2)
 #define GTMA_HAVE_CALL_TM		(1u << 3)
-#define GTMA_HAVE_CALL_IRREVOKABLE	(1u << 4)
+/* Atomic statement may enter serial irrevocable mode in its dynamic
+   scope.  */
+#define GTMA_MAY_ENTER_IRREVOKABLE	(1u << 4)
+/* An irrevocable block post-dominates the entire transaction, such
+   that all invocations of the transaction will go serial-irrevocable.
+   In such case, we don't bother instrumenting the transaction, and
+   tell the runtime that it should begin the transaction in
+   serial-irrevocable mode.  */
 #define GTMA_MUST_CALL_IRREVOKABLE	(1u << 5)
 #define GTMA_HAVE_UNCOMMITTED_THROW	(1u << 6)
 
Index: c-parser.c
===================================================================
--- c-parser.c	(revision 149958)
+++ c-parser.c	(working copy)
@@ -8645,6 +8645,9 @@ c_parser_tm_abort (c_parser *parser)
 
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_TM_ABORT));
 
+  if (!flag_tm)
+    error_at (loc,
+	      "%<__tm_abort%> without transactional memory support enabled");
   if (!parser->in_tm_atomic)
     {
       error_at (loc, "%<__tm_abort%> not within %<__tm_atomic%>");
@@ -8654,6 +8657,9 @@ c_parser_tm_abort (c_parser *parser)
 
   c_parser_consume_token (parser);
 
+  if (!flag_tm)
+    return build1 (NOP_EXPR, void_type_node, error_mark_node);
+
   return add_stmt (build_tm_abort_call (loc));
 }
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]