Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(170)

Issue 5752064: User directed Function Multiversioning via Function Overloading

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 9 months ago by Sriraman
Modified:
12 years, 1 month ago
Reviewers:
hjl.tools, jason, richard.guenther, iant2, davidxl, Diego Novillo, hubicka
CC:
gcc-patches_gcc.gnu.org
Base URL:
svn+ssh://gcc.gnu.org/svn/gcc/trunk/gcc/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : User directed Function Multiversioning via Function Overloading #

Patch Set 3 : User directed Function Multiversioning via Function Overloading #

Patch Set 4 : User directed Function Multiversioning via Function Overloading #

Patch Set 5 : User directed Function Multiversioning via Function Overloading #

Patch Set 6 : User directed Function Multiversioning via Function Overloading #

Patch Set 7 : User directed Function Multiversioning via Function Overloading #

Patch Set 8 : User directed Function Multiversioning via Function Overloading #

Patch Set 9 : User directed Function Multiversioning via Function Overloading #

Total comments: 5

Patch Set 10 : User directed Function Multiversioning via Function Overloading #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1673 lines, -22 lines) Patch
M gcc/c-family/c-common.c View 1 2 3 4 5 6 7 8 9 1 chunk +16 lines, -3 lines 0 comments Download
M gcc/cgraph.h View 1 2 3 4 5 6 7 8 9 3 chunks +22 lines, -0 lines 0 comments Download
M gcc/cgraph.c View 1 2 3 4 5 6 7 8 9 1 chunk +8 lines, -0 lines 0 comments Download
M gcc/cgraphbuild.c View 1 2 3 4 5 6 7 8 9 3 chunks +16 lines, -3 lines 0 comments Download
M gcc/config/i386/i386.c View 4 chunks +993 lines, -0 lines 0 comments Download
M gcc/cp/call.c View 1 2 3 4 5 6 7 8 9 5 chunks +74 lines, -0 lines 0 comments Download
M gcc/cp/class.c View 1 2 3 4 5 6 7 8 9 5 chunks +66 lines, -7 lines 0 comments Download
M gcc/cp/decl.c View 1 2 3 4 5 6 7 8 9 5 chunks +27 lines, -2 lines 0 comments Download
M gcc/cp/decl2.c View 1 2 3 4 5 6 7 8 9 1 chunk +4 lines, -0 lines 0 comments Download
M gcc/cp/error.c View 1 2 3 4 5 6 7 8 9 1 chunk +8 lines, -1 line 0 comments Download
M gcc/cp/mangle.c View 1 2 3 4 5 6 7 8 9 2 chunks +14 lines, -2 lines 0 comments Download
M gcc/cp/semantics.c View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -2 lines 0 comments Download
M gcc/doc/tm.texi View 1 2 3 4 5 6 7 8 9 2 chunks +31 lines, -0 lines 0 comments Download
M gcc/doc/tm.texi.in View 1 2 3 4 5 6 7 8 9 2 chunks +31 lines, -0 lines 0 comments Download
M gcc/target.def View 1 2 3 4 5 6 7 8 9 2 chunks +41 lines, -0 lines 0 comments Download
A gcc/testsuite/g++.dg/mv1.C View 1 2 3 4 5 6 7 8 9 1 chunk +130 lines, -0 lines 0 comments Download
A gcc/testsuite/g++.dg/mv2.C View 1 2 3 4 5 6 7 8 9 1 chunk +119 lines, -0 lines 0 comments Download
A gcc/testsuite/g++.dg/mv3.C View 1 2 3 4 5 6 7 8 9 1 chunk +37 lines, -0 lines 0 comments Download
A gcc/testsuite/g++.dg/mv4.C View 1 2 3 4 5 6 7 8 9 1 chunk +23 lines, -0 lines 0 comments Download
M gcc/tree.h View 1 2 3 4 5 6 7 8 9 2 chunks +8 lines, -2 lines 0 comments Download

Messages

