Patchwork [RFC] Merge 'expand' and 'rtl_eh'

login
register
mail settings
Submitter Richard Guenther
Date March 19, 2012, 3:41 p.m.
Message ID <alpine.LNX.2.00.1203191636450.4662@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/147559/
State New
Headers show

Comments

Richard Guenther - March 19, 2012, 3:41 p.m.
We have the unfortunate situation that expand may leave us with
unreachable blocks that are required by proper rtl_eh functioning
(unreachable landing-pads, that is).  This is bad for any verification
pass the pass manager may want to call that requires dominators to
be computed (I'm thinking of verifying loop structures, which I really
want to do after expand as I am trying to preserve loop structures
from tree loop optimization until after RTL loop optimization).

This limitation has been introduced with the introduction of
rtl_eh - and is, IMHO, bad as it exposed a partly broken IL state
to the passmanager.

Thus the following patch makes sure we can leave cfgexpand with
no unreachable blocks around and simply merges the rtl_eh pass
and expand.

Yes, we do have several tests in the testsuite where EH landing
pads get unreachable during expansion, by a disconnect of what
GIMPLE thinks can throw and what the expanded code sequence says
(bogus SSE builtin definitions in the x86_64 backend for example,
see g++.dg/other/i386-1.C for two cases).

Bootstrap and regtest on x86_64-unknown-linux-gnu running.

Comments?

Thanks,
Richard.

2012-03-19  Richard Guenther  <rguenther@suse.de>

	* tree-pass.h (pass_rtl_eh): Remove.
	* except.c (gate_handle_eh): Likewise.
	(rest_of_handle_eh): Likewise.
	(pass_rtl_eh): Likewise.
	(finish_eh_generation): Export.
	* except.h (finish_eh_generation): Declare.
	* passes.c (init_optimization_passes): Remove pass_rtl_eh.
	* cfgexpand.c (gimple_expand_cfg): Call finish_eh_generation
	after expanding stack alignment.  Instead of compacting blocks
	call cleanup_cfg.

Index: gcc/cfgexpand.c
===================================================================
*** gcc/cfgexpand.c	(revision 185518)
--- gcc/cfgexpand.c	(working copy)
*************** gimple_expand_cfg (void)
*** 4623,4632 ****
    sbitmap_free (blocks);
    purge_all_dead_edges ();
  
-   compact_blocks ();
- 
    expand_stack_alignment ();
  
  #ifdef ENABLE_CHECKING
    verify_flow_info ();
  #endif
--- 4632,4653 ----
    sbitmap_free (blocks);
    purge_all_dead_edges ();
  
    expand_stack_alignment ();
  
+   /* After initial rtl generation, call back to finish generating
+      exception support code.  We need to do this before cleaning up
+      the CFG as the code does not expect dead landing pads.  */
+   if (cfun->eh->region_tree != NULL)
+     finish_eh_generation ();
+ 
+   /* Remove unreachable blocks, otherwise we cannot compute dominators
+      which are needed for loop state verification.  As a side-effect
+      this also compacts blocks.
+      ???  We cannot remove trivially dead insns here as for example
+      the DRAP reg on i?86 is not magically live at this point.
+      gcc.c-torture/execute/ipa-sra-2.c execution, -Os -m32 fails otherwise.  */
+   cleanup_cfg (CLEANUP_NO_INSN_DEL);
+ 
  #ifdef ENABLE_CHECKING
    verify_flow_info ();
  #endif
Jan Hubicka - March 19, 2012, 3:57 p.m.
> 
> We have the unfortunate situation that expand may leave us with
> unreachable blocks that are required by proper rtl_eh functioning
> (unreachable landing-pads, that is).  This is bad for any verification
> pass the pass manager may want to call that requires dominators to
> be computed (I'm thinking of verifying loop structures, which I really
> want to do after expand as I am trying to preserve loop structures
> from tree loop optimization until after RTL loop optimization).
> 
> This limitation has been introduced with the introduction of
> rtl_eh - and is, IMHO, bad as it exposed a partly broken IL state
> to the passmanager.
> 
> Thus the following patch makes sure we can leave cfgexpand with
> no unreachable blocks around and simply merges the rtl_eh pass
> and expand.
> 
> Yes, we do have several tests in the testsuite where EH landing
> pads get unreachable during expansion, by a disconnect of what
> GIMPLE thinks can throw and what the expanded code sequence says
> (bogus SSE builtin definitions in the x86_64 backend for example,
> see g++.dg/other/i386-1.C for two cases).

