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: 'fix' for 11350 causes bootstrap hang on Darwin


> > 
> > Hi Jan,
> > 
> > This patch:
> > 
> > 2004-01-16  Jan Hubicka  <jh@suse.cz>
> > 
> >         PR opt/11350
> >         * cfgcleanup.c (try_optimize_cfg): Suppress tablejump removal
> >         after reload.
> >         * cfgrtl.c (rtl_can_merge_blocks, cfglayout_can_merge_blocks,
> >         rtl_try_redirect_by_replacing_branch): Likewise.
> > 
> > causes bootstrap to hang on powerpc-darwin.  The sequence is:
> > 
> > - try_optimize_cfg sees a branch like:
> > 
> > (jump_insn:HI 284 735 291 26 insn-attrtab.c:99366 (parallel [
> >             (set (pc)
> >                 (reg:SI 66 ctr [198]))
> >             (use (label_ref 285))
> >         ]) 515 {*rs6000.md:13804} (insn_list 735 (nil))
> >     (expr_list:REG_DEAD (reg:SI 66 ctr [198])
> >         (nil)))
> > 
> > which is the remains of a tablejump insn.  It runs this chunk of code:
> > 
> >               /* If B has a single outgoing edge, but uses a
> >                  non-trivial jump instruction without side-effects, we
> >                  can either delete the jump entirely, or replace it
> >                  with a simple unconditional jump.  Use
> >                  redirect_edge_and_branch to do the dirty work.  */
> >               if (b->succ
> >                   && ! b->succ->succ_next
> >                   && b->succ->dest != EXIT_BLOCK_PTR
> >                   && onlyjump_p (BB_END (b))
> >                   && redirect_edge_and_branch (b->succ,
> >                  b->succ->dest))
> >                 {
> >                   update_forwarder_flag (b);
> >                   changed_here = true;
> >                 }
> > 
> > - rtl_redirect_edge_and_branch calls try_redirect_by_replacing_jump
> > - try_redirect_by_replacing_jump returns false because
> >   this is (was) a tablejump and reload_completed is set
> > - rtl_redirect_edge_and_branch then enters this code:
> > 
> > 2444      if (e->dest == dest)
> > 2445        return true;
> > 
> > and since this is true (because it was called with 'dest' set to
> > b->succ->dest and 'e' set to b->succ), it'll return true without doing
> > anything.
> > 
> > I'm now testing this patch, and will commit if bootstrap & testrun
> > succeeds (I presume that a testcase was added for 11350, since
> > there's one in the bug report).
> > 
> > 2004-01-16  Geoffrey Keating  <geoffk@apple.com>
> > 
> >         * cfgrtl.c (try_redirect_by_replacing_jump): Optimize tablejumps
> >         even after reload, just don't remove the actual jump tables.
> 
> I actually read this patch backwards.  I am working on more correct
> sollution.  i think cfgcleanup should simply use
> try_redirect_by_replacing_jump instead of doing the trick of redirecting
> to itself.

This is the patch I am testing on i386 now.  If someone can check
whether it fix the original problem after reverting Geoff's patch, I
would be very hapy.
The idea of using redirect_edge_and_branch to do jump simplification
wasn't very smart, even tought it did sound fine to me at the time I
wrote it.  I should've taken some time to break up the logic as it is
now.

2004-01-17  Jan Hubicka  <jh@suse.cz>
	* basic-block.h (try_redirect_by_replacing_jump): Declare.
	* cfgcleanup.c (try_optimize_cfg): Use it.
	* cfgrtl.c (try_redirect_by_replacing_jump): Export.
	(rtl_redirect_edge_and_branch, cfg_layout_redirect_edge_and_branch):
	Kill hack.
Index: basic-block.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/basic-block.h,v
retrieving revision 1.186
diff -c -3 -p -r1.186 basic-block.h
*** basic-block.h	30 Dec 2003 10:40:48 -0000	1.186
--- basic-block.h	17 Jan 2004 19:10:15 -0000
*************** extern void iterate_fix_dominators (enum
*** 640,645 ****
--- 640,646 ----
  extern void verify_dominators (enum cdi_direction);
  extern basic_block first_dom_son (enum cdi_direction, basic_block);
  extern basic_block next_dom_son (enum cdi_direction, basic_block);
+ extern bool try_redirect_by_replacing_jump (edge, basic_block, bool);
  
  #include "cfghooks.h"
  
Index: cfgcleanup.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cfgcleanup.c,v
retrieving revision 1.99
diff -c -3 -p -r1.99 cfgcleanup.c
*** cfgcleanup.c	16 Jan 2004 11:30:47 -0000	1.99
--- cfgcleanup.c	17 Jan 2004 19:10:15 -0000
*************** try_optimize_cfg (int mode)
*** 1785,1797 ****
  	      /* If B has a single outgoing edge, but uses a
  		 non-trivial jump instruction without side-effects, we
  		 can either delete the jump entirely, or replace it
! 		 with a simple unconditional jump.  Use
! 		 redirect_edge_and_branch to do the dirty work.  */
  	      if (b->succ
  		  && ! b->succ->succ_next
  		  && b->succ->dest != EXIT_BLOCK_PTR
  		  && onlyjump_p (BB_END (b))
! 		  && redirect_edge_and_branch (b->succ, b->succ->dest))
  		{
  		  update_forwarder_flag (b);
  		  changed_here = true;
--- 1785,1797 ----
  	      /* If B has a single outgoing edge, but uses a
  		 non-trivial jump instruction without side-effects, we
  		 can either delete the jump entirely, or replace it
! 		 with a simple unconditional jump.  */
  	      if (b->succ
  		  && ! b->succ->succ_next
  		  && b->succ->dest != EXIT_BLOCK_PTR
  		  && onlyjump_p (BB_END (b))
! 		  && try_redirect_by_replacing_jump (b->succ, b->succ->dest,
! 						     (mode & CLEANUP_CFGLAYOUT)))
  		{
  		  update_forwarder_flag (b);
  		  changed_here = true;
Index: cfgrtl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cfgrtl.c,v
retrieving revision 1.104
diff -c -3 -p -r1.104 cfgrtl.c
*** cfgrtl.c	17 Jan 2004 07:46:49 -0000	1.104
--- cfgrtl.c	17 Jan 2004 19:10:16 -0000
*************** block_label (basic_block block)
*** 687,693 ****
     apply only if all edges now point to the same block.  The parameters and
     return values are equivalent to redirect_edge_and_branch.  */
  
! static bool
  try_redirect_by_replacing_jump (edge e, basic_block target, bool in_cfglayout)
  {
    basic_block src = e->src;
--- 687,693 ----
     apply only if all edges now point to the same block.  The parameters and
     return values are equivalent to redirect_edge_and_branch.  */
  
! bool
  try_redirect_by_replacing_jump (edge e, basic_block target, bool in_cfglayout)
  {
    basic_block src = e->src;
*************** rtl_redirect_edge_and_branch (edge e, ba
*** 971,985 ****
    if (e->flags & (EDGE_ABNORMAL_CALL | EDGE_EH))
      return false;
  
    if (try_redirect_by_replacing_jump (e, target, false))
      return true;
  
!   /* Do this fast path late, as we want above code to simplify for cases
!      where called on single edge leaving basic block containing nontrivial
!      jump insn.  */
!   else if (e->dest == target)
!     return false;
!   else if (!redirect_branch_edge (e, target))
      return false;
  
    return true;
--- 971,983 ----
    if (e->flags & (EDGE_ABNORMAL_CALL | EDGE_EH))
      return false;
  
+   if (e->dest == target)
+     return true;
+ 
    if (try_redirect_by_replacing_jump (e, target, false))
      return true;
  
!   if (!redirect_branch_edge (e, target))
      return false;
  
    return true;
*************** cfg_layout_redirect_edge_and_branch (edg
*** 2437,2447 ****
    if (e->flags & (EDGE_ABNORMAL_CALL | EDGE_EH))
      return false;
  
!   if (e->src != ENTRY_BLOCK_PTR
!       && try_redirect_by_replacing_jump (e, dest, true))
      return true;
  
!   if (e->dest == dest)
      return true;
  
    if (e->src == ENTRY_BLOCK_PTR
--- 2435,2445 ----
    if (e->flags & (EDGE_ABNORMAL_CALL | EDGE_EH))
      return false;
  
!   if (e->dest == dest)
      return true;
  
!   if (e->src != ENTRY_BLOCK_PTR
!       && try_redirect_by_replacing_jump (e, dest, true))
      return true;
  
    if (e->src == ENTRY_BLOCK_PTR


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