Re: [PATCH] Another canonical cselib_val bootstrap fix (PR bootstrap/51725)
On Fri, Jan 06, 2012 at 05:30:52PM -0200, Alexandre Oliva wrote: > Regstrapped on x86_64-linux-gnu and i686-linux-gnu, also verified with > gnu-CORBA.list with a ia64-linux-gnu cross. > > Ok to install? Ok. Jakub
Re: [PATCH] Another canonical cselib_val bootstrap fix (PR bootstrap/51725)
On Jan 3, 2012, Jakub Jelinek wrote:
> My previous patch apparently wasn't enough.
I think we need to propagate addr_lists to the canonical value too, and
I found other spots that AFAICT require or would benefit from canonical
values.
Regstrapped on x86_64-linux-gnu and i686-linux-gnu, also verified with
gnu-CORBA.list with a ia64-linux-gnu cross.
Ok to install?
for gcc/ChangeLog
from Alexandre Oliva
PR bootstrap/51725
* cselib.c (new_elt_loc_list): Promote addr_list to canonical node.
Add canonical node to containing_mem chain after the non-canonical
one, even if there weren't any locs to propagate.
(remove_useless_values): Keep only canonical values.
(add_mem_for_addr, cselib_lookup_mem): Canonicalize addr.
(cselib_invalidate_mem): Likewise. Ensure v is canonical, and
canonicalize mem_chain elements that are not discarded.
Index: gcc/cselib.c
===
--- gcc/cselib.c.orig 2012-01-06 14:15:37.049194330 -0200
+++ gcc/cselib.c 2012-01-06 14:18:38.954548069 -0200
@@ -277,12 +277,27 @@ 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;
- }
+ }
+
+ if (CSELIB_VAL_PTR (loc)->addr_list)
+ {
+ /* Bring in addr_list into canonical node. */
+ struct elt_list *last = CSELIB_VAL_PTR (loc)->addr_list;
+ while (last->next)
+ last = last->next;
+ last->next = val->addr_list;
+ val->addr_list = CSELIB_VAL_PTR (loc)->addr_list;
+ CSELIB_VAL_PTR (loc)->addr_list = NULL;
+ }
+
+ if (CSELIB_VAL_PTR (loc)->next_containing_mem != NULL
+ && val->next_containing_mem == NULL)
+ {
+ /* Add VAL to the containing_mem list after LOC. LOC will
+ be removed when we notice it doesn't contain any
+ MEMs. */
+ val->next_containing_mem = CSELIB_VAL_PTR (loc)->next_containing_mem;
+ CSELIB_VAL_PTR (loc)->next_containing_mem = val;
}
/* Chain LOC back to VAL. */
@@ -641,7 +656,7 @@ remove_useless_values (void)
p = &first_containing_mem;
for (v = *p; v != &dummy_val; v = v->next_containing_mem)
-if (v->locs)
+if (v->locs && v == canonical_cselib_val (v))
{
*p = v;
p = &(*p)->next_containing_mem;
@@ -1270,6 +1285,7 @@ add_mem_for_addr (cselib_val *addr_elt,
{
struct elt_loc_list *l;
+ addr_elt = canonical_cselib_val (addr_elt);
mem_elt = canonical_cselib_val (mem_elt);
/* Avoid duplicates. */
@@ -1318,6 +1334,7 @@ cselib_lookup_mem (rtx x, int create)
if (! addr)
return 0;
+ addr = canonical_cselib_val (addr);
/* Find a value that describes a value of our mode at that address. */
for (l = addr->addr_list; l; l = l->next)
if (GET_MODE (l->elt->val_rtx) == mode)
@@ -2214,15 +2231,22 @@ cselib_invalidate_mem (rtx mem_rtx)
/* We must have a mapping from this MEM's address to the
value (E). Remove that, too. */
addr = cselib_lookup (XEXP (x, 0), VOIDmode, 0, GET_MODE (x));
+ addr = canonical_cselib_val (addr);
+ gcc_checking_assert (v == canonical_cselib_val (v));
mem_chain = &addr->addr_list;
for (;;)
{
- if (canonical_cselib_val ((*mem_chain)->elt) == v)
+ cselib_val *canon = canonical_cselib_val ((*mem_chain)->elt);
+
+ if (canon == v)
{
unchain_one_elt_list (mem_chain);
break;
}
+ /* Record canonicalized elt. */
+ (*mem_chain)->elt = canon;
+
mem_chain = &(*mem_chain)->next;
}
--
Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: [PATCH] Another canonical cselib_val bootstrap fix (PR bootstrap/51725)
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
Re: [PATCH] Another canonical cselib_val bootstrap fix (PR bootstrap/51725)
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
>
> 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.0 +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
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
Re: [PATCH] Another canonical cselib_val bootstrap fix (PR bootstrap/51725)
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~
