Re: IPCP & versioning additions and fixes for tree profiling branch

[Steven Bosscher <stevenb at suse dot de>]

On Sunday 20 March 2005 16:21, Jan Hubicka wrote:
> > Index: cgraphunit.c
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/cgraphunit.c,v
> > retrieving revision 1.1.4.35.2.44
> > diff -c -3 -p -r1.1.4.35.2.44 cgraphunit.c
> > *** cgraphunit.c        4 Mar 2005 21:25:59 -0000       1.1.4.35.2.44
> > --- cgraphunit.c        20 Mar 2005 10:01:52 -0000
> > *************** cgraph_function_versioning (struct cgrap
> > *** 1473,1478 ****
> > --- 1473,1493 ----
> >     /* Update the call_expr on the edges
> >        to the new version node. */
> >     update_call_expr (new_version_node, redirect_callers);
> > +   if (new_version_node && DECL_EXTERNAL (new_version_node->decl))
> > +     DECL_EXTERNAL (new_version_node->decl) = 0;
> > +   if (DECL_ONE_ONLY (new_version_node->decl))
> > +     DECL_ONE_ONLY (new_version_node->decl) = 0;
> > +   if (TREE_PUBLIC (new_version_node->decl))
> > +     TREE_PUBLIC (new_version_node->decl) = 0;
> > +   if (DECL_COMDAT (new_version_node->decl))
> > +     DECL_COMDAT (new_version_node->decl) = 0;

All the "if" statements are unnecessary.  Just do this instead:

+   DECL_EXTERNAL (new_version_node->decl) = 0;
+   DECL_ONE_ONLY (new_version_node->decl) = 0;
+   TREE_PUBLIC (new_version_node->decl) = 0;
+   DECL_COMDAT (new_version_node->decl) = 0;

> > +   new_version_node->local.externally_visible = 0;
> > +   new_version_node->local.local = 1;
> > +   new_version_node->local.avail = AVAIL_LOCAL;
> > +   if (TREE_ADDRESSABLE (new_version_node->decl))
> > +     TREE_ADDRESSABLE (new_version_node->decl) = 0;
> > +   if (new_version_node->needed)
> > +     abort ();
>
> gcc_assert here. WHat is the reason for this check anyway? 
> otherwise this change is fine as well as the ipa-prop one.

Right.  "gcc_assert (!new_version_node->needed);".  But it is a strange
check indeed.

> >     return new_version_node;
> >   }
> >
> > Index: gimplify.c
> > ===================================================================
> > RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
> > retrieving revision 1.1.2.141.2.21
> > diff -c -3 -p -r1.1.2.141.2.21 gimplify.c
> > *** gimplify.c  1 Mar 2005 23:08:02 -0000       1.1.2.141.2.21
> > --- gimplify.c  20 Mar 2005 10:01:53 -0000
> > *************** create_function_name (const char *prefix
> > *** 337,343 ****
> >     len = strlen (tmp_name);
> >     for (i=0; i < len; i++)
> >       {
> > !       if (tmp_name[i] == '.')
> >           tmp_name[i] = '_';
> >       }
> >
> > --- 337,343 ----
> >     len = strlen (tmp_name);
> >     for (i=0; i < len; i++)
> >       {
> > !       if (!ISALPHA (tmp_name[i]) && !ISDIGIT (tmp_name[i]) &&
> > tmp_name[i] != '.')
> >           tmp_name[i] = '_';
>
> What exactly this is shooting for?  Won't we run into unwanted
> collisions or something like that?

No, because a unique number is appended.  It's ugly though, but
we talked about this last week and couldn't think of anything
better.

Gr.
Steven