| Message ID | 70b0358c-5060-b5c0-9116-3b4e8e0b5146@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Redirect call within specific target attribute among MV clones (PR ipa/82625). | expand |
On 10/4/18 7:56 AM, Martin Liška wrote: > Hi. > > When having a pair of target clones where foo calls bar, if the target > attribute are equal we can redirect the call and not use ifunc dispatcher. > > Patch survives regression tests on x86_64-linux-gnu. > Ready for trunk? > > Martin > > gcc/ChangeLog: > > 2018-10-04 Martin Liska <mliska@suse.cz> > > PR ipa/82625 > * multiple_target.c (redirect_to_specific_clone): New function. > (ipa_target_clone): Use it. > * tree-inline.c: Fix comment. > > gcc/testsuite/ChangeLog: > > 2018-10-04 Martin Liska <mliska@suse.cz> > > PR ipa/82625 > * g++.dg/ext/pr82625.C: New test. Your timing is good. The issues with unnecessary calls to ifunc dispatchers when we have an ifunc that is not a leaf in the call graph came up in some meetings I was having last week. I doubt this is enough to address all the issues folks raised, but ISTM it should certainly help. OK for the trunk. Jeff
On 10/4/18 4:17 PM, Jeff Law wrote: > On 10/4/18 7:56 AM, Martin Liška wrote: >> Hi. >> >> When having a pair of target clones where foo calls bar, if the target >> attribute are equal we can redirect the call and not use ifunc dispatcher. >> >> Patch survives regression tests on x86_64-linux-gnu. >> Ready for trunk? >> >> Martin >> >> gcc/ChangeLog: >> >> 2018-10-04 Martin Liska <mliska@suse.cz> >> >> PR ipa/82625 >> * multiple_target.c (redirect_to_specific_clone): New function. >> (ipa_target_clone): Use it. >> * tree-inline.c: Fix comment. >> >> gcc/testsuite/ChangeLog: >> >> 2018-10-04 Martin Liska <mliska@suse.cz> >> >> PR ipa/82625 >> * g++.dg/ext/pr82625.C: New test. > Your timing is good. The issues with unnecessary calls to ifunc > dispatchers when we have an ifunc that is not a leaf in the call graph > came up in some meetings I was having last week. Funny. > > I doubt this is enough to address all the issues folks raised, but ISTM > it should certainly help. Sure, are you talking about: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83411 https://hannes.hauswedell.net/post/2017/12/09/fmv/ or do you have any other specific situations? > > OK for the trunk. Installed as r264845. Martin > > Jeff > >
Hi Martin, pr82625.C failed on compiler builds which don't support "default" and "avx" target. For example, arm/aarch64 native linux gcc compiler. As I found in this gcc wiki: https://gcc.gnu.org/wiki/FunctionMultiVersioning ''' This support is available in GCC 4.8 and later. Support is only available in C++ for i386 targets. ''' Should the test be guarded with a target selector or require function multi-versioning instead of ifunc? Regards, Renlin On 10/04/2018 02:56 PM, Martin Liška wrote: > Hi. > > When having a pair of target clones where foo calls bar, if the target > attribute are equal we can redirect the call and not use ifunc dispatcher. > > Patch survives regression tests on x86_64-linux-gnu. > Ready for trunk? > > Martin > > gcc/ChangeLog: > > 2018-10-04 Martin Liska <mliska@suse.cz> > > PR ipa/82625 > * multiple_target.c (redirect_to_specific_clone): New function. > (ipa_target_clone): Use it. > * tree-inline.c: Fix comment. > > gcc/testsuite/ChangeLog: > > 2018-10-04 Martin Liska <mliska@suse.cz> > > PR ipa/82625 > * g++.dg/ext/pr82625.C: New test. > --- > gcc/multiple_target.c | 51 ++++++++++++++++++++++++++++++ > gcc/testsuite/g++.dg/ext/pr82625.C | 36 +++++++++++++++++++++ > gcc/tree-inline.c | 2 +- > 3 files changed, 88 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/ext/pr82625.C > >
On 10/8/18 10:46 AM, Renlin Li wrote: > Hi Martin, > > pr82625.C failed on compiler builds which don't support "default" and "avx" target. > For example, arm/aarch64 native linux gcc compiler. > > > As I found in this gcc wiki: https://gcc.gnu.org/wiki/FunctionMultiVersioning > ''' > This support is available in GCC 4.8 and later. Support is only available in C++ for i386 targets. > ''' > > Should the test be guarded with a target selector or require function multi-versioning instead of ifunc? Hi. Sure, sorry for the breakage. I'm going to install following tested patch. Martin > > Regards, > Renlin > > > On 10/04/2018 02:56 PM, Martin Liška wrote: >> Hi. >> >> When having a pair of target clones where foo calls bar, if the target >> attribute are equal we can redirect the call and not use ifunc dispatcher. >> >> Patch survives regression tests on x86_64-linux-gnu. >> Ready for trunk? >> >> Martin >> >> gcc/ChangeLog: >> >> 2018-10-04 Martin Liska <mliska@suse.cz> >> >> PR ipa/82625 >> * multiple_target.c (redirect_to_specific_clone): New function. >> (ipa_target_clone): Use it. >> * tree-inline.c: Fix comment. >> >> gcc/testsuite/ChangeLog: >> >> 2018-10-04 Martin Liska <mliska@suse.cz> >> >> PR ipa/82625 >> * g++.dg/ext/pr82625.C: New test. >> --- >> gcc/multiple_target.c | 51 ++++++++++++++++++++++++++++++ >> gcc/testsuite/g++.dg/ext/pr82625.C | 36 +++++++++++++++++++++ >> gcc/tree-inline.c | 2 +- >> 3 files changed, 88 insertions(+), 1 deletion(-) >> create mode 100644 gcc/testsuite/g++.dg/ext/pr82625.C >> >> From e3053abe58eba832262db0af77980012010a642c Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Mon, 8 Oct 2018 11:07:29 +0200 Subject: [PATCH] Limit a MV test just for x86 target. gcc/testsuite/ChangeLog: 2018-10-08 Martin Liska <mliska@suse.cz> * g++.dg/ext/pr82625.C: Add dg-compile filter. --- gcc/testsuite/g++.dg/ext/pr82625.C | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/testsuite/g++.dg/ext/pr82625.C b/gcc/testsuite/g++.dg/ext/pr82625.C index 47bd2df1104..59b174f8c51 100644 --- a/gcc/testsuite/g++.dg/ext/pr82625.C +++ b/gcc/testsuite/g++.dg/ext/pr82625.C @@ -1,6 +1,7 @@ /* { dg-do compile } */ /* { dg-require-ifunc "" } */ /* { dg-options "-O2 -fdump-tree-optimized" } */ +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ __attribute__ ((target ("default"))) static unsigned foo(const char *buf, unsigned size) {
On 08/10/18 10:47, Martin Liška wrote: > On 10/8/18 10:46 AM, Renlin Li wrote: >> Hi Martin, >> >> pr82625.C failed on compiler builds which don't support "default" and "avx" target. >> For example, arm/aarch64 native linux gcc compiler. >> >> >> As I found in this gcc wiki: https://gcc.gnu.org/wiki/FunctionMultiVersioning >> ''' >> This support is available in GCC 4.8 and later. Support is only available in C++ for i386 targets. >> ''' >> >> Should the test be guarded with a target selector or require function multi-versioning instead of ifunc? > > Hi. > > Sure, sorry for the breakage. I'm going to install following tested patch. > > Martin > >> >> Regards, >> Renlin >> >> >> On 10/04/2018 02:56 PM, Martin Liška wrote: >>> Hi. >>> >>> When having a pair of target clones where foo calls bar, if the target >>> attribute are equal we can redirect the call and not use ifunc dispatcher. >>> >>> Patch survives regression tests on x86_64-linux-gnu. >>> Ready for trunk? >>> >>> Martin >>> >>> gcc/ChangeLog: >>> >>> 2018-10-04 Martin Liska <mliska@suse.cz> >>> >>> PR ipa/82625 >>> * multiple_target.c (redirect_to_specific_clone): New function. >>> (ipa_target_clone): Use it. >>> * tree-inline.c: Fix comment. >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2018-10-04 Martin Liska <mliska@suse.cz> >>> >>> PR ipa/82625 >>> * g++.dg/ext/pr82625.C: New test. >>> --- >>> gcc/multiple_target.c | 51 ++++++++++++++++++++++++++++++ >>> gcc/testsuite/g++.dg/ext/pr82625.C | 36 +++++++++++++++++++++ >>> gcc/tree-inline.c | 2 +- >>> 3 files changed, 88 insertions(+), 1 deletion(-) >>> create mode 100644 gcc/testsuite/g++.dg/ext/pr82625.C >>> >>> > > > 0001-Limit-a-MV-test-just-for-x86-target.patch > > > From e3053abe58eba832262db0af77980012010a642c Mon Sep 17 00:00:00 2001 > From: marxin <mliska@suse.cz> > Date: Mon, 8 Oct 2018 11:07:29 +0200 > Subject: [PATCH] Limit a MV test just for x86 target. > > gcc/testsuite/ChangeLog: > > 2018-10-08 Martin Liska <mliska@suse.cz> > > * g++.dg/ext/pr82625.C: Add dg-compile filter. > --- > gcc/testsuite/g++.dg/ext/pr82625.C | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/gcc/testsuite/g++.dg/ext/pr82625.C b/gcc/testsuite/g++.dg/ext/pr82625.C > index 47bd2df1104..59b174f8c51 100644 > --- a/gcc/testsuite/g++.dg/ext/pr82625.C > +++ b/gcc/testsuite/g++.dg/ext/pr82625.C > @@ -1,6 +1,7 @@ > /* { dg-do compile } */ > /* { dg-require-ifunc "" } */ > /* { dg-options "-O2 -fdump-tree-optimized" } */ > +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ > > __attribute__ ((target ("default"))) > static unsigned foo(const char *buf, unsigned size) { > Which begs the question why is this not put under g++.target? R.
On 10/8/18 11:55 AM, Richard Earnshaw (lists) wrote: > On 08/10/18 10:47, Martin Liška wrote: >> On 10/8/18 10:46 AM, Renlin Li wrote: >>> Hi Martin, >>> >>> pr82625.C failed on compiler builds which don't support "default" and "avx" target. >>> For example, arm/aarch64 native linux gcc compiler. >>> >>> >>> As I found in this gcc wiki: https://gcc.gnu.org/wiki/FunctionMultiVersioning >>> ''' >>> This support is available in GCC 4.8 and later. Support is only available in C++ for i386 targets. >>> ''' >>> >>> Should the test be guarded with a target selector or require function multi-versioning instead of ifunc? >> >> Hi. >> >> Sure, sorry for the breakage. I'm going to install following tested patch. >> >> Martin >> >>> >>> Regards, >>> Renlin >>> >>> >>> On 10/04/2018 02:56 PM, Martin Liška wrote: >>>> Hi. >>>> >>>> When having a pair of target clones where foo calls bar, if the target >>>> attribute are equal we can redirect the call and not use ifunc dispatcher. >>>> >>>> Patch survives regression tests on x86_64-linux-gnu. >>>> Ready for trunk? >>>> >>>> Martin >>>> >>>> gcc/ChangeLog: >>>> >>>> 2018-10-04 Martin Liska <mliska@suse.cz> >>>> >>>> PR ipa/82625 >>>> * multiple_target.c (redirect_to_specific_clone): New function. >>>> (ipa_target_clone): Use it. >>>> * tree-inline.c: Fix comment. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2018-10-04 Martin Liska <mliska@suse.cz> >>>> >>>> PR ipa/82625 >>>> * g++.dg/ext/pr82625.C: New test. >>>> --- >>>> gcc/multiple_target.c | 51 ++++++++++++++++++++++++++++++ >>>> gcc/testsuite/g++.dg/ext/pr82625.C | 36 +++++++++++++++++++++ >>>> gcc/tree-inline.c | 2 +- >>>> 3 files changed, 88 insertions(+), 1 deletion(-) >>>> create mode 100644 gcc/testsuite/g++.dg/ext/pr82625.C >>>> >>>> >> >> >> 0001-Limit-a-MV-test-just-for-x86-target.patch >> >> >> From e3053abe58eba832262db0af77980012010a642c Mon Sep 17 00:00:00 2001 >> From: marxin <mliska@suse.cz> >> Date: Mon, 8 Oct 2018 11:07:29 +0200 >> Subject: [PATCH] Limit a MV test just for x86 target. >> >> gcc/testsuite/ChangeLog: >> >> 2018-10-08 Martin Liska <mliska@suse.cz> >> >> * g++.dg/ext/pr82625.C: Add dg-compile filter. >> --- >> gcc/testsuite/g++.dg/ext/pr82625.C | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/gcc/testsuite/g++.dg/ext/pr82625.C b/gcc/testsuite/g++.dg/ext/pr82625.C >> index 47bd2df1104..59b174f8c51 100644 >> --- a/gcc/testsuite/g++.dg/ext/pr82625.C >> +++ b/gcc/testsuite/g++.dg/ext/pr82625.C >> @@ -1,6 +1,7 @@ >> /* { dg-do compile } */ >> /* { dg-require-ifunc "" } */ >> /* { dg-options "-O2 -fdump-tree-optimized" } */ >> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ >> >> __attribute__ ((target ("default"))) >> static unsigned foo(const char *buf, unsigned size) { >> > > Which begs the question why is this not put under g++.target? > > R. > Agree, apparently we have quite some tests that should be moved: gcc/testsuite/g++.dg/ext/pr57362.C:/* { dg-require-ifunc "" } */ gcc/testsuite/g++.dg/ext/pr57548.C:/* { dg-require-ifunc "" } */ gcc/testsuite/g++.dg/ext/pr82625.C:/* { dg-require-ifunc "" } */ gcc/testsuite/g++.dg/ext/pr85329-2.C:/* { dg-require-ifunc "" } */ gcc/testsuite/g++.dg/ext/pr85329.C:/* { dg-require-ifunc "" } */ ... gcc/testsuite/g++.dg/ext/mv*.C I'll prepare patch for it. Martin
On Mon, Oct 8, 2018 at 12:14 PM Martin Liška <mliska@suse.cz> wrote: > > On 10/8/18 11:55 AM, Richard Earnshaw (lists) wrote: > > On 08/10/18 10:47, Martin Liška wrote: > >> On 10/8/18 10:46 AM, Renlin Li wrote: > >>> Hi Martin, > >>> > >>> pr82625.C failed on compiler builds which don't support "default" and "avx" target. > >>> For example, arm/aarch64 native linux gcc compiler. > >>> > >>> > >>> As I found in this gcc wiki: https://gcc.gnu.org/wiki/FunctionMultiVersioning > >>> ''' > >>> This support is available in GCC 4.8 and later. Support is only available in C++ for i386 targets. > >>> ''' > >>> > >>> Should the test be guarded with a target selector or require function multi-versioning instead of ifunc? > >> > >> Hi. > >> > >> Sure, sorry for the breakage. I'm going to install following tested patch. > >> > >> Martin > >> > >>> > >>> Regards, > >>> Renlin > >>> > >>> > >>> On 10/04/2018 02:56 PM, Martin Liška wrote: > >>>> Hi. > >>>> > >>>> When having a pair of target clones where foo calls bar, if the target > >>>> attribute are equal we can redirect the call and not use ifunc dispatcher. > >>>> > >>>> Patch survives regression tests on x86_64-linux-gnu. > >>>> Ready for trunk? > >>>> > >>>> Martin > >>>> > >>>> gcc/ChangeLog: > >>>> > >>>> 2018-10-04 Martin Liska <mliska@suse.cz> > >>>> > >>>> PR ipa/82625 > >>>> * multiple_target.c (redirect_to_specific_clone): New function. > >>>> (ipa_target_clone): Use it. > >>>> * tree-inline.c: Fix comment. > >>>> > >>>> gcc/testsuite/ChangeLog: > >>>> > >>>> 2018-10-04 Martin Liska <mliska@suse.cz> > >>>> > >>>> PR ipa/82625 > >>>> * g++.dg/ext/pr82625.C: New test. > >>>> --- > >>>> gcc/multiple_target.c | 51 ++++++++++++++++++++++++++++++ > >>>> gcc/testsuite/g++.dg/ext/pr82625.C | 36 +++++++++++++++++++++ > >>>> gcc/tree-inline.c | 2 +- > >>>> 3 files changed, 88 insertions(+), 1 deletion(-) > >>>> create mode 100644 gcc/testsuite/g++.dg/ext/pr82625.C > >>>> > >>>> > >> > >> > >> 0001-Limit-a-MV-test-just-for-x86-target.patch > >> > >> > >> From e3053abe58eba832262db0af77980012010a642c Mon Sep 17 00:00:00 2001 > >> From: marxin <mliska@suse.cz> > >> Date: Mon, 8 Oct 2018 11:07:29 +0200 > >> Subject: [PATCH] Limit a MV test just for x86 target. > >> > >> gcc/testsuite/ChangeLog: > >> > >> 2018-10-08 Martin Liska <mliska@suse.cz> > >> > >> * g++.dg/ext/pr82625.C: Add dg-compile filter. > >> --- > >> gcc/testsuite/g++.dg/ext/pr82625.C | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/gcc/testsuite/g++.dg/ext/pr82625.C b/gcc/testsuite/g++.dg/ext/pr82625.C > >> index 47bd2df1104..59b174f8c51 100644 > >> --- a/gcc/testsuite/g++.dg/ext/pr82625.C > >> +++ b/gcc/testsuite/g++.dg/ext/pr82625.C > >> @@ -1,6 +1,7 @@ > >> /* { dg-do compile } */ > >> /* { dg-require-ifunc "" } */ > >> /* { dg-options "-O2 -fdump-tree-optimized" } */ > >> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ > >> > >> __attribute__ ((target ("default"))) > >> static unsigned foo(const char *buf, unsigned size) { > >> > > > > Which begs the question why is this not put under g++.target? > > > > R. > > > > Agree, apparently we have quite some tests that should be moved: > gcc/testsuite/g++.dg/ext/pr57362.C:/* { dg-require-ifunc "" } */ > gcc/testsuite/g++.dg/ext/pr57548.C:/* { dg-require-ifunc "" } */ > gcc/testsuite/g++.dg/ext/pr82625.C:/* { dg-require-ifunc "" } */ > gcc/testsuite/g++.dg/ext/pr85329-2.C:/* { dg-require-ifunc "" } */ > gcc/testsuite/g++.dg/ext/pr85329.C:/* { dg-require-ifunc "" } */ > ... > gcc/testsuite/g++.dg/ext/mv*.C You cannot move C++ tests to gcc.target/ > I'll prepare patch for it. > Martin >
On 10/8/18 12:29 PM, Richard Biener wrote: > On Mon, Oct 8, 2018 at 12:14 PM Martin Liška <mliska@suse.cz> wrote: >> >> On 10/8/18 11:55 AM, Richard Earnshaw (lists) wrote: >>> On 08/10/18 10:47, Martin Liška wrote: >>>> On 10/8/18 10:46 AM, Renlin Li wrote: >>>>> Hi Martin, >>>>> >>>>> pr82625.C failed on compiler builds which don't support "default" and "avx" target. >>>>> For example, arm/aarch64 native linux gcc compiler. >>>>> >>>>> >>>>> As I found in this gcc wiki: https://gcc.gnu.org/wiki/FunctionMultiVersioning >>>>> ''' >>>>> This support is available in GCC 4.8 and later. Support is only available in C++ for i386 targets. >>>>> ''' >>>>> >>>>> Should the test be guarded with a target selector or require function multi-versioning instead of ifunc? >>>> >>>> Hi. >>>> >>>> Sure, sorry for the breakage. I'm going to install following tested patch. >>>> >>>> Martin >>>> >>>>> >>>>> Regards, >>>>> Renlin >>>>> >>>>> >>>>> On 10/04/2018 02:56 PM, Martin Liška wrote: >>>>>> Hi. >>>>>> >>>>>> When having a pair of target clones where foo calls bar, if the target >>>>>> attribute are equal we can redirect the call and not use ifunc dispatcher. >>>>>> >>>>>> Patch survives regression tests on x86_64-linux-gnu. >>>>>> Ready for trunk? >>>>>> >>>>>> Martin >>>>>> >>>>>> gcc/ChangeLog: >>>>>> >>>>>> 2018-10-04 Martin Liska <mliska@suse.cz> >>>>>> >>>>>> PR ipa/82625 >>>>>> * multiple_target.c (redirect_to_specific_clone): New function. >>>>>> (ipa_target_clone): Use it. >>>>>> * tree-inline.c: Fix comment. >>>>>> >>>>>> gcc/testsuite/ChangeLog: >>>>>> >>>>>> 2018-10-04 Martin Liska <mliska@suse.cz> >>>>>> >>>>>> PR ipa/82625 >>>>>> * g++.dg/ext/pr82625.C: New test. >>>>>> --- >>>>>> gcc/multiple_target.c | 51 ++++++++++++++++++++++++++++++ >>>>>> gcc/testsuite/g++.dg/ext/pr82625.C | 36 +++++++++++++++++++++ >>>>>> gcc/tree-inline.c | 2 +- >>>>>> 3 files changed, 88 insertions(+), 1 deletion(-) >>>>>> create mode 100644 gcc/testsuite/g++.dg/ext/pr82625.C >>>>>> >>>>>> >>>> >>>> >>>> 0001-Limit-a-MV-test-just-for-x86-target.patch >>>> >>>> >>>> From e3053abe58eba832262db0af77980012010a642c Mon Sep 17 00:00:00 2001 >>>> From: marxin <mliska@suse.cz> >>>> Date: Mon, 8 Oct 2018 11:07:29 +0200 >>>> Subject: [PATCH] Limit a MV test just for x86 target. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2018-10-08 Martin Liska <mliska@suse.cz> >>>> >>>> * g++.dg/ext/pr82625.C: Add dg-compile filter. >>>> --- >>>> gcc/testsuite/g++.dg/ext/pr82625.C | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/gcc/testsuite/g++.dg/ext/pr82625.C b/gcc/testsuite/g++.dg/ext/pr82625.C >>>> index 47bd2df1104..59b174f8c51 100644 >>>> --- a/gcc/testsuite/g++.dg/ext/pr82625.C >>>> +++ b/gcc/testsuite/g++.dg/ext/pr82625.C >>>> @@ -1,6 +1,7 @@ >>>> /* { dg-do compile } */ >>>> /* { dg-require-ifunc "" } */ >>>> /* { dg-options "-O2 -fdump-tree-optimized" } */ >>>> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ >>>> >>>> __attribute__ ((target ("default"))) >>>> static unsigned foo(const char *buf, unsigned size) { >>>> >>> >>> Which begs the question why is this not put under g++.target? >>> >>> R. >>> >> >> Agree, apparently we have quite some tests that should be moved: >> gcc/testsuite/g++.dg/ext/pr57362.C:/* { dg-require-ifunc "" } */ >> gcc/testsuite/g++.dg/ext/pr57548.C:/* { dg-require-ifunc "" } */ >> gcc/testsuite/g++.dg/ext/pr82625.C:/* { dg-require-ifunc "" } */ >> gcc/testsuite/g++.dg/ext/pr85329-2.C:/* { dg-require-ifunc "" } */ >> gcc/testsuite/g++.dg/ext/pr85329.C:/* { dg-require-ifunc "" } */ >> ... >> gcc/testsuite/g++.dg/ext/mv*.C > > You cannot move C++ tests to gcc.target/ I realized that we don't have ./gcc/testsuite/g++.target/i386 yet :/ Martin > >> I'll prepare patch for it. >> Martin >>
On Mon, Oct 08, 2018 at 12:34:19PM +0200, Martin Liška wrote: > >> gcc/testsuite/g++.dg/ext/mv*.C > > > > You cannot move C++ tests to gcc.target/ > > I realized that we don't have ./gcc/testsuite/g++.target/i386 yet :/ But we want it and there are multiple tests waiting for that move. Jakub
On 10/8/18 12:46 PM, Jakub Jelinek wrote: > On Mon, Oct 08, 2018 at 12:34:19PM +0200, Martin Liška wrote: >>>> gcc/testsuite/g++.dg/ext/mv*.C >>> >>> You cannot move C++ tests to gcc.target/ >> >> I realized that we don't have ./gcc/testsuite/g++.target/i386 yet :/ > > But we want it and there are multiple tests waiting for that move. > > Jakub > I'll try to set it up. Martin
On 08/10/18 11:29, Richard Biener wrote: > On Mon, Oct 8, 2018 at 12:14 PM Martin Liška <mliska@suse.cz> wrote: >> >> On 10/8/18 11:55 AM, Richard Earnshaw (lists) wrote: >>> On 08/10/18 10:47, Martin Liška wrote: >>>> On 10/8/18 10:46 AM, Renlin Li wrote: >>>>> Hi Martin, >>>>> >>>>> pr82625.C failed on compiler builds which don't support "default" and "avx" target. >>>>> For example, arm/aarch64 native linux gcc compiler. >>>>> >>>>> >>>>> As I found in this gcc wiki: https://gcc.gnu.org/wiki/FunctionMultiVersioning >>>>> ''' >>>>> This support is available in GCC 4.8 and later. Support is only available in C++ for i386 targets. >>>>> ''' >>>>> >>>>> Should the test be guarded with a target selector or require function multi-versioning instead of ifunc? >>>> >>>> Hi. >>>> >>>> Sure, sorry for the breakage. I'm going to install following tested patch. >>>> >>>> Martin >>>> >>>>> >>>>> Regards, >>>>> Renlin >>>>> >>>>> >>>>> On 10/04/2018 02:56 PM, Martin Liška wrote: >>>>>> Hi. >>>>>> >>>>>> When having a pair of target clones where foo calls bar, if the target >>>>>> attribute are equal we can redirect the call and not use ifunc dispatcher. >>>>>> >>>>>> Patch survives regression tests on x86_64-linux-gnu. >>>>>> Ready for trunk? >>>>>> >>>>>> Martin >>>>>> >>>>>> gcc/ChangeLog: >>>>>> >>>>>> 2018-10-04 Martin Liska <mliska@suse.cz> >>>>>> >>>>>> PR ipa/82625 >>>>>> * multiple_target.c (redirect_to_specific_clone): New function. >>>>>> (ipa_target_clone): Use it. >>>>>> * tree-inline.c: Fix comment. >>>>>> >>>>>> gcc/testsuite/ChangeLog: >>>>>> >>>>>> 2018-10-04 Martin Liska <mliska@suse.cz> >>>>>> >>>>>> PR ipa/82625 >>>>>> * g++.dg/ext/pr82625.C: New test. >>>>>> --- >>>>>> gcc/multiple_target.c | 51 ++++++++++++++++++++++++++++++ >>>>>> gcc/testsuite/g++.dg/ext/pr82625.C | 36 +++++++++++++++++++++ >>>>>> gcc/tree-inline.c | 2 +- >>>>>> 3 files changed, 88 insertions(+), 1 deletion(-) >>>>>> create mode 100644 gcc/testsuite/g++.dg/ext/pr82625.C >>>>>> >>>>>> >>>> >>>> >>>> 0001-Limit-a-MV-test-just-for-x86-target.patch >>>> >>>> >>>> From e3053abe58eba832262db0af77980012010a642c Mon Sep 17 00:00:00 2001 >>>> From: marxin <mliska@suse.cz> >>>> Date: Mon, 8 Oct 2018 11:07:29 +0200 >>>> Subject: [PATCH] Limit a MV test just for x86 target. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2018-10-08 Martin Liska <mliska@suse.cz> >>>> >>>> * g++.dg/ext/pr82625.C: Add dg-compile filter. >>>> --- >>>> gcc/testsuite/g++.dg/ext/pr82625.C | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/gcc/testsuite/g++.dg/ext/pr82625.C b/gcc/testsuite/g++.dg/ext/pr82625.C >>>> index 47bd2df1104..59b174f8c51 100644 >>>> --- a/gcc/testsuite/g++.dg/ext/pr82625.C >>>> +++ b/gcc/testsuite/g++.dg/ext/pr82625.C >>>> @@ -1,6 +1,7 @@ >>>> /* { dg-do compile } */ >>>> /* { dg-require-ifunc "" } */ >>>> /* { dg-options "-O2 -fdump-tree-optimized" } */ >>>> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ >>>> >>>> __attribute__ ((target ("default"))) >>>> static unsigned foo(const char *buf, unsigned size) { >>>> >>> >>> Which begs the question why is this not put under g++.target? >>> >>> R. >>> >> >> Agree, apparently we have quite some tests that should be moved: >> gcc/testsuite/g++.dg/ext/pr57362.C:/* { dg-require-ifunc "" } */ >> gcc/testsuite/g++.dg/ext/pr57548.C:/* { dg-require-ifunc "" } */ >> gcc/testsuite/g++.dg/ext/pr82625.C:/* { dg-require-ifunc "" } */ >> gcc/testsuite/g++.dg/ext/pr85329-2.C:/* { dg-require-ifunc "" } */ >> gcc/testsuite/g++.dg/ext/pr85329.C:/* { dg-require-ifunc "" } */ >> ... >> gcc/testsuite/g++.dg/ext/mv*.C > > You cannot move C++ tests to gcc.target/ Which is why I said g++.target... R. > >> I'll prepare patch for it. >> Martin >>
diff --git a/gcc/multiple_target.c b/gcc/multiple_target.c index a610d9a3345..2d892f201c5 100644 --- a/gcc/multiple_target.c +++ b/gcc/multiple_target.c @@ -451,6 +451,54 @@ expand_target_clones (struct cgraph_node *node, bool definition) return ret; } +/* When NODE is a target clone, consider all callees and redirect + to a clone with equal target attributes. That prevents multiple + multi-versioning dispatches and a call-chain can be optimized. */ + +static void +redirect_to_specific_clone (cgraph_node *node) +{ + cgraph_function_version_info *fv = node->function_version (); + if (fv == NULL) + return; + + tree attr_target = lookup_attribute ("target", DECL_ATTRIBUTES (node->decl)); + if (attr_target == NULL_TREE) + return; + + /* We need to remember NEXT_CALLER as it could be modified in the loop. */ + for (cgraph_edge *e = node->callees; e ; e = e->next_callee) + { + cgraph_function_version_info *fv2 = e->callee->function_version (); + if (!fv2) + continue; + + tree attr_target2 = lookup_attribute ("target", + DECL_ATTRIBUTES (e->callee->decl)); + + /* Function is not calling proper target clone. */ + if (!attribute_list_equal (attr_target, attr_target2)) + { + while (fv2->prev != NULL) + fv2 = fv2->prev; + + /* Try to find a clone with equal target attribute. */ + for (; fv2 != NULL; fv2 = fv2->next) + { + cgraph_node *callee = fv2->this_node; + attr_target2 = lookup_attribute ("target", + DECL_ATTRIBUTES (callee->decl)); + if (attribute_list_equal (attr_target, attr_target2)) + { + e->redirect_callee (callee); + e->redirect_call_stmt_to_callee (); + break; + } + } + } + } +} + static unsigned int ipa_target_clone (void) { @@ -464,6 +512,9 @@ ipa_target_clone (void) for (unsigned i = 0; i < to_dispatch.length (); i++) create_dispatcher_calls (to_dispatch[i]); + FOR_EACH_FUNCTION (node) + redirect_to_specific_clone (node); + return 0; } diff --git a/gcc/testsuite/g++.dg/ext/pr82625.C b/gcc/testsuite/g++.dg/ext/pr82625.C new file mode 100644 index 00000000000..47bd2df1104 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/pr82625.C @@ -0,0 +1,36 @@ +/* { dg-do compile } */ +/* { dg-require-ifunc "" } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +__attribute__ ((target ("default"))) +static unsigned foo(const char *buf, unsigned size) { + return 1; +} + +__attribute__ ((target ("avx"))) +static unsigned foo(const char *buf, unsigned size) { + return 2; +} + +__attribute__ ((target ("default"))) +unsigned bar() { + char buf[4096]; + unsigned acc = 0; + for (int i = 0; i < sizeof(buf); i++) { + acc += foo(&buf[i], 1); + } + return acc; +} + +__attribute__ ((target ("avx"))) +unsigned bar() { + char buf[4096]; + unsigned acc = 0; + for (int i = 0; i < sizeof(buf); i++) { + acc += foo(&buf[i], 1); + } + return acc; +} + +/* { dg-final { scan-tree-dump-times "return 4096;" 1 "optimized" } } */ +/* { dg-final { scan-tree-dump-times "return 8192;" 1 "optimized" } } */ diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index ff8ee8ce78f..913425394e0 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -2631,7 +2631,7 @@ copy_loops (copy_body_data *id, } } -/* Call cgraph_redirect_edge_call_stmt_to_callee on all calls in BB */ +/* Call redirect_call_stmt_to_callee on all calls in BB. */ void redirect_all_calls (copy_body_data * id, basic_block bb)