Re: [PATCH] Another canonical cselib_val bootstrap fix (PR bootstrap/51725)

[Richard Guenther <rguenther at suse dot de>]

On Tue, 3 Jan 2012, Jakub Jelinek wrote:

> Hi!
> 
> My previous patch apparently wasn't enough.  If at add_mem_*
> time mem_elt is still its own canonical cselib_val, but only afterwards
> new_elt_loc_list adds a new canonical cselib_val to it (and moves

Hm, shouldn't an existing cselib_val be always the canonical one?

> over its locs), then we can still crash in cselib_invalidate_mem.
> This patch ensures that new_elt_loc_list when moving over the locs also
> adds the canonical cselib_val to first_containing_mem list if it
> wasn't already there and the old val was (it would be better to
> remove it, but the chain is only single linked list and it would be
> expensive to remove it there - the next cselib_invalidate_mem
> will handle it automatically, as non-canonical values don't have any mem
> locs and thus are removed from the chain).
> In cselib_invalidate_mem it needs to call canonical_cselib_val on the
> addr_list chain elts (those aren't canonicalized by anything).
> There is no need to call canonical_cselib_val on v, the new_elt_loc_list
> change ensures that the canonical value is in the list always too
> and non-canonical values don't have MEM locs.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, tested with
> cross jc1 to ia64-linux on gnu-CORBA.list compilation.  Ok for trunk?
> 
> 2012-01-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR bootstrap/51725
> 	* cselib.c (new_elt_loc_list): When moving locs from one
> 	cselib_val to its new canonical_cselib_val and the
> 	cselib_val was in first_containing_mem chain, but
> 	the canonical_cselib_val was not, add the latter into the
> 	chain.
> 	(cselib_invalidate_mem): Compare canonical_cselib_val of
> 	addr_list chain elt with v.
> 
> --- gcc/cselib.c.jj	2012-01-03 16:22:48.000000000 +0100
> +++ gcc/cselib.c	2012-01-03 17:29:10.096229315 +0100
> @@ -277,6 +277,12 @@ new_elt_loc_list (cselib_val *val, rtx l
>  	    }
>  	  el->next = val->locs;
>  	  next = val->locs = CSELIB_VAL_PTR (loc)->locs;
> +	  if (CSELIB_VAL_PTR (loc)->next_containing_mem != NULL
> +	      && val->next_containing_mem == NULL)
> +	    {
> +	      val->next_containing_mem = first_containing_mem;
> +	      first_containing_mem = val;
> +	    }
>  	}
>  
>        /* Chain LOC back to VAL.  */
> @@ -2211,7 +2217,7 @@ cselib_invalidate_mem (rtx mem_rtx)
>  	  mem_chain = &addr->addr_list;
>  	  for (;;)
>  	    {
> -	      if ((*mem_chain)->elt == v)
> +	      if (canonical_cselib_val ((*mem_chain)->elt) == v)
>  		{
>  		  unchain_one_elt_list (mem_chain);
>  		  break;
> 
> 	Jakub
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer