patch for gcse bug

Richard Henderson rth@cygnus.com
Thu Oct 8 01:03:00 GMT 1998


Ok, so people have probably seen that the patch checked in this
morning exposed some latent problems.

Below is my proposed solution. 

First, some modifications to flow so as to make it easy to detect
abnormal flow control out of a call insn.  Iff the call ends a basic
block due to abnormal flow, it will be the insn at BLOCK_END.  To
preserve this, I add nop rtl when there is no such abnormal flow,
but a call just happens to end a basic block.

Second, and much more important, is the modification to the PRE
global property PPout.  Using a new local property I dub TRANSPout,
I remove expressions from PPout that could be killed by such a 
call, that is, most mems since we do no interprocedual alias analysis.


r~


	* flow.c (find_basic_blocks): Calc upper bound for extra nops in
	max_uids_for_flow.
	(find_basic_blocks_1): Add a nop to the end of a basic block when
	a trailing call insn does not have abnormal control flow.
	* gcse.c (pre_transpout): New variable.
	(alloc_pre_mem, free_pre_mem, dump_pre_data): Bookkeeping for it.
	(compute_pre_transpout): Calculate it.
	(compute_pre_ppinout): Use it to eliminate impossible placements
	due to abnormal control flow through calls.
	(compute_pre_data): Call compute_pre_transpout.


