From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32583 invoked by alias); 6 Jul 2012 17:38:01 -0000 Received: (qmail 32260 invoked by uid 22791); 6 Jul 2012 17:37:59 -0000 X-SWARE-Spam-Status: No, hits=-5.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-yx0-f175.google.com (HELO mail-yx0-f175.google.com) (209.85.213.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 06 Jul 2012 17:37:42 +0000 Received: by yenl13 with SMTP id l13so9091256yen.20 for ; Fri, 06 Jul 2012 10:37:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-system-of-record:x-gm-message-state; bh=yNowIEytk3hY6RWn1HHWt/96wKkvtnJw9h9PVuahWuI=; b=L1YA70eKcQ/THfCVJn2UXvX/kobnOhA7A6i7HOzZs+Omh+BQFJb7k6Moa/Ple07GBQ R+04v8UKKqqR3S0vvfa9SNx0IX2dcKkBZnAJO3MJ/At7kdjxjXL8z7OyzGg1TpmcWKMp nfcDxkWEvRTHiYPkNAWuGXprUhSHDYbvGhqJ7bbtS4G86buaJcybYy/ovC+yIIib2YT4 K4as9NNt5GzfM8ryyCBUk+IlCUR6bFAo+myJlvS+qWQMAnaGlF866Zofm27Hp2/1CpRW gx0Kknh9YiCwp6tQJIvtSq/HnbarDcSO5dYW9hi2h4wNVScefzJHldTVUVVS97FEe1X9 1boA== Received: by 10.60.11.9 with SMTP id m9mr31751842oeb.5.1341596261776; Fri, 06 Jul 2012 10:37:41 -0700 (PDT) MIME-Version: 1.0 Received: by 10.60.11.9 with SMTP id m9mr31751818oeb.5.1341596261578; Fri, 06 Jul 2012 10:37:41 -0700 (PDT) Received: by 10.182.45.231 with HTTP; Fri, 6 Jul 2012 10:37:41 -0700 (PDT) In-Reply-To: References: <20120307004630.A503DB21B6@azwildcat.mtv.corp.google.com> Date: Fri, 06 Jul 2012 17:38:00 -0000 Message-ID: Subject: Re: User directed Function Multiversioning via Function Overloading (issue5752064) From: Sriraman Tallam To: Richard Guenther Cc: jason@redhat.com, mark@codesourcery.com, nathan@codesourcery.com, "H.J. Lu" , Jan Hubicka , Uros Bizjak , reply@codereview.appspotmail.com, gcc-patches@gcc.gnu.org, David Li Content-Type: text/plain; charset=ISO-8859-1 X-System-Of-Record: true X-Gm-Message-State: ALoCoQl0JWPGM3HAO7o/+ij74b2RBZzmhP9nBLp8ypekB3j3cixDkMym810qy63hAj7bTZWx+FirJZl5IUWjxQmSlooIK2MpvC4Umdox6tlRK619JfrGh7icPf/OT5cQmTWzgvTxhevfTNl3gyhZ66uGrp9rGuz+Ays8a2lpw4orFU9ym5/GS1iXrvtqr3SoX2jP/IdiJwMZ X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-07/txt/msg00256.txt.bz2 On Fri, Jul 6, 2012 at 2:14 AM, Richard Guenther wrote: > > On Thu, Jun 14, 2012 at 10:13 PM, Sriraman Tallam 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 > void bar (T t) __attribute__((target("sse"))); > template > 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 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 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 wrote: > >>>> On Fri, May 11, 2012 at 7:04 PM, Sriraman Tallam 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.