Re: [PATCH, PR 43812] Dissolve same_comdat_groups when localizing decls in function_and_variable_visibility

[Richard Guenther <richard dot guenther at gmail dot com>]

On Tue, Apr 27, 2010 at 12:10 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> the patch below was approved by Honza on IRC and so I committed it as
> revision 158777. ?I intend to commit it to the 4.5 branch early next
> week - should I omit the additions to the verifier?

The verifier changes are fine as the branch is built with
release checking by default.

RIchard.

> Thanks,
>
> Martin
>
> On Wed, Apr 21, 2010 at 06:17:56PM +0200, Martin Jambor wrote:
>> Hi,
>>
>> PR 43812 is caused by the fact that ipa-cp asserts that there is no
>> function which has no incoming edges and yet cannot be called
>> indirectly or from outside the current compilation unit. ?That is
>> normally true because we remove all unreachable functions before
>> IPA-CP. ?However, at -fwhole-program we keep one such function because
>> it is in a same_comdat_group list even though
>> function_and_variable_visibility has made all of the functions in the
>> list local. ?The fix is to make the function dissolve the list in the
>> process as well.
>>
>> This patch also adds stuff to the call graph verifier that checks the
>> list is indeed circular, it has more than one member and that all of
>> the members have their fndecl in a DECL_COMDAT_GROUP (this last
>> property would have caught the problem).
>>
>> Bootstrapped and tested on x86-64-linux without any problems. ?OK fro
>> trunk and the 4.5 branch (without the verifier changes)?
>>
>> Thanks,
>>
>> Martin
>>
>> 2010-04-21 ?Martin Jambor ?<mjambor@suse.cz>
>>
>> ? ? ? PR middle-end/43812
>> ? ? ? * ipa.c (dissolve_same_comdat_group_list): New function.
>> ? ? ? (function_and_variable_visibility): Call
>> ? ? ? dissolve_same_comdat_group_list when comdat group contains external or
>> ? ? ? newly local nodes.
>> ? ? ? * cgraphunit.c (verify_cgraph_node): Verify that same_comdat_group
>> ? ? ? lists are circular and that they contain only DECL_ONE_ONLY nodes.
>>
>> ? ? ? * testsuite/g++.dg/ipa/pr43812.C: New test.
>>
>>
>> Index: icln/gcc/ipa.c
>> ===================================================================
>> --- icln.orig/gcc/ipa.c
>> +++ icln/gcc/ipa.c
>> @@ -353,6 +353,21 @@ cgraph_externally_visible_p (struct cgra
>> ? ?return false;
>> ?}
>>
>> +/* Dissolve the same_comdat_group list in which NODE resides. ?*/
>> +
>> +static void
>> +dissolve_same_comdat_group_list (struct cgraph_node *node)
>> +{
>> + ?struct cgraph_node *n = node, *next;
>> + ?do
>> + ? ?{
>> + ? ? ?next = n->same_comdat_group;
>> + ? ? ?n->same_comdat_group = NULL;
>> + ? ? ?n = next;
>> + ? ?}
>> + ?while (n != node);
>> +}
>> +
>> ?/* Mark visibility of all functions.
>>
>> ? ? A local function is one whose calls can occur only in the current
>> @@ -383,17 +398,17 @@ function_and_variable_visibility (bool w
>> ? ? ? ?and simplifies later passes. ?*/
>> ? ? ? ?if (node->same_comdat_group && DECL_EXTERNAL (node->decl))
>> ? ? ? {
>> - ? ? ? struct cgraph_node *n = node, *next;
>> - ? ? ? do
>> - ? ? ? ? {
>> +#ifdef ENABLE_CHECKING
>> + ? ? ? struct cgraph_node *n;
>> +
>> + ? ? ? for (n = node->same_comdat_group;
>> + ? ? ? ? ? ?n != node;
>> + ? ? ? ? ? ?n = n->same_comdat_group)
>> ? ? ? ? ? ? /* If at least one of same comdat group functions is external,
>> ? ? ? ? ? ? ? ?all of them have to be, otherwise it is a front-end bug. ?*/
>> ? ? ? ? ? ? gcc_assert (DECL_EXTERNAL (n->decl));
>> - ? ? ? ? ? next = n->same_comdat_group;
>> - ? ? ? ? ? n->same_comdat_group = NULL;
>> - ? ? ? ? ? n = next;
>> - ? ? ? ? }
>> - ? ? ? while (n != node);
>> +#endif
>> + ? ? ? dissolve_same_comdat_group_list (node);
>> ? ? ? }
>> ? ? ? ?gcc_assert ((!DECL_WEAK (node->decl) && !DECL_COMDAT (node->decl))
>> ? ? ? ? ? ? ? ? ? ? ? ? || TREE_PUBLIC (node->decl) || DECL_EXTERNAL (node->decl));
>> @@ -409,6 +424,12 @@ function_and_variable_visibility (bool w
>> ? ? ? {
>> ? ? ? ? gcc_assert (whole_program || !TREE_PUBLIC (node->decl));
>> ? ? ? ? cgraph_make_decl_local (node->decl);
>> + ? ? ? if (node->same_comdat_group)
>> + ? ? ? ? /* cgraph_externally_visible_p has already checked all other nodes
>> + ? ? ? ? ? ?in the group and they will all be made local. ?We need to
>> + ? ? ? ? ? ?dissolve the group at once so that the predicate does not
>> + ? ? ? ? ? ?segfault though. */
>> + ? ? ? ? dissolve_same_comdat_group_list (node);
>> ? ? ? }
>> ? ? ? ?node->local.local = (cgraph_only_called_directly_p (node)
>> ? ? ? ? ? ? ? ? ? ? ? ? ?&& node->analyzed
>> Index: icln/gcc/testsuite/g++.dg/ipa/pr43812.C
>> ===================================================================
>> --- /dev/null
>> +++ icln/gcc/testsuite/g++.dg/ipa/pr43812.C
>> @@ -0,0 +1,38 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -fwhole-program -fipa-cp" } */
>> +
>> +typedef float scoord_t;
>> +typedef scoord_t sdist_t;
>> +typedef sdist_t dist_t;
>> +template<typename T> class TRay { };
>> +typedef TRay<dist_t> Ray;
>> +class BBox { };
>> +class RenderContext { };
>> +class RefCounted {
>> +public:
>> + ? ?void deref () const {
>> + ? ? ? ?if (--ref_count <= 0) {
>> + ? ? ? ? ? ?delete this;
>> + ? ? ? ?}
>> + ? ?}
>> + ? ?mutable int ref_count;
>> +};
>> +template<class T> class Ref {
>> +public:
>> + ? ?~Ref () {
>> + ? ? ? ?if (obj) obj->deref ();
>> + ? ?}
>> + ? ?T *obj;
>> +};
>> +class Material : public RefCounted { };
>> +class Surface {
>> +public:
>> + ? ?virtual ~Surface () { }
>> + ? ?class IsecInfo ? { };
>> + ? ?virtual const IsecInfo *intersect (Ray &ray, RenderContext &context) const;
>> + ? ?Ref<const Material> material;
>> +};
>> +class LocalSurface : public Surface {
>> + ? ?virtual BBox bbox () const;
>> +};
>> +BBox LocalSurface::bbox () const { }
>> Index: icln/gcc/cgraphunit.c
>> ===================================================================
>> --- icln.orig/gcc/cgraphunit.c
>> +++ icln/gcc/cgraphunit.c
>> @@ -714,6 +714,32 @@ verify_cgraph_node (struct cgraph_node *
>> ? ? ? ?error ("double linked list of clones corrupted");
>> ? ? ? ?error_found = true;
>> ? ? ?}
>> + ?if (node->same_comdat_group)
>> + ? ?{
>> + ? ? ?struct cgraph_node *n = node->same_comdat_group;
>> +
>> + ? ? ?if (!DECL_ONE_ONLY (node->decl))
>> + ? ? {
>> + ? ? ? error ("non-DECL_ONE_ONLY node in a same_comdat_group list");
>> + ? ? ? error_found = true;
>> + ? ? }
>> + ? ? ?if (n == node)
>> + ? ? {
>> + ? ? ? error ("node is alone in a comdat group");
>> + ? ? ? error_found = true;
>> + ? ? }
>> + ? ? ?do
>> + ? ? {
>> + ? ? ? if (!n->same_comdat_group)
>> + ? ? ? ? {
>> + ? ? ? ? ? error ("same_comdat_group is not a circular list");
>> + ? ? ? ? ? error_found = true;
>> + ? ? ? ? ? break;
>> + ? ? ? ? }
>> + ? ? ? n = n->same_comdat_group;
>> + ? ? }
>> + ? ? ?while (n != node);
>> + ? ?}
>>
>> ? ?if (node->analyzed && gimple_has_body_p (node->decl)
>> ? ? ? ?&& !TREE_ASM_WRITTEN (node->decl)
>