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: [PATCH] Avoid extending reg lifetime when cprop_hardreg propagates into DEBUG_INSNs (PR debug/43161)


On Thu, 25 Feb 2010, Jakub Jelinek wrote:

> On Thu, Feb 25, 2010 at 12:25:43AM +0100, Jakub Jelinek wrote:
> > Good idea, I'll rework it using pool_alloc/pool_free (and of course at the
> > end there is no need to pool_free individually, free_alloc_pool will take
> > care about it.  Obstack wouldn't be so useful, because within bb's we pretty
> > often can commit the DEBUG_INSN changes and thus free immediately, and
> > for ends of bb's on the other side we IMHO want to keep them until end (to
> > avoid running df_analyze more than once).
> 
> Here it is, bootstrapped/regtested on x86_64-linux and i686-linux.

Ok.

Thanks,
Richard.

> 2010-02-25  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/43161
> 	* regcprop.c (struct queued_debug_insn_change): New type.
> 	(struct value_data_entry): Add debug_insn_changes field.
> 	(struct value_data): Add n_debug_insn_changes field.
> 	(debug_insn_changes_pool): New variable.
> 	(free_debug_insn_changes, apply_debug_insn_changes,
> 	cprop_find_used_regs_1, cprop_find_used_regs): New functions.
> 	(kill_value_one_regno): Call free_debug_insn_changes if needed.
> 	(init_value_data): Clear debug_insn_changes and n_debug_insn_changes
> 	fields.
> 	(replace_oldest_value_reg): Don't change DEBUG_INSNs, instead queue
> 	changes for them.
> 	(copyprop_hardreg_forward_1): Don't call apply_change_group for
> 	DEBUG_INSNs.  For a real insn, if there are queued DEBUG_INSN
> 	changes, call cprop_find_used_regs via note_stores.
> 	(copyprop_hardreg_forward): When copying vd from predecessor
> 	which has any queued DEBUG_INSN changes, make sure the pointers are
> 	cleared.  At the end call df_analyze and then if there are any
> 	DEBUG_INSN changes queued at the end of some basic block for still
> 	live registers, apply them.
> 	(pass_cprop_hardreg): Set TODO_df_finish in todo_flags_finish.
> 
> --- gcc/regcprop.c.jj	2010-02-23 18:09:24.000000000 +0100
> +++ gcc/regcprop.c	2010-02-25 11:10:37.000000000 +0100
> @@ -46,6 +46,18 @@
>     up some silly register allocation decisions made by reload.  This
>     code may be obsoleted by a new register allocator.  */
>  
> +/* DEBUG_INSNs aren't changed right away, as doing so might extend the
> +   lifetime of a register and get the DEBUG_INSN subsequently reset.
> +   So they are queued instead, and updated only when the register is
> +   used in some subsequent real insn before it is set.  */
> +struct queued_debug_insn_change
> +{
> +  struct queued_debug_insn_change *next;
> +  rtx insn;
> +  rtx *loc;
> +  rtx new_rtx;
> +};
> +
>  /* For each register, we have a list of registers that contain the same
>     value.  The OLDEST_REGNO field points to the head of the list, and
>     the NEXT_REGNO field runs through the list.  The MODE field indicates
> @@ -57,14 +69,18 @@ struct value_data_entry
>    enum machine_mode mode;
>    unsigned int oldest_regno;
>    unsigned int next_regno;
> +  struct queued_debug_insn_change *debug_insn_changes;
>  };
>  
>  struct value_data
>  {
>    struct value_data_entry e[FIRST_PSEUDO_REGISTER];
>    unsigned int max_value_regs;
> +  unsigned int n_debug_insn_changes;
>  };
>  
> +static alloc_pool debug_insn_changes_pool;
> +
>  static void kill_value_one_regno (unsigned, struct value_data *);
>  static void kill_value_regno (unsigned, unsigned, struct value_data *);
>  static void kill_value (rtx, struct value_data *);
> @@ -91,6 +107,22 @@ extern void debug_value_data (struct val
>  static void validate_value_data (struct value_data *);
>  #endif
>  
> +/* Free all queued updates for DEBUG_INSNs that change some reg to
> +   register REGNO.  */
> +
> +static void
> +free_debug_insn_changes (struct value_data *vd, unsigned int regno)
> +{
> +  struct queued_debug_insn_change *cur, *next;
> +  for (cur = vd->e[regno].debug_insn_changes; cur; cur = next)
> +    {
> +      next = cur->next;
> +      --vd->n_debug_insn_changes;
> +      pool_free (debug_insn_changes_pool, cur);
> +    }
> +  vd->e[regno].debug_insn_changes = NULL;
> +}
> +
>  /* Kill register REGNO.  This involves removing it from any value
>     lists, and resetting the value mode to VOIDmode.  This is only a
>     helper function; it does not handle any hard registers overlapping
> @@ -118,6 +150,8 @@ kill_value_one_regno (unsigned int regno
>    vd->e[regno].mode = VOIDmode;
>    vd->e[regno].oldest_regno = regno;
>    vd->e[regno].next_regno = INVALID_REGNUM;
> +  if (vd->e[regno].debug_insn_changes)
> +    free_debug_insn_changes (vd, regno);
>  
>  #ifdef ENABLE_CHECKING
>    validate_value_data (vd);
> @@ -204,8 +238,10 @@ init_value_data (struct value_data *vd)
>        vd->e[i].mode = VOIDmode;
>        vd->e[i].oldest_regno = i;
>        vd->e[i].next_regno = INVALID_REGNUM;
> +      vd->e[i].debug_insn_changes = NULL;
>      }
>    vd->max_value_regs = 0;
> +  vd->n_debug_insn_changes = 0;
>  }
>  
>  /* Called through note_stores.  If X is clobbered, kill its value.  */
> @@ -446,6 +482,24 @@ replace_oldest_value_reg (rtx *loc, enum
>    rtx new_rtx = find_oldest_value_reg (cl, *loc, vd);
>    if (new_rtx)
>      {
> +      if (DEBUG_INSN_P (insn))
> +	{
> +	  struct queued_debug_insn_change *change;
> +
> +	  if (dump_file)
> +	    fprintf (dump_file, "debug_insn %u: queued replacing reg %u with %u\n",
> +		     INSN_UID (insn), REGNO (*loc), REGNO (new_rtx));
> +
> +	  change = (struct queued_debug_insn_change *)
> +		   pool_alloc (debug_insn_changes_pool);
> +	  change->next = vd->e[REGNO (new_rtx)].debug_insn_changes;
> +	  change->insn = insn;
> +	  change->loc = loc;
> +	  change->new_rtx = new_rtx;
> +	  vd->e[REGNO (new_rtx)].debug_insn_changes = change;
> +	  ++vd->n_debug_insn_changes;
> +	  return true;
> +	}
>        if (dump_file)
>  	fprintf (dump_file, "insn %u: replaced reg %u with %u\n",
>  		 INSN_UID (insn), REGNO (*loc), REGNO (new_rtx));
> @@ -622,6 +676,58 @@ replace_oldest_value_mem (rtx x, rtx ins
>  				    GET_MODE (x), insn, vd);
>  }
>  
> +/* Apply all queued updates for DEBUG_INSNs that change some reg to
> +   register REGNO.  */
> +
> +static void
> +apply_debug_insn_changes (struct value_data *vd, unsigned int regno)
> +{
> +  struct queued_debug_insn_change *change;
> +  rtx last_insn = vd->e[regno].debug_insn_changes->insn;
> +
> +  for (change = vd->e[regno].debug_insn_changes;
> +       change;
> +       change = change->next)
> +    {
> +      if (last_insn != change->insn)
> +	{
> +	  apply_change_group ();
> +	  last_insn = change->insn;
> +	}
> +      validate_change (change->insn, change->loc, change->new_rtx, 1);
> +    }
> +  apply_change_group ();
> +}
> +
> +/* Called via for_each_rtx, for all used registers in a real
> +   insn apply DEBUG_INSN changes that change registers to the
> +   used register.  */
> +
> +static int
> +cprop_find_used_regs_1 (rtx *loc, void *data)
> +{
> +  if (REG_P (*loc))
> +    {
> +      struct value_data *vd = (struct value_data *) data;
> +      if (vd->e[REGNO (*loc)].debug_insn_changes)
> +	{
> +	  apply_debug_insn_changes (vd, REGNO (*loc));
> +	  free_debug_insn_changes (vd, REGNO (*loc));
> +	}
> +    }
> +  return 0;
> +}
> +
> +/* Called via note_uses, for all used registers in a real insn
> +   apply DEBUG_INSN changes that change registers to the used
> +   registers.  */
> +
> +static void
> +cprop_find_used_regs (rtx *loc, void *vd)
> +{
> +  for_each_rtx (loc, cprop_find_used_regs_1, vd);
> +}
> +
>  /* Perform the forward copy propagation on basic block BB.  */
>  
>  static bool
> @@ -643,15 +749,10 @@ copyprop_hardreg_forward_1 (basic_block 
>  	  if (DEBUG_INSN_P (insn))
>  	    {
>  	      rtx loc = INSN_VAR_LOCATION_LOC (insn);
> -	      if (!VAR_LOC_UNKNOWN_P (loc)
> -		  && replace_oldest_value_addr (&INSN_VAR_LOCATION_LOC (insn),
> -						ALL_REGS, GET_MODE (loc),
> -						insn, vd))
> -		{
> -		  changed = apply_change_group ();
> -		  gcc_assert (changed);
> -		  anything_changed = true;
> -		}
> +	      if (!VAR_LOC_UNKNOWN_P (loc))
> +		replace_oldest_value_addr (&INSN_VAR_LOCATION_LOC (insn),
> +					   ALL_REGS, GET_MODE (loc),
> +					   insn, vd);
>  	    }
>  
>  	  if (insn == BB_END (bb))
> @@ -684,6 +785,10 @@ copyprop_hardreg_forward_1 (basic_block 
>  	    recog_data.operand_type[i] = OP_INOUT;
>  	}
>  
> +      /* Apply changes to earlier DEBUG_INSNs if possible.  */
> +      if (vd->n_debug_insn_changes)
> +	note_uses (&PATTERN (insn), cprop_find_used_regs, vd);
> +
>        /* For each earlyclobber operand, zap the value data.  */
>        for (i = 0; i < n_ops; i++)
>  	if (recog_op_alt[i][alt].earlyclobber)
> @@ -871,12 +976,18 @@ copyprop_hardreg_forward (void)
>    struct value_data *all_vd;
>    basic_block bb;
>    sbitmap visited;
> +  bool analyze_called = false;
>  
>    all_vd = XNEWVEC (struct value_data, last_basic_block);
>  
>    visited = sbitmap_alloc (last_basic_block);
>    sbitmap_zero (visited);
>  
> +  if (MAY_HAVE_DEBUG_STMTS)
> +    debug_insn_changes_pool
> +      = create_alloc_pool ("debug insn changes pool",
> +			   sizeof (struct queued_debug_insn_change), 256);
> +
>    FOR_EACH_BB (bb)
>      {
>        SET_BIT (visited, bb->index);
> @@ -888,13 +999,57 @@ copyprop_hardreg_forward (void)
>        if (single_pred_p (bb)
>  	  && TEST_BIT (visited, single_pred (bb)->index)
>  	  && ! (single_pred_edge (bb)->flags & (EDGE_ABNORMAL_CALL | EDGE_EH)))
> -	all_vd[bb->index] = all_vd[single_pred (bb)->index];
> +	{
> +	  all_vd[bb->index] = all_vd[single_pred (bb)->index];
> +	  if (all_vd[bb->index].n_debug_insn_changes)
> +	    {
> +	      unsigned int regno;
> +
> +	      for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
> +		{
> +		  if (all_vd[bb->index].e[regno].debug_insn_changes)
> +		    {
> +		      all_vd[bb->index].e[regno].debug_insn_changes = NULL;
> +		      if (--all_vd[bb->index].n_debug_insn_changes == 0)
> +			break;
> +		    }
> +		}
> +	    }
> +	}
>        else
>  	init_value_data (all_vd + bb->index);
>  
>        copyprop_hardreg_forward_1 (bb, all_vd + bb->index);
>      }
>  
> +  if (MAY_HAVE_DEBUG_STMTS)
> +    {
> +      FOR_EACH_BB (bb)
> +	if (TEST_BIT (visited, bb->index)
> +	    && all_vd[bb->index].n_debug_insn_changes)
> +	  {
> +	    unsigned int regno;
> +	    bitmap live;
> +
> +	    if (!analyze_called)
> +	      {
> +		df_analyze ();
> +		analyze_called = true;
> +	      }
> +	    live = df_get_live_out (bb);
> +	    for (regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
> +	      if (all_vd[bb->index].e[regno].debug_insn_changes)
> +		{
> +		  if (REGNO_REG_SET_P (live, regno))
> +		    apply_debug_insn_changes (all_vd + bb->index, regno);
> +		  if (all_vd[bb->index].n_debug_insn_changes == 0)
> +		    break;
> +		}
> +	  }
> +
> +      free_alloc_pool (debug_insn_changes_pool);
> +    }
> +
>    sbitmap_free (visited);
>    free (all_vd);
>    return 0;
> @@ -1026,6 +1181,7 @@ struct rtl_opt_pass pass_cprop_hardreg =
>    0,                                    /* properties_provided */
>    0,                                    /* properties_destroyed */
>    0,                                    /* todo_flags_start */
> -  TODO_dump_func | TODO_verify_rtl_sharing /* todo_flags_finish */
> +  TODO_dump_func | TODO_df_finish
> +  | TODO_verify_rtl_sharing		/* todo_flags_finish */
>   }
>  };
> 
> 
> 	Jakub
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex


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