Re: User directed Function Multiversioning via Function Overloading (issue5752064)

[Sriraman Tallam <tmsriram@google.com>]

On Fri, Jul 6, 2012 at 2:14 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
>
> On Thu, Jun 14, 2012 at 10:13 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> > +cc c++ front-end maintainers
> >
> > Hi,
> >
> >    C++ Frontend maintainers, Could you please take a look at the
> > front-end part when you find the time?
>
> So you have (for now?) omitted the C frontend change(s)?

Yes, for now. I thought I will get the C++ changes and associated
middle-end checked in first. The C changes should be easy to add, I
have to introduce a new attribute for this. So, the C front-end should
look like this:

int foo (); // default version.
int foo_sse4() __attribute__ ((version("foo"), target("sse4.2"))); //
A version of foo.

and the call will be to foo.

The version attribute will be the new one, there may be an existing
attribute that I could use too for this purpose. I was thinking if the
"alias" attribute along with the "target" attribute could be used for
this purpose but it makes things unnecessarily complicated. What do
you think?

>
> >    Honza, your thoughts on the callgraph part?
> >
> >    Richard, any further comments/feedback?
>
> Overall I like it - the cgraph portions need comments from Honza and the
> C++ portions from a C++ maintainer though.
>
> I would appreciate a C version, too.

Sure, I will get to it immediately after the current patch reaches a
stable point.

>
> As you are tackling the C++ frontend first you should add some C++
> specific testcases - if only to verify you properly reject cases you do not
> or can not implement.  Like eventually

Sure, I will add these test cases.

Thanks for reviewing,
-Sri.

>
> class Foo {
>   virtual void bar() __attribute__((target("sse")));
>   virtual void bar() __attribute__((target("sse2")));
> };
>
> or
>
> template <class T>
> void bar (T t) __attribute__((target("sse")));
> template <class T>
> void bar (T t) __attribute__((target("sse2")));
> template <>
> void bar (int t);
>
> (how does regular C++ overload resolution / template specialization
> interfere with the target overloads?)
>
> Thanks,
> Richard.
>
> >    Additionally, I am working on generating better mangled names for
> > function versions, along the lines of C++ thunks.
> >
> > Thanks,
> > -Sri.
> >
> > On Mon, Jun 4, 2012 at 11:59 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> >> Hi,
> >>
> >>   Attaching updated patch for function multiversioning which brings
> >> in plenty of changes.
> >>
> >> * As suggested by Richard earlier, I have made cgraph aware of
> >> function versions. All nodes of function versions are chained and the
> >> dispatcher bodies are created on demand while building cgraph edges.
> >> The dispatcher body will be created if and only if there is a call or
> >> reference to a versioned function. Previously, I was maintaining the
> >> list of versions separately in a hash map, all that is gone now.
> >> * Now, the file multiverison.c has some helper routines that are used
> >> in the context of function versioning. There are no new passes and no
> >> new globals.
> >> * More tests, updated existing tests.
> >> * Fixed lots of bugs.
> >> * Updated patch description.
> >>
> >> Patch attached. Patch also available for review at
> >> http://codereview.appspot.com/5752064
> >>
> >> Please let me know what you think,
> >>
> >> Thanks,
> >> -Sri.
> >>
> >>
> >> On Mon, May 14, 2012 at 11:28 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> >>> Hi H.J,
> >>>
> >>>   Attaching new patch with 2 test cases, mv2.C checks ISAs only and
> >>> mv1.C checks ISAs and arches mixed. Right now, checking only arches is
> >>> not needed as they are mutually exclusive, any order should be fine.
> >>>
> >>> Patch also available for review here:  http://codereview.appspot.com/5752064
> >>>
> >>> Thanks,
> >>> -Sri.
> >>>
> >>> On Sat, May 12, 2012 at 6:37 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> >>>> On Fri, May 11, 2012 at 7:04 PM, Sriraman Tallam <tmsriram@google.com> wrote:
> >>>>> Hi H.J.,
> >>>>>
> >>>>>   I have updated the patch to improve the dispatching method like we
> >>>>> discussed. Each feature gets a priority now, and the dispatching is
> >>>>> done in priority order. Please see i386.c for the changes.
> >>>>>
> >>>>> Patch also available for review here:  http://codereview.appspot.com/5752064
> >>>>>
> >>>>
> >>>> I think you need 3 tests:
> >>>>
> >>>> 1.  Only with ISA.
> >>>> 2.  Only with arch
> >>>> 3.  Mixed with ISA and arch
> >>>>
> >>>> since test mixed ISA and arch may hide issues with ISA only or arch only.
> >>>>
> >>>> --
> >>>> H.J.