This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [tree-profiling-branch PATCH IPCP extensions + Function cloning


> > + /* Tree versioning hooks.  */
> > + #define LANG_HOOKS_TREE_VERSIONING_CANNOT_VERSION_TREE_FN \
> > +   lhd_tree_versioning_cannot_version_tree_fn
> > 
> > I tend to recall that we concluded that this hook is not needed?  Do you
> > have testcase?
> 
> True. The hook used to handle destructor/constructor cases, indeed 
> unnecessary now.
> However, we want to avoid versioning in cases of templates, so we need to 
> have a c++ 
> hook, in order to check those cases.

What makes templates so special so you want to avoid the versioning
here?
> 
> > !       if (!id->versioning_p)
> > !              new = make_edge (BASIC_BLOCK (last_basic_block-1), 
> EXIT_BLOCK_PTR,
> >                                       EDGE_FALLTHRU);
> > +       else
> > +              {
> > +                basic_block bb1,bb2;
> > +                edge e_step;
> > + 
> > +                bb2 = EXIT_BLOCK_PTR_FOR_FUNCTION(cfun_to_copy);
> > +                if (bb2->preds)
> > +                  { 
> > +                    FOR_EACH_EDGE(e_step,ei,bb2->preds)
> > +                              {
> > +                                bb1 = e_step->src;
> > +                                new = make_edge (BASIC_BLOCK 
> (bb1->index), EXIT_BLOCK_PTR,
> > + e_step->flags);
> > +                              }
> > +                  }
> > +              }
> > 
> > How exactly this relate to clonning code?  I think the function clonning
> > already knows how to duplicate the gimple function with the CFG, so you
> > should not need any more adjustments here...
> 
> bootstrap failed until this change was done...

Hmm, this sounds like latent inliner (or at least copy_body) bug.  Would
you mind to explain what went wrong without this hunk?
> 
> > + /* Return true if the function can have a new version.
> > +    This is a guard for the versioning functionality.  */
> > + static bool
> > + function_versionable_p (tree fndecl)
> > + {
> > +   if (fndecl == NULL_TREE)
> > +     return false;
> > +   /* ??? There are cases where a function is
> > +      uninlinable but can have a new versions.  */
> > +   if (lang_hooks.tree_versioning.cannot_version_tree_fn (&fndecl))
> > +      return false;
> > + 
> > +   if (!tree_inlinable_function_p (fndecl)) 
> > +     return false;
> > 
> > Why versionability is subset of inlinability?  I would guess that
> > varargs or setjmp are no stoppers for versioning.  It is not major issue
> > tought.
> 
> I agree that there's no direct relation between inlinable and versionable, 
> but 
> we wanted to use analysis already done for inlining and prevent 
> duplication of code.
> In the long term, I see inlining as an extension of versioning.

We ought to factor the code properly and break out the tests from
inlinability that also limits cloning to common function.  But I don't
seem to be able to come up with any actually.
> 
> 
> > + /* Copy NODE (which must be a DECL).  The DECL originally was in the 
> FROM_FN,
> > +    but now it will be in the TO_FN. As apposed for to 
> copy_decl_for_inlining ()
> > +    function; we do not give a special treatment to parm_decl and 
> result_decl.  */ 
> > + static tree
> > + copy_decl_for_versioning (tree decl, tree from_fn, tree to_fn)
> > 
> > Also it would be really cool to share at least partly this with the
> > copy_decl_for_inlining 
> 
> Sharing the code means that we would have to add another flag, saying 
> whether we are 
> versioning or inlining. This will require to add another argument to all 
> calls 
> to copy_decl_for_inlining(), and we wanted to minimize changes in existing 
> code.

There are overall 4 callees of this all in already heavilly modified
tree-inline.c so I would preffer this sollution (together with renaming
to some more fitting name) over code duplication.

Thanks,
Honza
> 
> 
> Razya
> 
> 
> 
> 
> 
> 
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]