This is the mail archive of the gcc@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]

[3.4] fixup_reorder_chain fix


> Hi Jan,
> 
> I think fixup_reorder_chain contains questionable code to cope with a 
> pathological case:
> 
> 	      /* 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;
> 		  bool redirected;
> 
> 		  e_fake = unchecked_make_edge (bb, e_fall->dest, 0);
> 
> 		  redirected = redirect_jump (BB_END (bb),
> 					      block_label (bb), 0);
> 		  gcc_assert (redirected);
> 
> Note the call to redirect_jump that creates a loop.  It is responsible for the 
> ICE on the attached Ada testcase with the 3.4.5pre compiler at -O3 because the 
> edge and the jump disagree on the target.
> 
> The final patch: http://gcc.gnu.org/ml/gcc-cvs/2003-03/msg01294.html
> The original version: http://gcc.gnu.org/ml/gcc-patches/2003-03/msg02097.html
> 
> Am I right in thinking that the call to redirect_jump must be removed?

Hi,
this is patch I am testing to fix the bug.  I apologize for delay on
fixing it.  I was on conference without my usual setup so building Ada
was bit challenging.  Since this code is so rarely excercised I would
welcome if you run the Ada testsuite on it.

To summarize some of offlist discussion.  The actual problem is
-fnon-call-exception (enabled by default by Ada) preventing GCC from
removing the conditional branch to next instruction.  THis results in
fallthru and branch edge to be indentified excercising interesting side
corners of cfglayout code previously used only by HP-PA.

The orignal hack created duplicated edge that resulted in CFG breaking
our invariants and since we added more sanity checking, it fires now.

The current workaround is to redirect the branch edge to form self-loop
around the current BB and redirect it properly later in function.  I
tried this once before in past but it falled into cracks when the
conditonal formed infinite self loop, so I went the way of duplicated
edges.  Alternate sollution is to create new basic block destination in
this very side case that I am doing now.
At least on artifically constructed testcases it seems to work as
expected.

Honza

Index: cfglayout.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cfglayout.c,v
retrieving revision 1.50.4.2
diff -c -3 -p -r1.50.4.2 cfglayout.c
*** cfglayout.c	7 Oct 2004 06:11:59 -0000	1.50.4.2
--- cfglayout.c	2 Oct 2005 17:28:40 -0000
*************** fixup_reorder_chain (void)
*** 636,641 ****
--- 636,643 ----
        edge e_fall, e_taken, e;
        rtx bb_end_insn;
        basic_block nb;
+       edge edge_to_redirect = NULL;
+       basic_block new_dest;
  
        if (bb->succ == NULL)
  	continue;
*************** fixup_reorder_chain (void)
*** 664,681 ****
  		 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)
--- 666,689 ----
  		 instruction can happen on target having jumps with side
  		 effects.
  
! 		 Redirect branch to construct a loop around current branch
! 		 so the branch and fallthru edges are no longer identified.
! 		 Once the fallthru is broken up, the original branch is redirected
! 		 back.  */
  	      if (!e_taken)
  		{
  		  rtx note;
  		  edge e_fake;
+ 		  basic_block fake_target = bb;
  
! 		  /* Even more side case: It is possible that the conditional is forming
! 		     infinite self loop so redirecting to BB would be no-op.  Create new
! 		     temporary dest for this that will be elliminated later in cfgcleanup.  */
! 		  if (e_fall->dest == bb)
! 		    fake_target = create_basic_block (NULL, NULL, EXIT_BLOCK_PTR->prev_bb);
! 		  e_fake = unchecked_make_edge (bb, fake_target, 0);
  
! 		  if (!redirect_jump (BB_END (bb), block_label (fake_target), 0))
  		    abort ();
  		  note = find_reg_note (BB_END (bb), REG_BR_PROB, NULL_RTX);
  		  if (note)
*************** fixup_reorder_chain (void)
*** 691,696 ****
--- 699,706 ----
  		      if (e_fall->count < 0)
  			e_fall->count = 0;
  		    }
+ 		  edge_to_redirect = e_fake;
+ 		  new_dest = e_fall->dest;
  		}
  	      /* There is one special case: if *neither* block is next,
  		 such as happens at the very end of a function, then we'll
*************** fixup_reorder_chain (void)
*** 762,767 ****
--- 772,779 ----
  
        /* We got here if we need to add a new jump insn.  */
        nb = force_nonfallthru (e_fall);
+       if (edge_to_redirect)
+ 	redirect_edge_and_branch (edge_to_redirect, new_dest);
        if (nb)
  	{
  	  cfg_layout_initialize_rbi (nb);


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