On Fri, 9 Sep 2016, Chung-Lin Tang wrote:
> On 2016/9/1 03:13 PM, Richard Biener wrote:
> > On Wed, 31 Aug 2016, Chung-Lin Tang wrote:
> >
> >> Hi Richard, Martin,
> >> this issue is actually sort of like PR 70856, basically the same ICE
> >> after IPA-ICF, due to DECL_PT_UIDs not consistent after reaching for the
> >> ultimate_alias_target().
> >>
> >> The reason this wasn't covered by the PR70856 fix is that, in this case,
> >> the DECL_PT_UID was not set in original->decl, evading the condition, and
> >> hence the the merged alias doesn't have DECL_PT_UID set, and causes the
> >> ICE in the second round of IPA-PTA (under -fopenacc).
> >>
> >> My fix is to simply remove the DECL_PT_UID_SET_P (original->decl) guard,
> >> and allow the DECL_PT_UID to be set using the DECL_UID in this case.
> >>
> >> Does this fix make sense?
> >> Testing does show no regressions, and the PR testcase ICE is fixed.
> >
> > If original is the ultimate alias target then this is indeed correct.
> >
> > The IPA PTA code asserts that all decls that are an alias of the ultimate
> > alias target X have the same DECL_PT_UID as X DECL_UID (or if the new
> > alias doesn't have DECL_PT_UID set it will set it that way).
> >
> > So I'd say it should be
> >
> > SET_DECL_PT_UID (alias->decl, DECL_UID (original->decl));
> >
> > instead.
> >
> > If IPA-ICF would merge
> >
> > static const int x = 1;
> > static const int xa __attribute__((alias("x")));
> > static const int y = 1;
> > static const int ya __attribute__((alias("y")));
> >
> > we'd initially have DECL_PT_UID (xa) be DECL_UID (x) and
> > DECL_PT_UID (ya) be DECL_UID (y). Depending on how exactly IPA-ICF
> > performs the merging of x and y (and thus adjusts the existing aliases)
> > IPA-ICF needs to adjust DECL_PT_UID of all aliases of the new
> > "ultimate alias target". I don't think the current code with the
> > proposed patch makes sure this will happen.
> >
> > Thanks,
> > Richard.
>
> How about this fix? The code now sets all DECLs reachable through referring
> IPA_REF_ALIAS references. I am not sure of the possibility of cycles,
> though it appears there's other code catching alias cycles, issuing an error
> to the user.
>
> Again tested without regressions.
Looks good to me.
Thanks,
Richard.
> Thanks,
> Chung-Lin
>
> * ipa-icf.c (set_alias_uids): New function.
> (sem_variable::merge): Use set_alias_uids to set DECL_PT_UID of
> all the merged variable's referring aliases.
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
