diff mbox

[trans-mem] PR46270: handle throwing statements

Message ID 4CE6B182.4040106@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Nov. 19, 2010, 5:18 p.m. UTC 
On 11/19/10 11:26, Richard Henderson wrote:
>> +      if (!gimple_call_nothrow_p (stmt)
>> +	&&  gsi_one_before_end_p (*gsi))
>>      
> As I said before: stmt_can_throw_internal.  You're doing
> unnecessary splitting in the case the call *can* throw,
> but there's no handler within the current function.
>    

Poop, missed that.  Thanks.
>> +	  gsi_insert_on_edge_immediate (fallthru_edge, stmt);
>> +
>> +	  /* Split the block so the instrumentation code resides in
>> +	     it's own separate BB.  Then we can mark this BB as handled
>> +	     and avoid rescanning it again.  */
>> +	  bb = gimple_bb (stmt);
>> +	  split_block (bb, stmt);
>> +	  *gsi = gsi_start_bb (bb);
>>      
> Splitting the block is pointless.  If you don't want to process
>    
Watcha talking about Willis?  If I split it, it ends up all by itself, 
and I can mark it quite nicely.  Not that it's elegant, or anything...  
Just curious, why is it pointless?

> the statement, and you're worried about it being placed into a
> block that you've not yet processed, then use the non-immediate
> form of edge insertion and then use gsi_commit_edge_inserts at
> the end.
>
> Normally when we do that, we also accumulate a flag that says
> whether calling gsi_commit_edge_inserts is necessary.
>    

Sigh, I'm beginning to see that there is no elegant solution.  Yuck, one 
more flag.

This is all nasty, but here it is...

OK?
PR/46270
	* trans-mem.c (struct tm_region): Add pending_edge_inserts_p.
	(tm_region_init_0): Initialize pending_edge_inserts_p.
	(expand_call_tm): Handle throwing calls.
	(execute_tm_mark): Handle clones and transactions the same.
	Flush pending statements if necessary.

Comments

Richard Henderson Nov. 19, 2010, 5:47 p.m. UTC | #1 
> +  region->pending_edge_inserts_p = false;

You don't want to put this flag on the region.  You want to do this
edge insertion once per function, not once per region.  I.e. outside
that big loop in execute_tm_mark.

The easiest thing is to make the flag file-scope static, cleared at
the beginning of execute_tm_mark.

Ok with that change.


r~
diff mbox

Patch

Index: testsuite/g++.dg/tm/pr46270.C
===================================================================
--- testsuite/g++.dg/tm/pr46270.C	(revision 0)
+++ testsuite/g++.dg/tm/pr46270.C	(revision 0)
@@ -0,0 +1,27 @@ 
+// { dg-do compile }
+// { dg-options "-fgnu-tm" }
+
+#include <list>
+class Game
+{
+public:
+  struct BuildProject
+  {
+    int posX;
+  };
+  std::list<BuildProject> buildProjects;
+};
+
+static Game game;
+static std::list<std::list<Game::BuildProject>::iterator> erasableBuildProjects;
+
+static void *buildProjectSyncStepConcurrently(int id, int localTeam)
+{
+  __transaction [[relaxed]] {
+    std::list<std::list<Game::BuildProject>::iterator>::iterator it
+      = erasableBuildProjects.begin();
+    game.buildProjects.erase( (std::list<Game::BuildProject> 
+			       ::iterator) *it);
+  }
+  return 0;
+}
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 166872)
+++ trans-mem.c	(working copy)
@@ -1678,6 +1678,9 @@  struct tm_region
 
   /* The set of all blocks that have an TM_IRREVOCABLE call.  */
   bitmap irr_blocks;
+
+  /* True if there are pending edge statements to be committed.  */
+  bool pending_edge_inserts_p;
 };
 
 static struct tm_region *all_tm_regions;
@@ -1707,6 +1710,7 @@  tm_region_init_0 (struct tm_region *oute
     }
   region->inner = NULL;
   region->outer = outer;
+  region->pending_edge_inserts_p = false;
 
   region->transaction_stmt = stmt;
 
@@ -2173,13 +2177,44 @@  expand_call_tm (struct tm_region *region
     {
       tree tmp = make_rename_temp (TREE_TYPE (lhs), NULL);
       location_t loc = gimple_location (stmt);
+      edge fallthru_edge = NULL;
+
+      /* Remember if the call was going to throw.  */
+      if (stmt_can_throw_internal (stmt))
+	{
+	  edge_iterator ei;
+	  edge e;
+	  basic_block bb = gimple_bb (stmt);
+
+	  FOR_EACH_EDGE (e, ei, bb->succs)
+	    if (e->flags & EDGE_FALLTHRU)
+	      {
+		fallthru_edge = e;
+		break;
+	      }
+	}
 
       gimple_call_set_lhs (stmt, tmp);
       update_stmt (stmt);
       stmt = gimple_build_assign (lhs, tmp);
       gimple_set_location (stmt, loc);
-      gsi_insert_after (gsi, stmt, GSI_CONTINUE_LINKING);
-      expand_assign_tm (region, gsi);
+
+      /* We cannot throw in the middle of a BB.  If the call was going
+	 to throw, place the instrumentation on the fallthru edge, so
+	 the call remains the last statement in the block.  */
+      if (fallthru_edge)
+	{
+	  gimple_seq fallthru_seq = gimple_seq_alloc_with_stmt (stmt);
+	  gimple_stmt_iterator fallthru_gsi = gsi_start (fallthru_seq);
+	  expand_assign_tm (region, &fallthru_gsi);
+	  gsi_insert_seq_on_edge (fallthru_edge, fallthru_seq);
+	  region->pending_edge_inserts_p = true;
+	}
+      else
+	{
+	  gsi_insert_after (gsi, stmt, GSI_CONTINUE_LINKING);
+	  expand_assign_tm (region, gsi);
+	}
 
       transaction_subcode_ior (region, GTMA_HAVE_STORE);
     }
@@ -2304,28 +2339,18 @@  execute_tm_mark (void)
 	  else
 	    subcode &= GTMA_DECLARATION_MASK;
 	  gimple_transaction_set_subcode (region->transaction_stmt, subcode);
-
-	  queue = get_tm_region_blocks (region->entry_block,
-					region->exit_blocks,
-					region->irr_blocks,
-					/*stop_at_irr_p=*/true);
-	  for (i = 0; VEC_iterate (basic_block, queue, i, bb); ++i)
-	    expand_block_tm (region, bb);
-	  VEC_free (basic_block, heap, queue);
-	}
-      else
-	/* ...otherwise, we're a clone and the entire function is the
-	   region.  */
-	{
-	  FOR_EACH_BB (bb)
-	    {
-	      /* Stop at irrevocable blocks.  */
-	      if (region->irr_blocks
-		  && bitmap_bit_p (region->irr_blocks, bb->index))
-		break;
-	      expand_block_tm (region, bb);
-	    }
 	}
+
+      queue = get_tm_region_blocks (region->entry_block,
+				    region->exit_blocks,
+				    region->irr_blocks,
+				    /*stop_at_irr_p=*/true);
+      for (i = 0; VEC_iterate (basic_block, queue, i, bb); ++i)
+	expand_block_tm (region, bb);
+      VEC_free (basic_block, heap, queue);
+      if (region->pending_edge_inserts_p)
+	gsi_commit_edge_inserts ();
+
       tm_log_emit ();
     }