Those cases are in fact mostly broken.  When gimple thinks somehting throws
and it does not, it is a missed optimization.
Especially the SSE intrincisc not having NOTHROW.  We should have at least tracking PR for them.
> 
> Bootstrap and regtest on x86_64-unknown-linux-gnu running.
> 
> Comments?

This seems OK to me...
>        NEXT_PASS (pass_init_function);
>        NEXT_PASS (pass_jump);
> -      NEXT_PASS (pass_rtl_eh);
>        NEXT_PASS (pass_initial_value_sets);
>        NEXT_PASS (pass_unshare_all_rtl);

IL is deeply irregular until this step, since RTL sharing is nothing any of
other passes are designed to handle and it does not match what rtl.texi says.
I would suggest merging all those into cfgexpand, not just rtl_eh.

Honza
Richard Guenther - March 19, 2012, 3:59 p.m.
On Mon, 19 Mar 2012, Jan Hubicka wrote:

> > 
> > We have the unfortunate situation that expand may leave us with
> > unreachable blocks that are required by proper rtl_eh functioning
> > (unreachable landing-pads, that is).  This is bad for any verification
> > pass the pass manager may want to call that requires dominators to
> > be computed (I'm thinking of verifying loop structures, which I really
> > want to do after expand as I am trying to preserve loop structures
> > from tree loop optimization until after RTL loop optimization).
> > 
> > This limitation has been introduced with the introduction of
> > rtl_eh - and is, IMHO, bad as it exposed a partly broken IL state
> > to the passmanager.
> > 
> > Thus the following patch makes sure we can leave cfgexpand with
> > no unreachable blocks around and simply merges the rtl_eh pass
> > and expand.
> > 
> > Yes, we do have several tests in the testsuite where EH landing
> > pads get unreachable during expansion, by a disconnect of what
> > GIMPLE thinks can throw and what the expanded code sequence says
> > (bogus SSE builtin definitions in the x86_64 backend for example,
> > see g++.dg/other/i386-1.C for two cases).
> 
> Those cases are in fact mostly broken.  When gimple thinks somehting throws
> and it does not, it is a missed optimization.
> Especially the SSE intrincisc not having NOTHROW.  We should have at least tracking PR for them.
> > 
> > Bootstrap and regtest on x86_64-unknown-linux-gnu running.
> > 
> > Comments?
> 
> This seems OK to me...
> >        NEXT_PASS (pass_init_function);
> >        NEXT_PASS (pass_jump);
> > -      NEXT_PASS (pass_rtl_eh);
> >        NEXT_PASS (pass_initial_value_sets);
> >        NEXT_PASS (pass_unshare_all_rtl);
> 
> IL is deeply irregular until this step, since RTL sharing is nothing any of
> other passes are designed to handle and it does not match what rtl.texi says.
> I would suggest merging all those into cfgexpand, not just rtl_eh.

Ok.  If the posted patch passes bootstrap & regtest I'll see what I can 
do.

Richard.
Steven Bosscher - March 19, 2012, 4:06 p.m.
On Mon, Mar 19, 2012 at 4:41 PM, Richard Guenther <rguenther@suse.de> wrote:
> Comments?

What does rtl_eh do for no-SJLJ exceptions?

Have you tested with SJLJ exceptions? (Can/should we move that code to GIMPLE?)

Ciao!
Steven
Richard Guenther - March 19, 2012, 4:08 p.m.
On Mon, 19 Mar 2012, Steven Bosscher wrote:

