| Message ID | CAOvf_xxP90cQkoLe_AUm=QqbcyUcA_xe-F=wGohw=1nWf-+nWg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
PING. On Fri, Jun 24, 2016 at 1:41 PM, Evgeny Stupachenko <evstupac@gmail.com> wrote: > Hi, > > Fix ICE when IPA-cp and target_clones are applied to the same function. > Is the patch ok for trunk? > > Thanks, > Evgeny > > 2016-06-24 Evgeny Stupachenko <evstupac@gmail.com> > > gcc/ > * ipa-cp.c (determine_versionability): Do not create constprop clones, > when target_clones attribute is set. > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > index 2710494..4b642ba 100644 > --- a/gcc/ipa-cp.c > +++ b/gcc/ipa-cp.c > @@ -533,6 +533,13 @@ determine_versionability (struct cgraph_node *node, > coexist, but that may not be worth the effort. */ > reason = "function has SIMD clones"; > } > + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (node->decl))) > + { > + /* Ideally we should clone the target clones themselves and create > + copies of them, so IPA-cp and target clones can happily > + coexist, but that may not be worth the effort. */ > + reason = "function target_clones attribute"; > + } > /* Don't clone decls local to a comdat group; it breaks and for C++ > decloned constructors, inlining is always better anyway. */ > else if (node->comdat_local_p ()) > diff --git a/gcc/testsuite/gcc.target/i386/mvc8.c > b/gcc/testsuite/gcc.target/i386/mvc8.c > new file mode 100644 > index 0000000..e9ab9e1 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/mvc8.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } */ > +/* { dg-require-ifunc "" } */ > +/* { dg-options "-O3 -fno-inline" } */ > +/* { dg-final { scan-assembler-not "constprop" } } */ > +__attribute__((target_clones("arch=core-avx2","arch=slm","default"))) > +void foo (float *a, int b) { > + *a = (float)b; > +} > +float a; > +int main() { > + int i; > + for (i = 0; i < 1024; i++) > + foo (&a, 5); > +}
Hi, On Fri, Jun 24, 2016 at 01:41:05PM -0700, Evgeny Stupachenko wrote: > Hi, > > Fix ICE when IPA-cp and target_clones are applied to the same function. > Is the patch ok for trunk? I can't approve anything but since I wrote most of IPA-CP, it may count that I am fine with the patch. However, it should also be backported to the 6 branch. In the future, it is useful to file a bugzilla PR for issues like this, even if you also provide a fix. It helps with tracking backports and with a PR, you would have probably caught my attention sooner. In any event, thanks for addressing this. Martin > > Thanks, > Evgeny > > 2016-06-24 Evgeny Stupachenko <evstupac@gmail.com> > > gcc/ > * ipa-cp.c (determine_versionability): Do not create constprop clones, > when target_clones attribute is set. > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > index 2710494..4b642ba 100644 > --- a/gcc/ipa-cp.c > +++ b/gcc/ipa-cp.c > @@ -533,6 +533,13 @@ determine_versionability (struct cgraph_node *node, > coexist, but that may not be worth the effort. */ > reason = "function has SIMD clones"; > } > + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (node->decl))) > + { > + /* Ideally we should clone the target clones themselves and create > + copies of them, so IPA-cp and target clones can happily > + coexist, but that may not be worth the effort. */ > + reason = "function target_clones attribute"; > + } > /* Don't clone decls local to a comdat group; it breaks and for C++ > decloned constructors, inlining is always better anyway. */ > else if (node->comdat_local_p ()) > diff --git a/gcc/testsuite/gcc.target/i386/mvc8.c > b/gcc/testsuite/gcc.target/i386/mvc8.c > new file mode 100644 > index 0000000..e9ab9e1 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/mvc8.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } */ > +/* { dg-require-ifunc "" } */ > +/* { dg-options "-O3 -fno-inline" } */ > +/* { dg-final { scan-assembler-not "constprop" } } */ > +__attribute__((target_clones("arch=core-avx2","arch=slm","default"))) > +void foo (float *a, int b) { > + *a = (float)b; > +} > +float a; > +int main() { > + int i; > + for (i = 0; i < 1024; i++) > + foo (&a, 5); > +}
> Hi, > > On Fri, Jun 24, 2016 at 01:41:05PM -0700, Evgeny Stupachenko wrote: > > Hi, > > > > Fix ICE when IPA-cp and target_clones are applied to the same function. > > Is the patch ok for trunk? > > I can't approve anything but since I wrote most of IPA-CP, it may > count that I am fine with the patch. Yep, the patch is OK Honza > > However, it should also be backported to the 6 branch. > > In the future, it is useful to file a bugzilla PR for issues like > this, even if you also provide a fix. It helps with tracking > backports and with a PR, you would have probably caught my attention > sooner. > > In any event, thanks for addressing this. > > Martin > > > > > Thanks, > > Evgeny > > > > 2016-06-24 Evgeny Stupachenko <evstupac@gmail.com> > > > > gcc/ > > * ipa-cp.c (determine_versionability): Do not create constprop clones, > > when target_clones attribute is set. > > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > > index 2710494..4b642ba 100644 > > --- a/gcc/ipa-cp.c > > +++ b/gcc/ipa-cp.c > > @@ -533,6 +533,13 @@ determine_versionability (struct cgraph_node *node, > > coexist, but that may not be worth the effort. */ > > reason = "function has SIMD clones"; > > } > > + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (node->decl))) > > + { > > + /* Ideally we should clone the target clones themselves and create > > + copies of them, so IPA-cp and target clones can happily > > + coexist, but that may not be worth the effort. */ > > + reason = "function target_clones attribute"; > > + } > > /* Don't clone decls local to a comdat group; it breaks and for C++ > > decloned constructors, inlining is always better anyway. */ > > else if (node->comdat_local_p ()) > > diff --git a/gcc/testsuite/gcc.target/i386/mvc8.c > > b/gcc/testsuite/gcc.target/i386/mvc8.c > > new file mode 100644 > > index 0000000..e9ab9e1 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/mvc8.c > > @@ -0,0 +1,14 @@ > > +/* { dg-do compile } */ > > +/* { dg-require-ifunc "" } */ > > +/* { dg-options "-O3 -fno-inline" } */ > > +/* { dg-final { scan-assembler-not "constprop" } } */ > > +__attribute__((target_clones("arch=core-avx2","arch=slm","default"))) > > +void foo (float *a, int b) { > > + *a = (float)b; > > +} > > +float a; > > +int main() { > > + int i; > > + for (i = 0; i < 1024; i++) > > + foo (&a, 5); > > +}
On 07/12/2016 03:31 AM, Martin Jambor wrote: > Hi, > > On Fri, Jun 24, 2016 at 01:41:05PM -0700, Evgeny Stupachenko wrote: >> Hi, >> >> Fix ICE when IPA-cp and target_clones are applied to the same function. >> Is the patch ok for trunk? > > I can't approve anything but since I wrote most of IPA-CP, it may > count that I am fine with the patch. Martin, we can probably change that ;-) Interested? Jeff
On 06/24/2016 02:41 PM, Evgeny Stupachenko wrote: > Hi, > > Fix ICE when IPA-cp and target_clones are applied to the same function. > Is the patch ok for trunk? > > Thanks, > Evgeny > > 2016-06-24 Evgeny Stupachenko <evstupac@gmail.com> > > gcc/ > * ipa-cp.c (determine_versionability): Do not create constprop clones, > when target_clones attribute is set. OK. As Martin suggested, please backport to the gcc-6 branch. Thanks, Jeff
Hi Jeff, On Wed, Jul 13, 2016 at 08:46:35AM -0600, Jeff Law wrote: > On 07/12/2016 03:31 AM, Martin Jambor wrote: > > Hi, > > > > On Fri, Jun 24, 2016 at 01:41:05PM -0700, Evgeny Stupachenko wrote: > > > Hi, > > > > > > Fix ICE when IPA-cp and target_clones are applied to the same function. > > > Is the patch ok for trunk? > > > > I can't approve anything but since I wrote most of IPA-CP, it may > > count that I am fine with the patch. > Martin, we can probably change that ;-) Interested? I have briefly discussed this with Honza and yes, I would be interested in maintaining IPA-CP (which in practice comes down to ipa-prop.[ch] and ipa-cp.c files now). Even in that position I would coordinate closely with Honza whenever it would come to more substantial changes. Thanks for considering me for the maintainership, Martin
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index 2710494..4b642ba 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -533,6 +533,13 @@ determine_versionability (struct cgraph_node *node, coexist, but that may not be worth the effort. */ reason = "function has SIMD clones"; } + else if (lookup_attribute ("target_clones", DECL_ATTRIBUTES (node->decl))) + { + /* Ideally we should clone the target clones themselves and create + copies of them, so IPA-cp and target clones can happily + coexist, but that may not be worth the effort. */ + reason = "function target_clones attribute"; + } /* Don't clone decls local to a comdat group; it breaks and for C++ decloned constructors, inlining is always better anyway. */ else if (node->comdat_local_p ()) diff --git a/gcc/testsuite/gcc.target/i386/mvc8.c b/gcc/testsuite/gcc.target/i386/mvc8.c new file mode 100644 index 0000000..e9ab9e1 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/mvc8.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-require-ifunc "" } */ +/* { dg-options "-O3 -fno-inline" } */ +/* { dg-final { scan-assembler-not "constprop" } } */ +__attribute__((target_clones("arch=core-avx2","arch=slm","default"))) +void foo (float *a, int b) { + *a = (float)b; +} +float a; +int main() { + int i; + for (i = 0; i < 1024; i++) + foo (&a, 5); +}
Hi, Fix ICE when IPA-cp and target_clones are applied to the same function. Is the patch ok for trunk? Thanks, Evgeny 2016-06-24 Evgeny Stupachenko <evstupac@gmail.com> gcc/ * ipa-cp.c (determine_versionability): Do not create constprop clones, when target_clones attribute is set.