Index: flow.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/flow.c,v
retrieving revision 1.63
diff -c -p -d -r1.63 flow.c
*** flow.c	1998/10/06 09:03:37	1.63
--- flow.c	1998/10/08 06:25:41
*************** find_basic_blocks (f, nregs, file, live_
*** 306,311 ****
--- 306,312 ----
    register int i;
    rtx nonlocal_label_list = nonlocal_label_rtx_list ();
    int in_libcall_block = 0;
+   int extra_uids_for_flow = 0;
  
    /* Count the basic blocks.  Also find maximum insn uid value used.  */
  
*************** find_basic_blocks (f, nregs, file, live_
*** 318,324 ****
  
      for (insn = f, i = 0; insn; insn = NEXT_INSN (insn))
        {
- 
  	/* Track when we are inside in LIBCALL block.  */
  	if (GET_RTX_CLASS (GET_CODE (insn)) == 'i'
  	    && find_reg_note (insn, REG_LIBCALL, NULL_RTX))
--- 319,324 ----
*************** find_basic_blocks (f, nregs, file, live_
*** 327,339 ****
  	code = GET_CODE (insn);
  	if (INSN_UID (insn) > max_uid_for_flow)
  	  max_uid_for_flow = INSN_UID (insn);
! 	if (code == CODE_LABEL
! 	    || (GET_RTX_CLASS (code) == 'i'
! 		&& (prev_code == JUMP_INSN
! 		    || (prev_code == CALL_INSN
! 			&& (nonlocal_label_list != 0 || eh_region))
! 		    || prev_code == BARRIER)))
  	  i++;
  
  	/* We change the code of the CALL_INSN, so that it won't start a
  	   new block.  */
--- 327,354 ----
  	code = GET_CODE (insn);
  	if (INSN_UID (insn) > max_uid_for_flow)
  	  max_uid_for_flow = INSN_UID (insn);
! 	if (code == CODE_LABEL)
  	  i++;
+ 	else if (GET_RTX_CLASS (code) == 'i')
+ 	  {
+ 	    if (prev_code == JUMP_INSN || prev_code == BARRIER)
+ 	      i++;
+ 	    else if (prev_code == CALL_INSN)
+ 	      {
+ 		if (nonlocal_label_list != 0 || eh_region)
+ 		  i++;
+ 		else
+ 		  {
+ 		    /* Else this call does not end a basic block.  In
+ 		       order to disambiguate this call from one that
+ 		       does end a block, we might need to add a nop
+ 		       insn following.  We'll find this out in 
+ 		       find_basic_blocks_1, but we need to account
+ 		       for the possibility in max_uid_for_flow.  */
+ 		    extra_uids_for_flow++;
+ 		  }
+ 	      }
+ 	  }
  
  	/* We change the code of the CALL_INSN, so that it won't start a
  	   new block.  */
*************** find_basic_blocks (f, nregs, file, live_
*** 360,365 ****
--- 375,381 ----
       These cases are rare, so we don't need too much space.  */
    max_uid_for_flow += max_uid_for_flow / 10;
  #endif
+   max_uid_for_flow += extra_uids_for_flow;
  
    /* Allocate some tables that last till end of compiling this function
       and some needed only in find_basic_blocks and life_analysis.  */
*************** find_basic_blocks_1 (f, nonlocal_label_l
*** 410,415 ****
--- 426,432 ----
    int depth, pass;
    int in_libcall_block = 0;
    int deleted_handler = 0;
+   int call_was_edge = 0;
  
    pass = 1;
    active_eh_region = (int *) alloca ((max_uid_for_flow + 1) * sizeof (int));
*************** find_basic_blocks_1 (f, nonlocal_label_l
*** 466,477 ****
  
  	  if (code == CODE_LABEL)
  	    {
! 		LABEL_REFS (insn) = insn;
! 		/* Any label that cannot be deleted
! 		   is considered to start a reachable block.  */
! 		if (LABEL_PRESERVE_P (insn))
! 		  block_live[i] = 1;
! 	      }
  	}
  
        else if (GET_RTX_CLASS (code) == 'i')
--- 483,507 ----
  
  	  if (code == CODE_LABEL)
  	    {
! 	      LABEL_REFS (insn) = insn;
! 	      /* Any label that cannot be deleted
! 		 is considered to start a reachable block.  */
! 	      if (LABEL_PRESERVE_P (insn))
! 		block_live[i] = 1;
! 	    }
! 
! 	  /* If the previous insn was a call that was not itself an edge,
! 	     we want to add a nop so that the call insn itself is not at
! 	     basic_block_end.  This allows us to simply distinguish the
! 	     two cases elsewhere.  */
! 
! 	  if (i > 0 && !call_was_edge
! 	      && GET_CODE (basic_block_end[i-1]) == CALL_INSN)
! 	    {
! 	      rtx nop = gen_rtx_USE (VOIDmode, const0_rtx);
! 	      nop = emit_insn_after (nop, basic_block_end[i-1]);
! 	      basic_block_end[i-1] = nop;
! 	    }
  	}
  
        else if (GET_RTX_CLASS (code) == 'i')
*************** find_basic_blocks_1 (f, nonlocal_label_l
*** 523,528 ****
--- 553,562 ----
  	 new block.  */
        if (code == CALL_INSN && in_libcall_block)
  	code = INSN;
+ 
+       /* Record whether this call created an edge.  */
+       if (code == CALL_INSN)
+ 	call_was_edge = (nonlocal_label_list != 0 || eh_note);
  
        if (code != NOTE)
  	prev_code = code;
Index: gcse.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/gcse.c,v
retrieving revision 1.15
diff -c -p -d -r1.15 gcse.c
*** gcse.c	1998/10/07 17:36:32	1.15
--- gcse.c	1998/10/08 06:25:42
*************** static sbitmap *pre_pavout;
*** 3888,3893 ****
--- 3888,3898 ----
  static sbitmap *pre_ppin;
  static sbitmap *pre_ppout;
  
+ /* Nonzero for expressions that are transparent at the end of the block.
+    This is only zero for expressions killed by abnormal critical edge
+    created by a calls.  */
+ static sbitmap *pre_transpout;
+ 
  /* Used while performing PRE to denote which insns are redundant.  */
  static sbitmap pre_redundant;
  
*************** alloc_pre_mem (n_blocks, n_exprs)
*** 3910,3915 ****
--- 3915,3922 ----
    pre_pavout = sbitmap_vector_alloc (n_blocks, n_exprs);
    pre_ppin = sbitmap_vector_alloc (n_blocks, n_exprs);
    pre_ppout = sbitmap_vector_alloc (n_blocks, n_exprs);
+ 
+   pre_transpout = sbitmap_vector_alloc (n_blocks, n_exprs);
  }
  
  /* Free vars used for PRE analysis.  */
*************** free_pre_mem ()
*** 3920,3926 ****
    free (pre_transp);
    free (pre_comp);
    free (pre_antloc);
- 
    free (pre_avin);
    free (pre_avout);
    free (pre_antin);
--- 3927,3932 ----
*************** free_pre_mem ()
*** 3930,3935 ****
--- 3936,3942 ----
    free (pre_pavout);
    free (pre_ppin);
    free (pre_ppout);
+   free (pre_transpout);
  }
  
  /* Dump PRE data.  */
*************** dump_pre_data (file)
*** 3962,3967 ****
--- 3969,3977 ----
  		       pre_ppin, n_basic_blocks);
    dump_sbitmap_vector (file, "PRE placement possible on outgoing", "BB",
  		       pre_ppout, n_basic_blocks);
+ 
+   dump_sbitmap_vector (file, "PRE transparent on outgoing", "BB",
+ 		       pre_transpout, n_basic_blocks);
  }
  
  /* Compute the local properties of each recorded expression.
*************** compute_pre_pavinout ()
*** 4129,4134 ****
--- 4139,4188 ----
      fprintf (gcse_file, "partially avail expr computation: %d passes\n", passes);
  }
  
+ /* Compute transparent outgoing information for each block.
+ 
+    An expression is transparent to an edge unless it is killed by
+    the edge itself.  This can only happen when the edge is traversed
+    through a call, as may happen with non-local labels and exceptions.  */
+ 
+ static void
+ compute_pre_transpout ()
+ {
+   int bb;
+ 
+   sbitmap_vector_ones (pre_transpout, n_basic_blocks);
+ 
+   for (bb = 0; bb < n_basic_blocks; ++bb)
+     {
+       int i;
+ 
+       /* Note that flow inserted a nop a the end of basic blocks that
+ 	 end in call instructions for reasons other than abnormal
+ 	 control flow.  */
+       if (GET_CODE (BLOCK_END (bb)) != CALL_INSN)
+ 	continue;
+ 
+       for (i = 0; i < expr_hash_table_size; i++)
+ 	{
+ 	  struct expr *expr;
+ 	  for (expr = expr_hash_table[i]; expr ; expr = expr->next_same_hash)
+ 	    if (GET_CODE (expr->expr) == MEM)
+ 	      {
+ 		rtx addr = XEXP (expr->expr, 0);
+ 
+ 		if (GET_CODE (addr) == SYMBOL_REF
+ 		    && CONSTANT_POOL_ADDRESS_P (addr))
+ 		  continue;
+ 		
+ 		/* ??? Optimally, we would use interprocedural alias
+ 		   analysis to determine if this mem is actually killed
+ 		   by this call.  */
+ 		RESET_BIT (pre_transpout[bb], expr->bitmap_index);
+ 	      }
+ 	}
+     }
+ }   
+ 
  /* Compute "placement possible" information on entrance and exit of
     each block.
  
*************** compute_pre_ppinout ()
*** 4209,4219 ****
        for (bb = 0; bb < n_basic_blocks - 1; bb++)
  	{
  	  sbitmap_ptr ppout = pre_ppout[bb]->elms;
  
  	  for (i = 0; i < size; i++)
  	    {
  	      int_list_ptr succ;
! 	      SBITMAP_ELT_TYPE tmp = -1L;
  
  	      for (succ = s_succs[bb]; succ != NULL; succ = succ->next)
  		{
--- 4263,4274 ----
        for (bb = 0; bb < n_basic_blocks - 1; bb++)
  	{
  	  sbitmap_ptr ppout = pre_ppout[bb]->elms;
+ 	  sbitmap_ptr transpout = pre_transpout[bb]->elms;
  
  	  for (i = 0; i < size; i++)
  	    {
  	      int_list_ptr succ;
! 	      SBITMAP_ELT_TYPE tmp = *transpout;
  
  	      for (succ = s_succs[bb]; succ != NULL; succ = succ->next)
  		{
*************** compute_pre_ppinout ()
*** 4226,4238 ****
  		  ppin = pre_ppin[succ_bb]->elms + i;
  		  tmp &= *ppin;
  		}
  	      if (*ppout != tmp)
  		{
  		  changed = 1;
! 		  *ppout++ = tmp;
  		}
! 	      else
! 		ppout++;
  	    }
  	}
  
--- 4281,4294 ----
  		  ppin = pre_ppin[succ_bb]->elms + i;
  		  tmp &= *ppin;
  		}
+ 
  	      if (*ppout != tmp)
  		{
  		  changed = 1;
! 		  *ppout = tmp;
  		}
! 
! 	      ppout++; transpout++;
  	    }
  	}
  
*************** compute_pre_data ()
*** 4252,4257 ****
--- 4308,4314 ----
    compute_pre_avinout ();
    compute_pre_antinout ();
    compute_pre_pavinout ();
+   compute_pre_transpout ();
    compute_pre_ppinout ();
    if (gcse_file)
      fprintf (gcse_file, "\n");
*************** pre_insert_insn (expr, bb)
*** 4376,4385 ****
      }
    /* Likewise if the last insn is a call, as will happen in the presence
       of exception handling.  */
!   /* ??? The flag_exceptions test is not exact.  We don't know if we are
!      actually in an eh region.  Fix flow to tell us this.  */
!   else if (GET_CODE (insn) == CALL_INSN
! 	   && (current_function_has_nonlocal_label || flag_exceptions))
      {
        HARD_REG_SET parm_regs;
        int nparm_regs;
--- 4433,4439 ----
      }
    /* Likewise if the last insn is a call, as will happen in the presence
       of exception handling.  */
!   else if (GET_CODE (insn) == CALL_INSN)
      {
        HARD_REG_SET parm_regs;
        int nparm_regs;
*************** pre_insert_insn (expr, bb)
*** 4409,4415 ****
  	  {
  	    int regno = REGNO (XEXP (XEXP (p, 0), 0));
  	    if (regno >= FIRST_PSEUDO_REGISTER)
! 	      abort();
  	    SET_HARD_REG_BIT (parm_regs, regno);
  	    nparm_regs++;
  	  }
--- 4463,4469 ----
  	  {
  	    int regno = REGNO (XEXP (XEXP (p, 0), 0));
  	    if (regno >= FIRST_PSEUDO_REGISTER)
! 	      abort ();
  	    SET_HARD_REG_BIT (parm_regs, regno);
  	    nparm_regs++;
  	  }



More information about the Gcc-patches mailing list