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] fix buglet in expand_assign_tm


Yo!

tm_log_add() saves the store statement so it can insert the call to the
logging function right before it (if applicable).  This statement can be
removed from under us in expand_assign_tm.  Needless to say, when we try
to emit the logging function call, we're a bit lost.

I've fixed this by inserting a nop where the original store was, which
we can then use as a reference to insert the logging call.  This fixes
the ICE we were getting on large loads.

Worry not, the nop gets cleaned up right before we exit the pass.

OK for branch?

	* trans-mem.c (tm_log_add): Remove nop marker.
	(tm_log_emit_stmt): Same.
	(requires_barrier): Insert nop as a marker.
	(expand_assign_tm): Call requires_barrier with gsi.
	(expand_call_tm): Same.

Index: testsuite/gcc.dg/tm/memopt-6.c
===================================================================
--- testsuite/gcc.dg/tm/memopt-6.c	(revision 0)
+++ testsuite/gcc.dg/tm/memopt-6.c	(revision 0)
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -O -fdump-tree-tmedge --param tm-max-aggregate-size=1" } */
+
+struct large { int x[100]; };
+struct large bark();
+extern int test (void) __attribute__((transaction_safe));
+struct large lacopy;
+
+int f() /* { dg-message "unimplemented: transactional load" } */
+{
+  int i = readint();
+  struct large lala = bark();
+  __transaction {
+    lala.x[55] = 666;
+    lala = lacopy;
+  }
+  return lala.x[i];
+}
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 154307)
+++ trans-mem.c	(working copy)
@@ -866,6 +866,8 @@ tm_log_add (basic_block entry_block, tre
 	     special constructors and the like.  */
 	  && !TREE_ADDRESSABLE (type))
 	{
+	  gimple_stmt_iterator gsi;
+
 	  lp->save_var = create_tmp_var (TREE_TYPE (lp->addr), "tm_save");
 	  add_referenced_var (lp->save_var);
 	  lp->stmts = NULL;
@@ -873,6 +875,10 @@ tm_log_add (basic_block entry_block, tre
 	     get confused by overlapping addresses in the save/restore
 	     sequence.  */
 	  VEC_safe_push (tree, heap, tm_log_save_addresses, lp->addr);
+
+	  /* This marker is only useful when inserting logging calls.  */
+	  gsi = gsi_for_stmt (stmt);
+	  gsi_remove (&gsi, true);
 	}
       else
 	{
@@ -963,6 +969,9 @@ tm_log_emit_stmt (tree addr, gimple stmt
 	break;
       }
 
+  gcc_assert (gimple_nop_p (stmt));
+  gsi_remove (&gsi, true);
+
   addr = gimplify_addr (&gsi, addr);
   if (code == BUILT_IN_TM_LOG)
     log = gimple_build_call (built_in_decls[code], 2, addr,  size);
@@ -1125,10 +1134,14 @@ static tree lower_sequence_no_tm (gimple
 /* Determine whether X has to be instrumented using a read
    or write barrier.
 
-   ENTRY_BLOCK is the entry block for the region where stmt resides
-   in.  NULL if unknown.  */
+   ENTRY_BLOCK is the entry block for the region where the stmt
+   resides in.  NULL if unknown.
+
+   GSI is the gsi for the statement we are examining.  If no
+   thread-private memory optimization logging should be done, GSI
+   should be NULL.  */
 static bool
-requires_barrier (basic_block entry_block, tree x, gimple stmt)
+requires_barrier (basic_block entry_block, tree x, gimple_stmt_iterator *gsi)
 {
   tree orig = x;
   while (handled_component_p (x))
@@ -1175,8 +1188,15 @@ requires_barrier (basic_block entry_bloc
 	     the transaction and restore on restart, or call a tm
 	     function to dynamically save and restore on restart
 	     (ITM_L*).  */
-	  if (stmt)
-	    tm_log_add (entry_block, orig, stmt);
+	  if (gsi)
+	    {
+	      /* Use a nop to mark the spot because the current
+		 statement may be swiped from under us in
+		 expand_assign_tm.  */
+	      gimple stmt = gimple_build_nop ();
+	      gsi_insert_before (gsi, stmt, GSI_SAME_STMT);
+	      tm_log_add (entry_block, orig, stmt);
+	    }
 	  return false;
 	}
 
@@ -1731,7 +1751,7 @@ expand_assign_tm (struct tm_region *regi
   gimple stmt = gsi_stmt (*gsi);
   tree lhs = gimple_assign_lhs (stmt);
   tree rhs = gimple_assign_rhs1 (stmt);
-  bool store_p = requires_barrier (region->entry_block, lhs, stmt);
+  bool store_p = requires_barrier (region->entry_block, lhs, gsi);
   bool load_p = requires_barrier (region->entry_block, rhs, NULL);
   gimple gcall;
 
@@ -1815,7 +1835,7 @@ expand_call_tm (struct tm_region *region
       return true;
     }
 
-  if (lhs && requires_barrier (region->entry_block, lhs, stmt))
+  if (lhs && requires_barrier (region->entry_block, lhs, gsi))
     transaction_subcode_ior (region, GTMA_HAVE_STORE);
 
   return is_tm_ending_fndecl (fn_decl);


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