patch for computing conditional register lifetimes in flow

Jim Wilson wilson@cygnus.com
Fri Mar 23 16:01:00 GMT 2001


This fixes an ia64-linux -O2 compiler abort.  The original testcase is bash.
A reduced testcase is at the end.

The problem here is that the flow code for computing conditional register
life info has a flaw.  It returns different results for different instruction
orders.  So we compute register life into before the scheduler and get one
answer.  We schedule.  We compute register life into after the scheduler,
get a different answer, and hit the consistency checking abort.

The example looks something like this before scheduling
	cond_exec (eq p6 0) (set r8 ...)
	cond_exec (eq p6 0) (use r8)
	cond_exec (ne p6 0) (set r8 ...)
	cond_exec (ne p6 0) (use r8)
	(set p8 (eq ...)
	if (eq p8 0) goto foo:
r8 is used at foo, but not in the block immediately afterwards.  When flow
processes this block, it sets the dead condition for r8 as (ne p8 0).
We then add in (eq p6 0) and then remove it.  We then add in (ne p6 0)
and then remove it.  When we get to the beginning of the block, we think
that r8 is still live.  This is wrong but it is harmless by itself.

Now we schedule, and get something like this:
	cond_exec (eq p6 0) (set r8 ...)
	cond_exec (ne p6 0) (set r8 ...)
	cond_exec (eq p6 0) (use r8)
	cond_exec (ne p6 0) (use r8)
	(set p8 (eq ...)
	if (eq p8 0) goto foo:
This time, we start with (ne p8 0), add in (eq p6 0), add in (ne p6 0),
and at this point we now know that the register is unconditionally live.
We then remove (ne p6 0), and then remove (eq p6 0), and then we know that
the register is unconditionally dead.  Since this answer is different than
the one we got before, we hit an abort.

To fix this I added code to keep track of two more values.  In addition to
keeping track of the conditional dead status of a register, we also keep
track of the initial value for the dead status, and we keep track of all
conditional stores in the basic block.  If we get to the beginning of the
basic block, and we see that all of the conditional stores sum into an
unconditional store, and the dead status is back at the original value that
we initialized it to, then the register must be unconditionally dead.

To make this work, I also needed to add code to and_reg_cond to avoid
redundant conditions.  Given something like
	and_reg_cond ((AND foo bar), foo, 1)
it would return (AND (AND foo bar) foo).  The redundant condition confused
later simplifications.

Also, I noticed a few places where we cleared fields before freeing memory,
which served no useful purpose.

If anyone sees a flaw with this patch, or sees a simpler way to solve this
problem, please let me know.  If I don't get any comments I plan to check
this in early next week.

This was tested with an ia64-linux bootstrap and testsuite run.

2001-03-23  Jim Wilson  <wilson@redhat.com>

	* flow.c (struct reg_cond_life_info): New fields orig_condition
	and stores.
	(init_propagate_block_info): Set new fields.
	(mark_regno_cond_dead): Set and use new fields.
	(flush_reg_cond_reg_1): Likewise.
	(and_reg_cond, case AND): Check for redundant AND conditions.
	(mark_used_reg): Delete unnecessary clears before freeing splay trees.
	Set new fields.

Index: flow.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/flow.c,v
retrieving revision 1.374.2.3
diff -p -r1.374.2.3 flow.c
*** flow.c	2001/03/12 19:35:53	1.374.2.3
--- flow.c	2001/03/23 23:25:57
*************** static rtx tail_recursion_label_list;
*** 270,278 ****
  /* Holds information for tracking conditional register life information.  */
  struct reg_cond_life_info
  {
!   /* An EXPR_LIST of conditions under which a register is dead.  */
    rtx condition;
  
    /* ??? Could store mask of bytes that are dead, so that we could finally
       track lifetimes of multi-word registers accessed via subregs.  */
  };
--- 270,284 ----
  /* Holds information for tracking conditional register life information.  */
  struct reg_cond_life_info
  {
!   /* A boolean expression of conditions under which a register is dead.  */
    rtx condition;
+   /* Conditions under which a register is dead at the basic block end.  */
+   rtx orig_condition;
  
+   /* A boolean expression of conditions under which a register has been
+      stored into.  */
+   rtx stores;
+ 
    /* ??? Could store mask of bytes that are dead, so that we could finally
       track lifetimes of multi-word registers accessed via subregs.  */
  };
*************** init_propagate_block_info (bb, live, loc
*** 4076,4081 ****
--- 4082,4089 ----
  	       else
  		 cond = cond_true;
  	       rcli->condition = cond;
+ 	       rcli->stores = const0_rtx;
+ 	       rcli->orig_condition = cond;
  
  	       splay_tree_insert (pbi->reg_cond_dead, i,
  				  (splay_tree_value) rcli);
*************** mark_regno_cond_dead (pbi, regno, cond)
*** 5019,5024 ****
--- 5027,5034 ----
  	     which it is dead.  */
  	  rcli = (struct reg_cond_life_info *) xmalloc (sizeof (*rcli));
  	  rcli->condition = cond;
+ 	  rcli->stores = cond;
+ 	  rcli->orig_condition = const0_rtx;
  	  splay_tree_insert (pbi->reg_cond_dead, regno,
  			     (splay_tree_value) rcli);
  
*************** mark_regno_cond_dead (pbi, regno, cond)
*** 5034,5043 ****
  	  rcli = (struct reg_cond_life_info *) node->value;
  	  ncond = rcli->condition;
  	  ncond = ior_reg_cond (ncond, cond, 1);
! 
! 	  /* If the register is now unconditionally dead,
! 	     remove the entry in the splay_tree.  */
! 	  if (ncond == const1_rtx)
  	    splay_tree_remove (pbi->reg_cond_dead, regno);
  	  else
  	    {
--- 5044,5064 ----
  	  rcli = (struct reg_cond_life_info *) node->value;
  	  ncond = rcli->condition;
  	  ncond = ior_reg_cond (ncond, cond, 1);
! 	  if (rcli->stores == const0_rtx)
! 	    rcli->stores = cond;
! 	  else if (rcli->stores != const1_rtx)
! 	    rcli->stores = ior_reg_cond (rcli->stores, cond, 1);
! 
! 	  /* If the register is now unconditionally dead, remove the entry
! 	     in the splay_tree.  A register is unconditionally dead if the
! 	     dead condition ncond is true.  A register is also unconditionally
! 	     dead if the sum of all conditional stores is an unconditional
! 	     store (stores is true), and the dead condition is identically the
! 	     same as the original dead condition initialized at the end of
! 	     the block.  This is a pointer compare, not an rtx_equal_p
! 	     compare.  */
! 	  if (ncond == const1_rtx
! 	      || (ncond == rcli->orig_condition && rcli->stores == const1_rtx))
  	    splay_tree_remove (pbi->reg_cond_dead, regno);
  	  else
  	    {
*************** flush_reg_cond_reg_1 (node, data)
*** 5083,5088 ****
--- 5104,5111 ----
    /* Splice out portions of the expression that refer to regno.  */
    rcli = (struct reg_cond_life_info *) node->value;
    rcli->condition = elim_reg_cond (rcli->condition, regno);
+   if (rcli->stores != const0_rtx && rcli->stores != const1_rtx)
+     rcli->stores = elim_reg_cond (rcli->stores, regno);
  
    /* If the entire condition is now false, signal the node to be removed.  */
    if (rcli->condition == const0_rtx)
*************** and_reg_cond (old, x, add)
*** 5289,5294 ****
--- 5312,5328 ----
  	}
        if (! add)
  	return old;
+ 
+       /* If X is identical to one of the existing terms of the AND,
+ 	 then just return what we already have.  */
+       /* ??? There really should be some sort of recursive check here in
+ 	 case there are nested ANDs.  */
+       if ((GET_CODE (XEXP (old, 0)) == GET_CODE (x)
+ 	   && REGNO (XEXP (XEXP (old, 0), 0)) == REGNO (XEXP (x, 0)))
+ 	  || (GET_CODE (XEXP (old, 1)) == GET_CODE (x)
+ 	      && REGNO (XEXP (XEXP (old, 1), 0)) == REGNO (XEXP (x, 0))))
+ 	return old;
+ 
        return gen_rtx_AND (0, old, x);
  
      case NOT:
*************** mark_used_reg (pbi, reg, cond, insn)
*** 5772,5781 ****
  	      /* If the register is now unconditionally live, remove the
  		 entry in the splay_tree.  */
  	      if (ncond == const0_rtx)
! 		{
! 		  rcli->condition = NULL_RTX;
! 		  splay_tree_remove (pbi->reg_cond_dead, regno);
! 		}
  	      else
  		{
  		  rcli->condition = ncond;
--- 5806,5812 ----
  	      /* If the register is now unconditionally live, remove the
  		 entry in the splay_tree.  */
  	      if (ncond == const0_rtx)
! 		splay_tree_remove (pbi->reg_cond_dead, regno);
  	      else
  		{
  		  rcli->condition = ncond;
*************** mark_used_reg (pbi, reg, cond, insn)
*** 5789,5794 ****
--- 5820,5827 ----
  	     the condition under which it is still dead.  */
  	  rcli = (struct reg_cond_life_info *) xmalloc (sizeof (*rcli));
  	  rcli->condition = not_reg_cond (cond);
+ 	  rcli->stores = const0_rtx;
+ 	  rcli->orig_condition = const0_rtx;
  	  splay_tree_insert (pbi->reg_cond_dead, regno,
  			     (splay_tree_value) rcli);
  
*************** mark_used_reg (pbi, reg, cond, insn)
*** 5807,5814 ****
  	     unconditionally so.  Remove it from the conditionally dead
  	     list, so that a conditional set won't cause us to think
  	     it dead.  */
- 	  rcli = (struct reg_cond_life_info *) node->value;
- 	  rcli->condition = NULL_RTX;
  	  splay_tree_remove (pbi->reg_cond_dead, regno);
  	}
      }
--- 5840,5845 ----

typedef struct variable *DYNAMIC_FUNC ();

typedef struct variable {
  char *name;
  char *value;
  char *exportstr;
  DYNAMIC_FUNC *dynamic_value;
  DYNAMIC_FUNC *assign_func;
  int attributes;
  int context;
  struct variable *prev_context;
} SHELL_VAR;

extern SHELL_VAR *find_variable (char *);
extern SHELL_VAR *find_tempenv_variable (char *);
extern int array_needs_making;

void
set_var_attribute (name, attribute, undo)
     char *name;
     int attribute, undo;
{
  SHELL_VAR *var, *tv;
  if (undo)
    var = find_variable (name);
  else
    tv = find_tempenv_variable (name);
  if (var)
    ((undo == 0) ? ((var)->attributes |= (attribute)) :
((var)->attributes &= ~(attribute)));
  if (var && ((var)->attributes) & (0x001))
    array_needs_making++;
}



More information about the Gcc-patches mailing list