Total messages: 86
Sriraman
User directed Function Multiversioning (MV) via Function Overloading ==================================================================== This patch adds support for user ...
12 years, 9 months ago (2012-03-07 00:46:33 UTC) #1
richard.guenther_gmail.com
On Wed, Mar 7, 2012 at 1:46 AM, Sriraman Tallam <tmsriram@google.com> wrote: > User directed ...
12 years, 9 months ago (2012-03-07 14:05:16 UTC) #2
Sriraman
On Wed, Mar 7, 2012 at 6:05 AM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Wed, ...
12 years, 9 months ago (2012-03-07 19:08:04 UTC) #3
davidxl
> You don't give an overview of the frontend implementation. Thus I have > extracted ...
12 years, 9 months ago (2012-03-08 21:00:25 UTC) #4
davidxl
On Wed, Mar 7, 2012 at 11:08 AM, Sriraman Tallam <tmsriram@google.com> wrote: > On Wed, ...
12 years, 9 months ago (2012-03-08 21:36:55 UTC) #5
Sriraman
Hi Richard, Here is a more detailed overview of the front-end description: * Tracking decls ...
12 years, 9 months ago (2012-03-09 20:04:11 UTC) #6
Sriraman
Hi, I have made the following changes in this new patch which is attached: * ...
12 years, 8 months ago (2012-04-27 05:08:48 UTC) #7
hjl.tools_gmail.com
On Thu, Apr 26, 2012 at 10:08 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi, > ...
12 years, 8 months ago (2012-04-27 13:38:54 UTC) #8
Sriraman
On Fri, Apr 27, 2012 at 6:38 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, ...
12 years, 8 months ago (2012-04-27 14:35:37 UTC) #9
hjl.tools_gmail.com
On Fri, Apr 27, 2012 at 7:35 AM, Sriraman Tallam <tmsriram@google.com> wrote: > On Fri, ...
12 years, 8 months ago (2012-04-27 14:38:48 UTC) #10
Sriraman
On Fri, Apr 27, 2012 at 7:38 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, ...
12 years, 8 months ago (2012-04-27 14:53:33 UTC) #11
hjl.tools_gmail.com
On Fri, Apr 27, 2012 at 7:53 AM, Sriraman Tallam <tmsriram@google.com> wrote: > On Fri, ...
12 years, 8 months ago (2012-04-27 15:36:47 UTC) #12
Sriraman
On Fri, Apr 27, 2012 at 8:36 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, ...
12 years, 8 months ago (2012-04-27 15:45:11 UTC) #13
Sriraman
Hi, New patch attached, updated test case and fixed bugs related to __PRETTY_FUNCTION_. Patch also ...
12 years, 7 months ago (2012-05-01 23:51:01 UTC) #14
hjl.tools_gmail.com
On Tue, May 1, 2012 at 4:51 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi, > ...
12 years, 7 months ago (2012-05-02 00:08:50 UTC) #15
Sriraman
Hi H.J, Done now. Patch attached. Thanks, -Sri. On Tue, May 1, 2012 at 5:08 ...
12 years, 7 months ago (2012-05-02 02:45:10 UTC) #16
hjl.tools_gmail.com
On Tue, May 1, 2012 at 7:45 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi H.J, ...
12 years, 7 months ago (2012-05-02 13:42:37 UTC) #17
Sriraman
On Wed, May 2, 2012 at 6:42 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, ...
12 years, 7 months ago (2012-05-02 15:08:28 UTC) #18
hjl.tools_gmail.com
On Wed, May 2, 2012 at 8:08 AM, Sriraman Tallam <tmsriram@google.com> wrote: > On Wed, ...
12 years, 7 months ago (2012-05-02 16:05:51 UTC) #19
Sriraman
On Wed, May 2, 2012 at 9:05 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, ...
12 years, 7 months ago (2012-05-02 17:44:04 UTC) #20
hjl.tools_gmail.com
On Wed, May 2, 2012 at 10:44 AM, Sriraman Tallam <tmsriram@google.com> wrote: >>>> >>>> 1. ...
12 years, 7 months ago (2012-05-02 18:04:05 UTC) #21
Sriraman
On Wed, May 2, 2012 at 11:04 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, ...
12 years, 7 months ago (2012-05-07 16:58:24 UTC) #22
Sriraman
Hi, Attached new patch with more bug fixes. I will fix the dispatching method to ...
12 years, 7 months ago (2012-05-09 19:01:07 UTC) #23
hjl.tools_gmail.com
On Wed, May 9, 2012 at 12:01 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi, > ...
12 years, 7 months ago (2012-05-10 17:55:25 UTC) #24
Sriraman
Hi H.J., I have updated the patch to improve the dispatching method like we discussed. ...
12 years, 7 months ago (2012-05-12 02:04:17 UTC) #25
hjl.tools_gmail.com
On Fri, May 11, 2012 at 7:04 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi H.J., ...
12 years, 7 months ago (2012-05-12 13:37:58 UTC) #26
Sriraman
Hi H.J, Attaching new patch with 2 test cases, mv2.C checks ISAs only and mv1.C ...
12 years, 7 months ago (2012-05-14 18:29:00 UTC) #27
hjl.tools_gmail.com
On Mon, May 14, 2012 at 11:28 AM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi H.J, ...
12 years, 7 months ago (2012-05-26 00:07:25 UTC) #28
Sriraman
Hi H.J., On Fri, May 25, 2012 at 5:07 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > ...
12 years, 7 months ago (2012-05-26 00:16:24 UTC) #29
hjl.tools_gmail.com
On Fri, May 25, 2012 at 5:16 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi H.J., ...
12 years, 7 months ago (2012-05-26 00:27:34 UTC) #30
Sriraman
On Fri, May 25, 2012 at 5:27 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, ...
12 years, 7 months ago (2012-05-26 01:54:08 UTC) #31
hjl.tools_gmail.com
On May 25, 2012 6:54 PM, "Sriraman Tallam" <tmsriram@google.com> wrote: > > > >> > ...
12 years, 7 months ago (2012-05-26 02:15:10 UTC) #32
Sriraman
On May 25, 2012 7:15 PM, "H.J. Lu" <hjl.tools@gmail.com> wrote: > > > On May ...
12 years, 7 months ago (2012-05-26 03:38:21 UTC) #33
hjl.tools_gmail.com
On Fri, May 25, 2012 at 8:38 PM, Sriraman Tallam <tmsriram@google.com> wrote: > > On ...
12 years, 7 months ago (2012-05-26 05:05:46 UTC) #34
Sriraman
On Fri, May 25, 2012 at 10:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Fri, ...
12 years, 7 months ago (2012-05-26 22:34:52 UTC) #35
hjl.tools_gmail.com
On Sat, May 26, 2012 at 3:34 PM, Sriraman Tallam <tmsriram@google.com> wrote: > On Fri, ...
12 years, 7 months ago (2012-05-26 23:56:08 UTC) #36
Sriraman
On Sat, May 26, 2012 at 4:56 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sat, ...
12 years, 7 months ago (2012-05-27 00:23:52 UTC) #37
hjl.tools_gmail.com
On Sat, May 26, 2012 at 5:23 PM, Sriraman Tallam <tmsriram@google.com> wrote: > On Sat, ...
12 years, 7 months ago (2012-05-27 02:06:04 UTC) #38
Sriraman
On Sat, May 26, 2012 at 7:06 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Sat, ...
12 years, 7 months ago (2012-05-27 02:23:17 UTC) #39
hjl.tools_gmail.com
On Sat, May 26, 2012 at 7:23 PM, Sriraman Tallam <tmsriram@google.com> wrote: >> >> ï¼´hat ...
12 years, 7 months ago (2012-05-27 02:31:23 UTC) #40
iant2
Sriraman Tallam <tmsriram@google.com> writes: > Any reason why gcc should not be made to prefer ...
12 years, 7 months ago (2012-05-27 19:02:35 UTC) #41
Sriraman
Hi, Attaching updated patch for function multiversioning which brings in plenty of changes. * As ...
12 years, 6 months ago (2012-06-04 18:59:44 UTC) #42
hjl.tools_gmail.com
On Mon, Jun 4, 2012 at 11:59 AM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi, > ...
12 years, 6 months ago (2012-06-04 21:36:27 UTC) #43
Sriraman
Bug fixed and new patch attached. Patch also available for review at http://codereview.appspot.com/5752064 Thanks, -Sri. ...
12 years, 6 months ago (2012-06-04 22:29:05 UTC) #44
hjl.tools_gmail.com
On Mon, Jun 4, 2012 at 3:29 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Bug fixed ...
12 years, 6 months ago (2012-06-05 13:56:44 UTC) #45
Sriraman
On Jun 5, 2012 6:56 AM, "H.J. Lu" <hjl.tools@gmail.com> wrote: > > On Mon, Jun ...
12 years, 6 months ago (2012-06-05 14:14:10 UTC) #46
Sriraman
+cc c++ front-end maintainers Hi, C++ Frontend maintainers, Could you please take a look at ...
12 years, 6 months ago (2012-06-14 20:13:20 UTC) #47
Sriraman
Ping. On Thu, Jun 14, 2012 at 1:13 PM, Sriraman Tallam <tmsriram@google.com> wrote: > +cc ...
12 years, 6 months ago (2012-06-20 01:03:29 UTC) #48
Sriraman
Ping. On Tue, Jun 19, 2012 at 6:03 PM, Sriraman Tallam <tmsriram@google.com>wrote: > Ping. > ...
12 years, 5 months ago (2012-06-29 19:31:08 UTC) #49
richard.guenther_gmail.com
On Thu, Jun 14, 2012 at 10:13 PM, Sriraman Tallam <tmsriram@google.com> wrote: > +cc c++ ...
12 years, 5 months ago (2012-07-06 09:14:05 UTC) #50
Sriraman
On Fri, Jul 6, 2012 at 2:14 AM, Richard Guenther <richard.guenther@gmail.com> wrote: > > On ...
12 years, 5 months ago (2012-07-06 17:37:42 UTC) #51
jason_redhat.com
On 06/14/2012 04:13 PM, Sriraman Tallam wrote: > C++ Frontend maintainers, Could you please take ...
12 years, 5 months ago (2012-07-07 06:06:03 UTC) #52
davidxl
On Fri, Jul 6, 2012 at 11:05 PM, Jason Merrill <jason@redhat.com> wrote: > On 06/14/2012 ...
12 years, 5 months ago (2012-07-07 18:38:09 UTC) #53
jason_redhat.com
On 07/07/2012 08:38 PM, Xinliang David Li wrote: >> It seems to me that what ...
12 years, 5 months ago (2012-07-08 11:20:50 UTC) #54
davidxl
http://codereview.appspot.com/5752064/diff/51001/gcc/cgraph.c File gcc/cgraph.c (right): http://codereview.appspot.com/5752064/diff/51001/gcc/cgraph.c#newcode1282 gcc/cgraph.c:1282: is needed as the address can be used to ...
12 years, 5 months ago (2012-07-09 21:22:21 UTC) #55
davidxl
Ok. Do you have specific comments on the patch? thanks, David On Sun, Jul 8, ...
12 years, 5 months ago (2012-07-09 21:27:29 UTC) #56
jason_redhat.com
On 07/09/2012 11:27 PM, Xinliang David Li wrote: > Ok. Do you have specific comments ...
12 years, 5 months ago (2012-07-10 09:46:34 UTC) #57
davidxl
On Tue, Jul 10, 2012 at 2:46 AM, Jason Merrill <jason@redhat.com> wrote: > On 07/09/2012 ...
12 years, 5 months ago (2012-07-10 16:08:49 UTC) #58
Sriraman
Hi Jason/David, Thanks for the comments. On Tue, Jul 10, 2012 at 9:08 AM, Xinliang ...
12 years, 5 months ago (2012-07-10 19:11:35 UTC) #59
Sriraman
On Tue, Jul 10, 2012 at 9:08 AM, Xinliang David Li <davidxl@google.com>wrote: > On Tue, ...
12 years, 5 months ago (2012-07-10 19:14:10 UTC) #60
jason_redhat.com
On 07/10/2012 03:14 PM, Sriraman Tallam wrote: > I am using the questions you asked ...
12 years, 5 months ago (2012-07-19 20:39:46 UTC) #61
Sriraman
On Thu, Jul 19, 2012 at 1:39 PM, Jason Merrill <jason@redhat.com> wrote: > > On ...
12 years, 4 months ago (2012-07-30 19:01:14 UTC) #62
Sriraman
Hi Jason, I have created a new patch to use target hooks for all the ...
12 years, 4 months ago (2012-08-25 00:34:19 UTC) #63
Sriraman
Ping. On Aug 25, 2012 6:04 AM, "Sriraman Tallam" <tmsriram@google.com> wrote: > Hi Jason, > ...
12 years, 3 months ago (2012-09-01 03:15:04 UTC) #64
Sriraman
Ping. On Fri, Aug 24, 2012 at 5:34 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Hi ...
12 years, 3 months ago (2012-09-18 16:29:12 UTC) #65
davidxl
Hi Jason, Sri has addressed the comments you had on FE part. Can you take ...
12 years, 2 months ago (2012-10-05 17:07:38 UTC) #66
jason_redhat.com
On 08/24/2012 08:34 PM, Sriraman Tallam wrote: > + /* If the address of a ...
12 years, 2 months ago (2012-10-05 17:44:01 UTC) #67
jason_redhat.com
On 10/05/2012 01:43 PM, Jason Merrill wrote: > On 08/24/2012 08:34 PM, Sriraman Tallam wrote: ...
12 years, 2 months ago (2012-10-05 18:14:36 UTC) #68
jason_redhat.com
On 08/24/2012 08:34 PM, Sriraman Tallam wrote: > + /* For function versions, their parms ...
12 years, 2 months ago (2012-10-05 18:32:19 UTC) #69
Sriraman
On Fri, Oct 5, 2012 at 10:43 AM, Jason Merrill <jason@redhat.com> wrote: > On 08/24/2012 ...
12 years, 2 months ago (2012-10-05 21:57:57 UTC) #70
jason_redhat.com
On 10/05/2012 05:57 PM, Sriraman Tallam wrote: > In general, the dispatcher is always necessary ...
12 years, 2 months ago (2012-10-05 22:50:07 UTC) #71
Sriraman
On Fri, Oct 5, 2012 at 3:50 PM, Jason Merrill <jason@redhat.com> wrote: > On 10/05/2012 ...
12 years, 2 months ago (2012-10-05 23:45:41 UTC) #72
Sriraman
Hi Jason, I have addressed all your comments and attached the new patch. On Fri, ...
12 years, 2 months ago (2012-10-10 23:45:45 UTC) #73
Sriraman
Hi Jason, I have attached the latest patch with more cleanups. Please let me know ...
12 years, 2 months ago (2012-10-12 22:19:04 UTC) #74
Diego Novillo
On 2012-10-12 18:19 , Sriraman Tallam wrote: > When the front-end sees more than one ...
12 years, 2 months ago (2012-10-19 15:10:30 UTC) #75
Sriraman
Hi Diego, Thanks for the review. I have addressed all your comments. New patch attached. ...
12 years, 2 months ago (2012-10-20 02:33:11 UTC) #76
Sriraman
Hi, I have attached the latest patch with bug fixes, comments. I have also added ...
12 years, 2 months ago (2012-10-23 21:09:21 UTC) #77
Diego Novillo
On Fri, Oct 19, 2012 at 10:33 PM, Sriraman Tallam <tmsriram@google.com> wrote: > Yes, the ...
12 years, 1 month ago (2012-10-26 13:36:32 UTC) #78
hubicka_ucw.cz
Hi, sorry for jumping in late, for too long I did not had chnce to ...
12 years, 1 month ago (2012-10-26 15:54:49 UTC) #79
davidxl
On Fri, Oct 26, 2012 at 8:54 AM, Jan Hubicka <hubicka@ucw.cz> wrote: > Hi, > ...
12 years, 1 month ago (2012-10-26 16:07:05 UTC) #80
Sriraman
On Fri, Oct 26, 2012 at 9:07 AM, Xinliang David Li <davidxl@google.com> wrote: > On ...
12 years, 1 month ago (2012-10-26 16:54:20 UTC) #81
Sriraman
Hi Diego and Honza, I have made all the changes mentioned and attached the new ...
12 years, 1 month ago (2012-10-28 01:16:05 UTC) #82
hubicka_ucw.cz
> Index: gcc/cgraph.c > =================================================================== > --- gcc/cgraph.c (revision 192623) > +++ gcc/cgraph.c (working copy) ...
12 years, 1 month ago (2012-10-29 12:55:16 UTC) #83
Sriraman
On Mon, Oct 29, 2012 at 5:55 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >> Index: gcc/cgraph.c ...
12 years, 1 month ago (2012-10-29 17:38:38 UTC) #84
jason_redhat.com
On 10/27/2012 09:16 PM, Sriraman Tallam wrote: > + /* See if there's a match. ...
12 years, 1 month ago (2012-10-30 19:10:33 UTC) #85
Sriraman
12 years, 1 month ago (2012-10-31 00:36:30 UTC) #86
On Tue, Oct 30, 2012 at 12:10 PM, Jason Merrill <jason@redhat.com> wrote:
> On 10/27/2012 09:16 PM, Sriraman Tallam wrote:
>>
>> +         /* See if there's a match.   For functions that are
>> multi-versioned,
>> +            all the versions match.  */
>>           if (same_type_p (target_fn_type, static_fn_type (fn)))
>> -           matches = tree_cons (fn, NULL_TREE, matches);
>> +           {
>> +             matches = tree_cons (fn, NULL_TREE, matches);
>> +             /*If versioned, push all possible versions into a vector.
>> */
>> +             if (DECL_FUNCTION_VERSIONED (fn))
>> +               {
>> +                 if (fn_ver_vec == NULL)
>> +                  fn_ver_vec = VEC_alloc (tree, heap, 2);
>> +                 VEC_safe_push (tree, heap, fn_ver_vec, fn);
>> +               }
>> +           }
>
>
> Why do we need to keep both a list and vector of the matches?

