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]

Remove prev_try pointer from ehcleanup region datastructure


Hi,
since I added prev_try verification to the verify_eh_tree, I am
constantly getting new side cases where prev_try ends up being wrong.
This pointer is used only at once place: in foreach_reachable_handler
to skip cleanup regions after first region is found (since CFG will have
RESX edges so dirrect edges are not neccesary) and as found by my
previous patch it does not serve its purpose well anyway.

It is a lot easier to just walk the tree here (via find_prev_try) and I
can't imagine this causing noticeable slowdowns.  I benchmarked build of
libstdc++ and Ada and found no difference.  It made sense when EH tree
was largely static datastructure built once by fronend, but now when
it is being dynamically updated all the time, it is not doing any good
job.

Botstrapped/regtested x86_64-linux, OK?

Honza

	* tree-eh.c (struct leh_state): Remove prev_try.
	(lower_try_finally, lower_catch, lower_eh_filter, lower_cleanup): Do
	not track prev_try.
	* except.c (gen_eh_region_cleanup, duplicate_eh_regions, 
	copy_eh_region_1, copy_eh_region, redirect_eh_edge_to_label,
	remove_eh_handler_and_replace, foreach_reachable_handler,
	verify_eh_region, verify_eh_tree): Remove tracking of prev_try pointer.
	* except.h (struct eh_region): Remove eh_region_u_cleanup.
	(gen_eh_region_cleanup): Update prototype.
Index: tree-eh.c
===================================================================
--- tree-eh.c	(revision 147317)
+++ tree-eh.c	(working copy)
@@ -352,7 +352,6 @@ struct leh_state
      correspond to variables of the same name in cfun->eh, which we
      don't have easy access to.  */
   struct eh_region *cur_region;
-  struct eh_region *prev_try;
 
   /* Processing of TRY_FINALLY requires a bit more state.  This is
      split out into a separate structure so that we don't have to
@@ -1566,12 +1565,11 @@ lower_try_finally (struct leh_state *sta
   this_tf.outer = state;
   if (using_eh_for_cleanups_p)
     this_tf.region
-      = gen_eh_region_cleanup (state->cur_region, state->prev_try);
+      = gen_eh_region_cleanup (state->cur_region);
   else
     this_tf.region = NULL;
 
   this_state.cur_region = this_tf.region;
-  this_state.prev_try = state->prev_try;
   this_state.tf = &this_tf;
 
   lower_eh_constructs_1 (&this_state, gimple_try_eval(tp));
@@ -1650,7 +1648,6 @@ lower_catch (struct leh_state *state, gi
 
   try_region = gen_eh_region_try (state->cur_region);
   this_state.cur_region = try_region;
-  this_state.prev_try = try_region;
   this_state.tf = state->tf;
 
   lower_eh_constructs_1 (&this_state, gimple_try_eval (tp));
@@ -1672,7 +1669,6 @@ lower_catch (struct leh_state *state, gi
                                           gimple_catch_types (gcatch));
 
       this_state.cur_region = catch_region;
-      this_state.prev_try = state->prev_try;
       lower_eh_constructs_1 (&this_state, gimple_catch_handler (gcatch));
 
       eh_label = create_artificial_label ();
@@ -1719,10 +1715,6 @@ lower_eh_filter (struct leh_state *state
 					 gimple_eh_filter_types (inner));
   this_state = *state;
   this_state.cur_region = this_region;
-  /* For must not throw regions any cleanup regions inside it
-     can't reach outer catch regions.  */
-  if (gimple_eh_filter_must_not_throw (inner))
-    this_state.prev_try = NULL;
 
   lower_eh_constructs_1 (&this_state, gimple_try_eval (tp));
 
@@ -1759,7 +1751,7 @@ lower_cleanup (struct leh_state *state, 
       return result;
     }
 
-  this_region = gen_eh_region_cleanup (state->cur_region, state->prev_try);
+  this_region = gen_eh_region_cleanup (state->cur_region);
   this_state = *state;
   this_state.cur_region = this_region;
 
Index: except.c
===================================================================
--- except.c	(revision 147317)
+++ except.c	(working copy)
@@ -338,10 +338,9 @@ gen_eh_region (enum eh_region_type type,
 }
 
 struct eh_region *
-gen_eh_region_cleanup (struct eh_region *outer, struct eh_region *prev_try)
+gen_eh_region_cleanup (struct eh_region *outer)
 {
   struct eh_region *cleanup = gen_eh_region (ERT_CLEANUP, outer);
-  cleanup->u.cleanup.prev_try = prev_try;
   return cleanup;
 }
 
@@ -1183,8 +1182,8 @@ duplicate_eh_regions_1 (eh_region old, e
   return ret;
 }
 
-/* Return prev_try pointers catch subregions of R should
-   point to.  */
+/* Look for first outer region of R (or R itself) that is
+   TRY region. Return NULL if none.  */
 
 static struct eh_region *
 find_prev_try (struct eh_region * r)
