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]

Re: [trans-mem] fix buglet in expand_assign_tm


Well, Richard okayed the patch but didn't like it, something about a
pass ordering problem which I was sure to be on the hook to fix later.
So might as well fix it correctly now...

The problem here is that if we instrument a store whose LHS is thread
private memory, but whose RHS requires expansion (to an instrumented
variant), we will mark the LHS for logging using the original statement,
but this statement will be removed/replaced during the RHS expansion.

Instead of inserting the unpopular nops in my previous patch, the
following patch calls requires_barrier twice, once without making any
changes to the log, and once if/after we have expanded the assignment,
thus picking up the correct statement (which won't disappear ;-)).

Simpler, cleaner.

OK for branch?

	* trans-mem.c (requires_barrier): Document STMT argument.
	(expand_assign_tm): Call requires_barrier after assignment has
	been expanded.

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 154493)
+++ trans-mem.c	(working copy)
@@ -1134,7 +1134,11 @@ static tree lower_sequence_no_tm (gimple
    or write barrier.
 
    ENTRY_BLOCK is the entry block for the region where stmt resides
-   in.  NULL if unknown.  */
+   in.  NULL if unknown.
+
+   STMT is the statement in which X occurs in.  It is used for thread
+   private memory instrumentation.  If no TPM instrumentation is
+   desired, STMT should be null.  */
 static bool
 requires_barrier (basic_block entry_block, tree x, gimple stmt)
 {
@@ -1739,12 +1743,14 @@ 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, NULL);
   bool load_p = requires_barrier (region->entry_block, rhs, NULL);
   gimple gcall;
 
   if (!load_p && !store_p)
     {
+      /* Add thread private addresses to log if applicable.  */
+      requires_barrier (region->entry_block, lhs, stmt);
       gsi_next (gsi);
       return;
     }
@@ -1774,6 +1780,11 @@ expand_assign_tm (struct tm_region *regi
       gcall = build_tm_store (lhs, rhs, gsi);
     }
 
+  /* Now that we have the load/store in its instrumented form, add
+     thread private addresses to the log if applicable.  */
+  if (!store_p)
+    requires_barrier (region->entry_block, lhs, gcall);
+
   /* add_stmt_to_tm_region  (region, gcall); */
 }
 


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