Right, but we later call the target hook
get_function_versions_dispatcher which takes a vector. I could change
that to accept a list instead if that is preferable?

>
>> +        Call decls_match to make sure they are different because they are
>> +        versioned.  */
>> +      if (DECL_FUNCTION_VERSIONED (fn))
>> +       {
>> +          for (match = TREE_CHAIN (matches); match; match = TREE_CHAIN
>> (match))
>> +           if (decls_match (fn, TREE_PURPOSE (match)))
>> +             break;
>> +       }
>
>
> What if you have multiple matches that aren't all versions of the same
> function?

Right, I should really check if there are versions by comparing params
too. I fixed this in joust but missed out here. I will make the change
so that any matches with functions that do not belong to the
semantically identical group of function versions will be caught and
the ambiguity will be flagged.

>
> Why would it be a problem to have two separate declarations of the same
> function?

AFAIU, this should not be a problem. For duplicate declarations,
duplicate_decls should merge them and they should never be seen here.
Did I miss something?

>
>> +      dispatcher_decl = targetm.get_function_versions_dispatcher
>> (fn_ver_vec);
>
>
> Is the idea here that if you have some versions declared, then a call, then
> more versions declared, then another call, you will call two different
> dispatchers,

No, I thought about this but I did not want to handle this case in
this iteration. The dispatcher is created only once and if more
functions are declared later, they will not be dispatched atleast in
this iteration.