> On Mon, Mar 19, 2012 at 4:41 PM, Richard Guenther <rguenther@suse.de> wrote:
> > Comments?
> 
> What does rtl_eh do for no-SJLJ exceptions?

Quoting from except.c

'Then, via finish_eh_generation, we generate the real landing pads
   to which the runtime will actually transfer control.  These new
   landing pads perform whatever bookkeeping is needed by the target
   backend in order to resume execution within the current function.
   Each of these new landing pads falls through into the post_landing_pad
   label which had been used within the CFG up to this point.  All
   exception edges within the CFG are redirected to the new landing pads.
   If the target uses setjmp to implement exceptions, the various extra
   calls into the runtime to register and unregister the current stack
   frame are emitted at this time.'

> Have you tested with SJLJ exceptions? (Can/should we move that code to GIMPLE?)

No.  The only thing that changes is the time when we call 
fixup_tail_calls, otherwise the patch should be a no-op basically
hiding the inconsistent state during the piecewise RTL expansion
from the pass manager.

Richard.
Michael Matz - March 19, 2012, 4:45 p.m.
Hi,

On Mon, 19 Mar 2012, Steven Bosscher wrote:

> On Mon, Mar 19, 2012 at 4:41 PM, Richard Guenther <rguenther@suse.de> wrote:
> > Comments?
> 
> What does rtl_eh do for no-SJLJ exceptions?

Emitting the landing pad code (copy-out from EH_RETURN_DATA_REGNO, or 
target specific code) and redirecting edges to that one.

> (Can/should we move that code to GIMPLE?)

We could, but for the sjlj case it's some work to rework the whole 
thing to gimple, and for bb-reorder we at least need the dwarf landing pad 
expander also on RTL (meaning also the target expander for that).  Not 
much gain there, so IMO we shouldn't.

We could also do away with the separation of landing pad and post landing 
pad (e.g. by a new _builtin_landing_pad_magic()), but that would get rid 
of the RTL redirecting of edges only for the dwarf case, as the sjlj still 
would rewrite the CFG by emitting explicit switch statements.  Might be 
slightly nicer nevertheless, but also not terribly important gain.


Ciao,
Michael.
Jan Hubicka - March 19, 2012, 4:55 p.m.
> On Mon, 19 Mar 2012, Steven Bosscher wrote:
> 
> > On Mon, Mar 19, 2012 at 4:41 PM, Richard Guenther <rguenther@suse.de> wrote:
> > > Comments?
> > 
> > What does rtl_eh do for no-SJLJ exceptions?
> 
> Quoting from except.c
> 
> 'Then, via finish_eh_generation, we generate the real landing pads
>    to which the runtime will actually transfer control.  These new
>    landing pads perform whatever bookkeeping is needed by the target
>    backend in order to resume execution within the current function.
>    Each of these new landing pads falls through into the post_landing_pad
>    label which had been used within the CFG up to this point.  All
>    exception edges within the CFG are redirected to the new landing pads.
>    If the target uses setjmp to implement exceptions, the various extra
>    calls into the runtime to register and unregister the current stack
>    frame are emitted at this time.'
> 
> > Have you tested with SJLJ exceptions? (Can/should we move that code to GIMPLE?)
> 
> No.  The only thing that changes is the time when we call 
> fixup_tail_calls, otherwise the patch should be a no-op basically
> hiding the inconsistent state during the piecewise RTL expansion
> from the pass manager.

As discussed on IRC, we ought to merge the passes that keeps RTL inconsistent
into single pass. Until RTl reaches its specified form (that is after unsharing)
those are not realy independent passes anyway. They just come from historical
way rest_of_compilation function was shaped.

As a followup I will try to cleanup those early stages of compilation getting
rid of pass_jump/pass_jump2 and friends.

Honza
> 
> Richard.

Patch

