Message ID | 20120103191457.GZ18937@tyan-ft48-01.lab.bos.redhat.com |
---|---|
State | New |
Headers | show |
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
--- 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;