> where the first one will only dispatch to the versions declared
> before the first call?  If not, why do we care about the set of declarations
> at this point?

I am taking the address of a multi-versioned function here. The
front-end is returning the address of the dispatcher decl instead.
Since, I am building the dispatcher here, why not construct the cgraph
datastructures for these versions too?  That is why I aggregate all
the declarations here.

>
>> +      /* Mark this functio to be output.  */
>> +      node->local.finalized = 1;
>
>
> Missing 'n' in "function".
>
>> @@ -14227,7 +14260,11 @@ cxx_comdat_group (tree decl)
>>           else
>>             break;
>>         }
>> -      name = DECL_ASSEMBLER_NAME (decl);
>> +      if (TREE_CODE (decl) == FUNCTION_DECL
>> +            && DECL_FUNCTION_VERSIONED (decl))
>> +          name = DECL_NAME (decl);
>
>
> This would mean that f in the global namespace and f in namespace foo would
> end up in the same comdat group.  Why do we need special handling here at
> all?


Right, we do not need special handling. It is ok for each function
version to be in its own comdat group, I will remove this.

>
>>  dump_function_name (tree t, int flags)
>>  {
>> -  tree name = DECL_NAME (t);
>> +  tree name;
>>
>> +  /* For function versions, use the assembler name as the decl name is
>> +     the same for all versions.  */
>> +  if (TREE_CODE (t) == FUNCTION_DECL
>> +      && DECL_FUNCTION_VERSIONED (t))
>> +    name = DECL_ASSEMBLER_NAME (t);
>
>
> This shouldn't be necessary; we should print the target attribute when
> printing the function declaration.

Ok.

>
>> +        Also, mark this function as needed if it is marked inline but
>> +        is a multi-versioned function.  */
>> +      if (((flag_keep_inline_functions
>> +           || DECL_FUNCTION_VERSIONED (fn))
>
>
> This should be marked as needed by the code that builds the dispatcher.

I had some trouble previously figuring out where to mark this as
needed. I will fix it.

>
>> +  /* For calls to a multi-versioned function, overload resolution
>> +     returns the function with the highest target priority, that is,
>> +     the version that will checked for dispatching first.  If this
>> +     version is inlinable, a direct call to this version can be made
>> +     otherwise the call should go through the dispatcher.  */
>
>
> I'm a bit confused why people would want both dispatched calls and
> non-dispatched inlining; I would expect that if a function can be compiled
> differently enough on newer hardware to make versioning worthwhile, that
> would be a larger difference than the call overhead.

Simple example:

int
foo ()
{
  return 1;
}
int __attribute__ ((target ("popcnt")))
foo ()
{
  return 0;
}

int __attribute__ ((target ("popcnt")))
bar ()
{
  return foo ();
}

Here, the call to foo () from bar () will be turned into a direct call
to the popcnt version.

Here, if bar is executed, then popcnt is supported and the call to foo
from bar will be dispatched to the popcnt version even if it goes
through the dispatcher and this is known at compile time. So, why not
make a direct call?  I am only making direct calls to versions when I
am sure the dispatcher would do the same.

>
>> +  if (DECL_FUNCTION_VERSIONED (fn)
>> +      && !targetm.target_option.can_inline_p (current_function_decl, fn))
>> +    {
>> +      struct cgraph_node *dispatcher_node = NULL;
>> +      fn = get_function_version_dispatcher (fn);
>> +      if (fn == NULL)
>> +       return NULL;
>> +      dispatcher_node = cgraph_get_create_node (fn);
>> +      gcc_assert (dispatcher_node != NULL);
>> +      /* Mark this function to be output.  */
>> +      dispatcher_node->local.finalized = 1;
>> +    }
>
>
> Why do you need to mark this here?  If you generate a call to the
> dispatcher, cgraph should mark it to be output automatically.

dispatcher_node does not have a body  until it is generated in
cgraphunit.c, so cgraph does not mark this field before this is
processed in cgraph_analyze_function.

>
>> +  /* For candidates of a multi-versioned function,  make the version with
>> +     the highest priority win.  This version will be checked for
>> dispatching
>> +     first.  If this version can be inlined into the caller, the
>> front-end
>> +     will simply make a direct call to this function.  */
>
>
> This is still too high in joust.  I believe I said before that this code
> should come just above
>
>    /* If the two function declarations represent the same function (this can
>       happen with declarations in multiple scopes and arg-dependent lookup),
>       arbitrarily choose one.  But first make sure the default args we're
>       using match.  */

Yes, I missed this the last time around. Will fix it this time.

>
>> +  /* For multiversioned functions, aggregate all the versions here for
>> +     generating the dispatcher body later if necessary.  Check to see if
>> +     the dispatcher is already generated to avoid doing this more than
>> +     once.  */
>
>
> This caching seems to assume that you'll always be considering the same
> group of declarations, which goes back to my earlier question.


Yes, for now I want to be only considering the same group of
declarations.  I am assuming that all declarations/definitions of all
versions of foo are seen before the first call to foo. I do not want
multiple dispatcher support complexity in this iteration. Is it ok to
delay this to the next patch iteration?

Your earlier question on this was:

 "This seems to assume that all the functions in the list of
candidates are versioned, but there might be unrelated functions from
different namespaces.  Also, doing this every time someone calls a
versioned function seems like the wrong place; I would think it would
be better to build up a list of versions as you seed declarations, and
then use that list to define the dispatcher at EOF if it's needed."

I have fixed the problem of unrelated functions by always checking the
type (same_type_p) and params (comp_params) in
get_function_version_dispatcher. You talked about doing the dispatcher
building later, but I did it here since I am doing it only once.


Thanks,
-Sri.


>
> Jason
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b