[patch] Fix pessimization in EH cleanup pass

[Eric Botcazou]

Hi,

the cleanup_all_empty_eh function was originally doing a post-order traversal 
of the EH region tree to optimize it:

/* Do a post-order traversal of the EH region tree.  Examine each
   post_landing_pad block and see if we can eliminate it as empty.  */

That's sensible since the worker function cleanup_empty_eh punts if the number 
of outgoing edges from landing pads is not 0 or 1:

  /* There can be zero or one edges out of BB.  This is the quickest test.  */
  switch (EDGE_COUNT (bb->succs))
    {
    case 0:
      e_out = NULL;
      break;
    case 1:
      e_out = single_succ_edge (bb);
      break;
    default:
      return false;
    }

This was recently changed to use another order and this trivially breaks 
testcases with nested regions like the attached one.  So the attached patch 
restores the post-order traversal and it also contains a small tweak to the 
line debug info to avoid a problematic inheritance for coverage measurement.

Tested on x86_64-suse-linux, OK for the mainline?


2020-09-15  Eric Botcazou  <ebotca...@adacore.com>
            Pierre-Marie Derodat  <dero...@adacore.com>

        * tree-eh.c (lower_try_finally_dup_block): Backward propagate slocs 
        to stack restore builtin calls.
        (cleanup_all_empty_eh): Do again a post-order traversal of the EH
        region tree.


2020-09-15  Eric Botcazou  <ebotca...@adacore.com>

        * gnat.dg/concat4.adb: New test.

-- 
Eric Botcazou
-- { dg-do compile }

with Ada.Text_IO; use Ada.Text_IO;

procedure Concat4 (X : Integer) is
   Ximg : constant String := Integer'Image (X);
begin
   if X > 0 then
      Put_Line (Ximg & " is Positive");
   elsif X < 0 then
      Put_Line (Ximg & " is Negative");
   else
      Put_Line (Ximg & " is Null");
   end if;
end;

-- { dg-final { scan-assembler-not "_Unwind_Resume" } }

diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 4246dca8806..ed32f80220d 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -899,23 +899,26 @@ lower_try_finally_dup_block (gimple_seq seq, struct leh_state *outer_state,
   gtry *region = NULL;
   gimple_seq new_seq;
   gimple_stmt_iterator gsi;
+  location_t last_loc = UNKNOWN_LOCATION;
 
   new_seq = copy_gimple_seq_and_replace_locals (seq);
 
-  for (gsi = gsi_start (new_seq); !gsi_end_p (gsi); gsi_next (&gsi))
+  for (gsi = gsi_last (new_seq); !gsi_end_p (gsi); gsi_prev (&gsi))
     {
       gimple *stmt = gsi_stmt (gsi);
       /* We duplicate __builtin_stack_restore at -O0 in the hope of eliminating
-	 it on the EH paths.  When it is not eliminated, make it transparent in
-	 the debug info.  */
+	 it on the EH paths.  When it is not eliminated, give it the next
+	 location in the sequence or make it transparent in the debug info.  */
       if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
-	gimple_set_location (stmt, UNKNOWN_LOCATION);
+	gimple_set_location (stmt, last_loc);
       else if (LOCATION_LOCUS (gimple_location (stmt)) == UNKNOWN_LOCATION)
 	{
 	  tree block = gimple_block (stmt);
 	  gimple_set_location (stmt, loc);
 	  gimple_set_block (stmt, block);
 	}
+      else
+	last_loc = gimple_location (stmt);
     }
 
   if (outer_state->tf)
@@ -4751,15 +4754,9 @@ cleanup_all_empty_eh (void)
   eh_landing_pad lp;
   int i;
 
-  /* Ideally we'd walk the region tree and process LPs inner to outer
-     to avoid quadraticness in EH redirection.  Walking the LP array
-     in reverse seems to be an approximation of that.  */
-  for (i = vec_safe_length (cfun->eh->lp_array) - 1; i >= 1; --i)
-    {
-      lp = (*cfun->eh->lp_array)[i];
-      if (lp)
-	changed |= cleanup_empty_eh (lp);
-    }
+  for (i = 1; vec_safe_iterate (cfun->eh->lp_array, i, &lp); ++i)
+    if (lp)
+      changed |= cleanup_empty_eh (lp);
 
   return changed;
 }