Index: gcc/tree-pass.h
===================================================================
--- gcc/tree-pass.h.orig	2012-03-19 16:29:36.000000000 +0100
+++ gcc/tree-pass.h	2012-03-19 16:28:03.000000000 +0100
@@ -488,7 +488,6 @@  extern struct gimple_opt_pass pass_fixup
 extern struct rtl_opt_pass pass_expand;
 extern struct rtl_opt_pass pass_init_function;
 extern struct rtl_opt_pass pass_jump;
-extern struct rtl_opt_pass pass_rtl_eh;
 extern struct rtl_opt_pass pass_initial_value_sets;
 extern struct rtl_opt_pass pass_unshare_all_rtl;
 extern struct rtl_opt_pass pass_instantiate_virtual_regs;
Index: gcc/except.c
===================================================================
--- gcc/except.c.orig	2012-03-19 16:29:36.000000000 +0100
+++ gcc/except.c	2012-03-19 16:29:06.000000000 +0100
@@ -81,7 +81,7 @@  along with GCC; see the file COPYING3.
    gimple to eh_region mapping that had been recorded in the
    THROW_STMT_TABLE.
 
-   During pass_rtl_eh (except.c), we generate the real landing pads
+   Then, via finish_eh_generation, we generate the real landing pads
    to which the runtime will actually transfer control.  These new
    landing pads perform whatever bookkeeping is needed by the target
    backend in order to resume execution within the current function.
@@ -1414,7 +1414,7 @@  sjlj_build_landing_pads (void)
 /* After initial rtl generation, call back to finish generating
    exception support code.  */
 
-static void
+void
 finish_eh_generation (void)
 {
   basic_block bb;
@@ -1461,41 +1461,6 @@  finish_eh_generation (void)
 	}
     }
 }
-
-static bool
-gate_handle_eh (void)
-{
-  /* Nothing to do if no regions created.  */
-  return cfun->eh->region_tree != NULL;
-}
-
-/* Complete generation of exception handling code.  */
-static unsigned int
-rest_of_handle_eh (void)
-{
-  finish_eh_generation ();
-  cleanup_cfg (CLEANUP_NO_INSN_DEL);
-  return 0;
-}
-
-struct rtl_opt_pass pass_rtl_eh =
-{
- {
-  RTL_PASS,
-  "rtl_eh",                             /* name */
-  gate_handle_eh,                       /* gate */
-  rest_of_handle_eh,			/* execute */
-  NULL,                                 /* sub */
-  NULL,                                 /* next */
-  0,                                    /* static_pass_number */
-  TV_JUMP,                              /* tv_id */
-  0,                                    /* properties_required */
-  0,                                    /* properties_provided */
-  0,                                    /* properties_destroyed */
-  0,                                    /* todo_flags_start */
-  0                                     /* todo_flags_finish */
- }
-};
 
 /* This section handles removing dead code for flow.  */
 
Index: gcc/except.h
===================================================================
--- gcc/except.h.orig	2011-07-25 10:50:26.000000000 +0200
+++ gcc/except.h	2012-03-19 16:24:58.000000000 +0100
@@ -291,6 +291,8 @@  extern void assign_filter_values (void);
 extern eh_region get_eh_region_from_rtx (const_rtx);
 extern eh_landing_pad get_eh_landing_pad_from_rtx (const_rtx);
 
+extern void finish_eh_generation (void);
+
 struct GTY(()) throw_stmt_node {
   gimple stmt;
   int lp_nr;
Index: gcc/passes.c
===================================================================
--- gcc/passes.c.orig	2012-03-19 14:52:11.000000000 +0100
+++ gcc/passes.c	2012-03-19 16:27:22.000000000 +0100
@@ -1431,7 +1431,6 @@  init_optimization_passes (void)
       struct opt_pass **p = &pass_rest_of_compilation.pass.sub;
       NEXT_PASS (pass_init_function);
       NEXT_PASS (pass_jump);
-      NEXT_PASS (pass_rtl_eh);
       NEXT_PASS (pass_initial_value_sets);
       NEXT_PASS (pass_unshare_all_rtl);
       NEXT_PASS (pass_instantiate_virtual_regs);