* [PATCH] disable IPA-cp cloning for functions with target_clones attribute
@ 2016-06-24 20:56 Evgeny Stupachenko
2016-07-12 3:18 ` Evgeny Stupachenko
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Evgeny Stupachenko @ 2016-06-24 20:56 UTC (permalink / raw)
To: GCC Patches, Jan Hubicka,
Илья
Энкович,
Jeff Law
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);
+}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] disable IPA-cp cloning for functions with target_clones attribute
2016-06-24 20:56 [PATCH] disable IPA-cp cloning for functions with target_clones attribute Evgeny Stupachenko
@ 2016-07-12 3:18 ` Evgeny Stupachenko
2016-07-12 9:31 ` Martin Jambor
2016-07-13 14:47 ` Jeff Law
2 siblings, 0 replies; 7+ messages in thread
From: Evgeny Stupachenko @ 2016-07-12 3:18 UTC (permalink / raw)
To: GCC Patches, Jan Hubicka,
Илья
Энкович,
Jeff Law
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);
> +}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] disable IPA-cp cloning for functions with target_clones attribute
2016-06-24 20:56 [PATCH] disable IPA-cp cloning for functions with target_clones attribute Evgeny Stupachenko
2016-07-12 3:18 ` Evgeny Stupachenko
@ 2016-07-12 9:31 ` Martin Jambor
2016-07-13 9:48 ` Jan Hubicka
2016-07-13 14:46 ` Jeff Law
2016-07-13 14:47 ` Jeff Law
2 siblings, 2 replies; 7+ messages in thread
From: Martin Jambor @ 2016-07-12 9:31 UTC (permalink / raw)
To: Evgeny Stupachenko
Cc: GCC Patches, Jan Hubicka,
Илья
Энкович,
Jeff Law
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);
> +}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] disable IPA-cp cloning for functions with target_clones attribute
2016-07-12 9:31 ` Martin Jambor
@ 2016-07-13 9:48 ` Jan Hubicka
2016-07-13 14:46 ` Jeff Law
1 sibling, 0 replies; 7+ messages in thread
From: Jan Hubicka @ 2016-07-13 9:48 UTC (permalink / raw)
To: Evgeny Stupachenko, GCC Patches, Jan Hubicka,
�?л�?�?
Энкови�?,
Jeff Law
> 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);
> > +}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] disable IPA-cp cloning for functions with target_clones attribute
2016-07-12 9:31 ` Martin Jambor
2016-07-13 9:48 ` Jan Hubicka
@ 2016-07-13 14:46 ` Jeff Law
2016-07-14 12:53 ` Martin Jambor
1 sibling, 1 reply; 7+ messages in thread
From: Jeff Law @ 2016-07-13 14:46 UTC (permalink / raw)
To: Evgeny Stupachenko, GCC Patches, Jan Hubicka,
Илья
Энкович
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] disable IPA-cp cloning for functions with target_clones attribute
2016-06-24 20:56 [PATCH] disable IPA-cp cloning for functions with target_clones attribute Evgeny Stupachenko
2016-07-12 3:18 ` Evgeny Stupachenko
2016-07-12 9:31 ` Martin Jambor
@ 2016-07-13 14:47 ` Jeff Law
2 siblings, 0 replies; 7+ messages in thread
From: Jeff Law @ 2016-07-13 14:47 UTC (permalink / raw)
To: Evgeny Stupachenko, GCC Patches, Jan Hubicka,
Илья
Энкович
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] disable IPA-cp cloning for functions with target_clones attribute
2016-07-13 14:46 ` Jeff Law
@ 2016-07-14 12:53 ` Martin Jambor
0 siblings, 0 replies; 7+ messages in thread
From: Martin Jambor @ 2016-07-14 12:53 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC Patches, Jan Hubicka
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-07-14 12:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-24 20:56 [PATCH] disable IPA-cp cloning for functions with target_clones attribute Evgeny Stupachenko
2016-07-12 3:18 ` Evgeny Stupachenko
2016-07-12 9:31 ` Martin Jambor
2016-07-13 9:48 ` Jan Hubicka
2016-07-13 14:46 ` Jeff Law
2016-07-14 12:53 ` Martin Jambor
2016-07-13 14:47 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).