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