Another canonical cselib_val bootstrap fix (PR bootstrap/51725)

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
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.


	Jakub

On 01/04/2012 06:14 AM, Jakub Jelinek wrote:
> 	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.

Ok.


r~

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
> 
>

On Wed, Jan 04, 2012 at 10:33:50AM +0100, Richard Guenther wrote:
> > 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?

Canonical is the one with lower uid.  If cselib_add_permanent_equiv
is called with two existing cselib_vals, just one of them will be
canonical.

	Jakub