| Message ID | f91e18ec-f614-5744-afbb-380d17c035bf@mentor.com |
|---|---|
| State | New |
| Headers | show |
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. > >
Index: ipa-icf.c =================================================================== --- ipa-icf.c (revision 239624) +++ ipa-icf.c (working copy) @@ -2133,6 +2133,23 @@ sem_variable::get_hash (void) return m_hash; } +/* Set all points-to UIDs of aliases pointing to node N as UID. */ + +static void +set_alias_uids (symtab_node *n, int uid) +{ + ipa_ref *ref; + FOR_EACH_ALIAS (n, ref) + { + if (dump_file) + fprintf (dump_file, " Setting points-to UID of [%s] as %d\n", + xstrdup_for_dump (ref->referring->asm_name ()), uid); + + SET_DECL_PT_UID (ref->referring->decl, uid); + set_alias_uids (ref->referring, uid); + } +} + /* Merges instance with an ALIAS_ITEM, where alias, thunk or redirection can be applied. */ @@ -2258,12 +2275,11 @@ sem_variable::merge (sem_item *alias_item) varpool_node::create_alias (alias_var->decl, decl); alias->resolve_alias (original); - if (DECL_PT_UID_SET_P (original->decl)) - SET_DECL_PT_UID (alias->decl, DECL_PT_UID (original->decl)); if (dump_file) - fprintf (dump_file, "Unified; Variable alias has been created.\n\n"); + fprintf (dump_file, "Unified; Variable alias has been created.\n"); + set_alias_uids (original, DECL_UID (original->decl)); return true; } }