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: [3.4] fixup_reorder_chain fix


> > > force_nonfallthru leads to rtl_redirect_edge_and_branch_force that has
> > > it's own code to unidentify edges:
> > >
> > >   /* In the case the last instruction is conditional jump to the next
> > >      instruction, first redirect the jump itself and then continue
> > >      by creating a basic block afterwards to redirect fallthru edge.  */
> > >   if (e->src != ENTRY_BLOCK_PTR && e->dest != EXIT_BLOCK_PTR
> > >       && any_condjump_p (BB_END (e->src))
> > >       /* When called from cfglayout, fallthru edges do not
> > >          necessarily go to the next block.  */
> > >       && e->src->next_bb == e->dest
> > >       && JUMP_LABEL (BB_END (e->src)) == BB_HEAD (e->dest))
> >
> > Well, thiking of this little bit harder, there is one better way to
> > avoid the logic from happening twice..  The logic to force_nonfallthru
> > was actually added after cfglayout hack and mostly duplicates what
> > cfglayout does.
> 
> They were actually added in the same patch, but
> 
>        /* When called from cfglayout, fallthru edges do not
>           necessarily go to the next block.  */
>       && e->src->next_bb == e->dest
> 
> was added a couple of weeks later.  That's why I think doing nothing in 
> fixup_reorder_chain is not quite correct, because we would miss an edge if 
> the code in force_nonfallthru_and_redirect doesn't trigger.

Yep, I noticed this today too :( Obviously I am in recusive screw up
process ;))
> 
> What about something like this (attached)?

I must admit that I don't quite like pushing more knowledge about
implementation details of force_nonfallthru_and_redirect to cfglayout.
The code is improperly factored here as f_n_a_r is not supposed to
opperate on cfglayout mode CFG at all, but I don't see much better way
except for duplicating all the logic either.

I originally used the knowledge that f_n_a_r won't break when edge and
jump destination don't match and obviously it bit us already.  From this
point of view I sort of like my original hard working patch as at least
it transform CFG into something more or less valid before calling
further.  (the case it will create new BB is really unlikely as it
happens only for infinite self loops)

On the other hand I don't quite see why the added condidtional would be
neccesary if we simplify cfglayout like in my second patch... If I
recall my original procedure that lead to first patch, I first did the
work on cfglayout and then eventually noticed that f_n_a_r needs similar
code as it might run into degnerate conditionals too.  I will figure out
tonight and I am running testing with the attached patch ;)

Honza

Index: cfglayout.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cfglayout.c,v
retrieving revision 1.50
diff -c -3 -p -r1.50 cfglayout.c
*** cfglayout.c	30 Dec 2003 10:40:50 -0000	1.50
--- cfglayout.c	4 Oct 2005 13:10:07 -0000
*************** fixup_reorder_chain (void)
*** 662,697 ****
  
  	      /* The degenerated case of conditional jump jumping to the next
  		 instruction can happen on target having jumps with side
! 		 effects.
! 
! 		 Create temporarily the duplicated edge representing branch.
! 		 It will get unidentified by force_nonfallthru_and_redirect
! 		 that would otherwise get confused by fallthru edge not pointing
! 		 to the next basic block.  */
  	      if (!e_taken)
! 		{
! 		  rtx note;
! 		  edge e_fake;
! 
! 		  e_fake = unchecked_make_edge (bb, e_fall->dest, 0);
! 
! 		  if (!redirect_jump (BB_END (bb), block_label (bb), 0))
! 		    abort ();
! 		  note = find_reg_note (BB_END (bb), REG_BR_PROB, NULL_RTX);
! 		  if (note)
! 		    {
! 		      int prob = INTVAL (XEXP (note, 0));
! 
! 		      e_fake->probability = prob;
! 		      e_fake->count = e_fall->count * prob / REG_BR_PROB_BASE;
! 		      e_fall->probability -= e_fall->probability;
! 		      e_fall->count -= e_fake->count;
! 		      if (e_fall->probability < 0)
! 			e_fall->probability = 0;
! 		      if (e_fall->count < 0)
! 			e_fall->count = 0;
! 		    }
! 		}
  	      /* There is one special case: if *neither* block is next,
  		 such as happens at the very end of a function, then we'll
  		 need to add a new unconditional jump.  Choose the taken
--- 662,671 ----
  
  	      /* The degenerated case of conditional jump jumping to the next
  		 instruction can happen on target having jumps with side
! 		 effects.  We always need to construct forwarder block that will
! 		 be handled by firce_nonfallthru.  */
  	      if (!e_taken)
! 		;
  	      /* There is one special case: if *neither* block is next,
  		 such as happens at the very end of a function, then we'll
  		 need to add a new unconditional jump.  Choose the taken
Index: cfgrtl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cfgrtl.c,v
retrieving revision 1.103.2.4
diff -c -3 -p -r1.103.2.4 cfgrtl.c
*** cfgrtl.c	2 May 2004 19:37:41 -0000	1.103.2.4
--- cfgrtl.c	4 Oct 2005 13:10:07 -0000
*************** force_nonfallthru_and_redirect (edge e, 
*** 999,1007 ****
       by creating a basic block afterwards to redirect fallthru edge.  */
    if (e->src != ENTRY_BLOCK_PTR && e->dest != EXIT_BLOCK_PTR
        && any_condjump_p (BB_END (e->src))
-       /* When called from cfglayout, fallthru edges do not
-          necessarily go to the next block.  */
-       && e->src->next_bb == e->dest
        && JUMP_LABEL (BB_END (e->src)) == BB_HEAD (e->dest))
      {
        rtx note;
--- 999,1004 ----
Index: opts.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/opts.c,v
retrieving revision 1.51.4.3
diff -c -3 -p -r1.51.4.3 opts.c
*** opts.c	18 Feb 2004 00:09:04 -0000	1.51.4.3
--- opts.c	4 Oct 2005 13:10:09 -0000
*************** decode_options (unsigned int argc, const
*** 524,529 ****
--- 524,530 ----
        flag_merge_constants = 0;
      }
  
+       flag_non_call_exceptions = 1;
    if (optimize >= 1)
      {
        flag_defer_pop = 1;


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