@@ -1208,7 +1207,7 @@ int
 duplicate_eh_regions (struct function *ifun, duplicate_eh_regions_map map,
 		      void *data, int copy_region, int outer_region)
 {
-  eh_region cur, prev_try, old_prev_try, outer, *splice;
+  eh_region cur, outer, *splice;
   int i, min_region, max_region, eh_offset, cfun_last_region_number;
   int num_regions;
 
@@ -1228,14 +1227,12 @@ duplicate_eh_regions (struct function *i
       max_region = 0;
 
       cur = VEC_index (eh_region, ifun->eh->region_array, copy_region);
-      old_prev_try = find_prev_try (cur);
       duplicate_eh_regions_0 (cur, &min_region, &max_region);
     }
   else
     {
       min_region = 1;
       max_region = ifun->eh->last_region_number;
-      old_prev_try = NULL;
     }
   num_regions = max_region - min_region + 1;
   cfun_last_region_number = cfun->eh->last_region_number;
@@ -1302,12 +1299,6 @@ duplicate_eh_regions (struct function *i
     if (cur && cur->tree_label)
       cur->tree_label = map (cur->tree_label, data);
 
-  /* Search for the containing ERT_TRY region to fix up
-     the prev_try short-cuts for ERT_CLEANUP regions.  */
-  prev_try = NULL;
-  if (outer_region > 0)
-    prev_try = find_prev_try (VEC_index (eh_region, cfun->eh->region_array, outer_region));
-
   /* Remap all of the internal catch and cleanup linkages.  Since we 
      duplicate entire subtrees, all of the referenced regions will have
      been copied too.  And since we renumbered them as a block, a simple
@@ -1354,13 +1345,6 @@ duplicate_eh_regions (struct function *i
 	    REMAP (cur->u.eh_catch.prev_catch);
 	  break;
 
-	case ERT_CLEANUP:
-	  if (cur->u.cleanup.prev_try != old_prev_try)
-	    REMAP (cur->u.cleanup.prev_try);
-	  else
-	    cur->u.cleanup.prev_try = prev_try;
-	  break;
-
 	default:
 	  break;
 	}
@@ -1394,14 +1378,10 @@ copy_eh_region_1 (struct eh_region *old,
 
 /* Return new copy of eh region OLD inside region NEW_OUTER.  
   
-   Copy whole catch-try chain if neccesary and update cleanup region prev_try
-   pointers.
-
-   PREV_TRY_MAP points to outer TRY region if it was copied in trace already.  */
+   Copy whole catch-try chain if neccesary.  */
 
 static struct eh_region *
-copy_eh_region (struct eh_region *old, struct eh_region *new_outer,
-		struct eh_region *prev_try_map)
+copy_eh_region (struct eh_region *old, struct eh_region *new_outer)
 {
   struct eh_region *r, *n, *old_try, *new_try, *ret = NULL;
   VEC(eh_region,heap) *catch_list = NULL;
@@ -1410,11 +1390,6 @@ copy_eh_region (struct eh_region *old, s
     {
       gcc_assert (old->type != ERT_TRY);
       r = copy_eh_region_1 (old, new_outer);
-      if (r->type == ERT_CLEANUP)
-        {
-	  gcc_assert (r->u.cleanup.prev_try || !prev_try_map);
-          r->u.cleanup.prev_try = prev_try_map;
-	}
       return r;
     }
 
@@ -1477,7 +1452,7 @@ struct eh_region *
 redirect_eh_edge_to_label (edge e, tree new_dest_label, bool is_resx,
 			   bool inlinable_call, int region_number)
 {
-  struct eh_region *outer, *prev_try_map;
+  struct eh_region *outer;
   struct eh_region *region;
   VEC (eh_region, heap) * trace = NULL;
   int i;
@@ -1539,7 +1514,6 @@ redirect_eh_edge_to_label (edge e, tree 
 	}
       outer = VEC_index (eh_region, trace, start_here)->outer;
       gcc_assert (start_here >= 0);
-      prev_try_map = find_prev_try (outer);
 
       /* And now do the dirty job!  */
       for (i = start_here; i >= 0; i--)
@@ -1548,7 +1522,7 @@ redirect_eh_edge_to_label (edge e, tree 
 	  gcc_assert (!outer || old->outer != outer->outer);
 
 	  /* Copy region and update label.  */
-	  r = copy_eh_region (old, outer, prev_try_map);
+	  r = copy_eh_region (old, outer);
 	  VEC_replace (eh_region, trace, i, r);
 	  if (r->tree_label && label_to_block (r->tree_label) == old_bb)
 	    {
@@ -1588,14 +1562,11 @@ redirect_eh_edge_to_label (edge e, tree 
 		 }
 	     }
 
-	  /* Cleanup regions points to outer TRY blocks.  */
-	  if (r->type == ERT_TRY)
-	    prev_try_map = r;
 	  outer = r;
 	}
         
       if (is_resx || region->type == ERT_THROW)
-	r = copy_eh_region (region, outer, prev_try_map);
+	r = copy_eh_region (region, outer);
     }
 
   VEC_free (eh_region, heap, trace);
@@ -2674,32 +2645,6 @@ remove_eh_handler_and_replace (struct eh
 
   outer = region->outer;
 
-  /* When we are moving the region in EH tree, update prev_try pointers.  */
-  if ((outer != replace || region->type == ERT_TRY) && region->inner)
-    {
-      struct eh_region *prev_try = find_prev_try (replace);
-      p = region->inner;
-      while (p != region)
-	{
-	  if (p->type == ERT_CLEANUP)
-	    p->u.cleanup.prev_try = prev_try;
-          if (p->type != ERT_TRY
-	      && p->type != ERT_MUST_NOT_THROW
-	      && (p->type != ERT_ALLOWED_EXCEPTIONS
-	          || p->u.allowed.type_list)
-	      && p->inner)
-	    p = p->inner;
-	  else if (p->next_peer)
-	    p = p->next_peer;
-	  else
-	    {
-	      while (p != region && !p->next_peer)
-	        p = p->outer;
-	      if (p != region)
-		p = p->next_peer;
-	    }
-	}
-    }
   /* For the benefit of efficiently handling REG_EH_REGION notes,
      replace this region in the region array with its containing
      region.  Note that previous region deletions may result in
@@ -3123,7 +3068,7 @@ foreach_reachable_handler (int region_nu
       if (region->type == ERT_CLEANUP)
         {
 	  enum reachable_code code = RNL_NOT_CAUGHT;
-	  region = region->u.cleanup.prev_try;
+	  region = find_prev_try (region->outer);
 	  /* Continue looking for outer TRY region until we find one
 	     that might cath something.  */
           while (region
@@ -4431,9 +4376,6 @@ dump_eh_tree (FILE * out, struct functio
       switch (i->type)
 	{
 	case ERT_CLEANUP:
-	  if (i->u.cleanup.prev_try)
-	    fprintf (out, " prev try:%i",
-		     i->u.cleanup.prev_try->region_number);
 	  break;
 
 	case ERT_TRY:
@@ -4513,21 +4455,13 @@ debug_eh_tree (struct function *fn)
 /* Verify EH region invariants.  */
 
 static bool
-verify_eh_region (struct eh_region *region, struct eh_region *prev_try)
+verify_eh_region (struct eh_region *region)
 {
   bool found = false;
   if (!region)
     return false;
   switch (region->type)
     {
-    case ERT_CLEANUP:
-      if (region->u.cleanup.prev_try != prev_try)
-	{
-	  error ("Wrong prev_try pointer in EH region %i",
-		 region->region_number);
-	  found = true;
-	}
-      break;
     case ERT_TRY:
       {
 	struct eh_region *c, *prev = NULL;
@@ -4574,6 +4508,7 @@ verify_eh_region (struct eh_region *regi
 	  found = true;
 	}
       break;
+    case ERT_CLEANUP:
     case ERT_ALLOWED_EXCEPTIONS:
     case ERT_MUST_NOT_THROW:
     case ERT_THROW:
@@ -4581,14 +4516,8 @@ verify_eh_region (struct eh_region *regi
     case ERT_UNKNOWN:
       gcc_unreachable ();
     }
-  if (region->type == ERT_TRY)
-    prev_try = region;
-  else if (region->type == ERT_MUST_NOT_THROW
-	   || (region->type == ERT_ALLOWED_EXCEPTIONS
-	       && !region->u.allowed.type_list))
-    prev_try = NULL;
   for (region = region->inner; region; region = region->next_peer)
-    found |= verify_eh_region (region, prev_try);
+    found |= verify_eh_region (region);
   return found;
 }
 
@@ -4672,7 +4601,7 @@ verify_eh_tree (struct function *fun)
 		    }
 		  if (!err)
 		    for (i = fun->eh->region_tree; i; i = i->next_peer)
-		      err |= verify_eh_region (i, NULL);
+		      err |= verify_eh_region (i);
 		  
 		  if (err)
 		    {
Index: except.h
===================================================================
--- except.h	(revision 147317)
+++ except.h	(working copy)
@@ -85,12 +85,6 @@ struct GTY(()) eh_region
     struct eh_region_u_throw {
       tree type;
     } GTY ((tag ("ERT_THROW"))) eh_throw;
-
-    /* Retain the cleanup expression even after expansion so that
-       we can match up fixup regions.  */
-    struct eh_region_u_cleanup {
-      struct eh_region *prev_try;
-    } GTY ((tag ("ERT_CLEANUP"))) cleanup;
   } GTY ((desc ("%0.type"))) u;
 
   /* Entry point for this region's handler before landing pads are built.  */
@@ -185,8 +179,7 @@ extern int duplicate_eh_regions (struct 
 extern void sjlj_emit_function_exit_after (rtx);
 extern void default_init_unwind_resume_libfunc (void);
 
-extern struct eh_region *gen_eh_region_cleanup (struct eh_region *,
-						struct eh_region *);
+extern struct eh_region *gen_eh_region_cleanup (struct eh_region *);
 extern struct eh_region *gen_eh_region_try (struct eh_region *);
 extern struct eh_region *gen_eh_region_catch (struct eh_region *, tree);
 extern struct eh_region *gen_eh_region_allowed (struct eh_region *, tree);


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