* [PATCH] rs6000 PR83660 fix ICE with vec_extract
@ 2018-04-13 20:37 Aaron Sawdey
2018-04-13 23:25 ` Segher Boessenkool
2018-04-23 16:42 ` Aaron Sawdey
0 siblings, 2 replies; 4+ messages in thread
From: Aaron Sawdey @ 2018-04-13 20:37 UTC (permalink / raw)
To: gcc-patches; +Cc: Segher Boessenkool, David Edelsohn
[-- Attachment #1: Type: text/plain, Size: 989 bytes --]
Per the discussion on the 83660, I've come to a minimal patch to
prevent this. Basically marking the vec_extract tree as having side
effects later makes sure that it gets all the cleanup points it needs
so that gimplify_cleanup_point_expr () is happy. Also because
vec_insert puts a MODIFY_EXPR in there, it has side effects and this
problem will not occur.
Doing bootstrap/regtest on ppc64le with -mcpu=power7 since that is
where this issue arises. OK for trunk if everything passes?
Thanks,
Aaron
2018-04-13 Aaron Sawdey <acsawdey@linux.ibm.com>
PR target/83660
* config/rs6000/rs6000-c.c (altivec_resolve_overloaded_builtin): Mark
vec_extract expression as having side effects to make sure it gets
a cleanup point.
2018-04-13 Aaron Sawdey <acsawdey@linux.ibm.com>
PR target/83660
* gcc.target/powerpc/pr83660.C: New test.
--
Aaron Sawdey, Ph.D. acsawdey@linux.vnet.ibm.com
050-2/C113 (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain
[-- Attachment #2: pr83660.patch --]
[-- Type: text/x-patch, Size: 1312 bytes --]
Index: config/rs6000/rs6000-c.c
===================================================================
--- config/rs6000/rs6000-c.c (revision 259353)
+++ config/rs6000/rs6000-c.c (working copy)
@@ -6705,6 +6705,15 @@
stmt = build_binary_op (loc, PLUS_EXPR, stmt, arg2, 1);
stmt = build_indirect_ref (loc, stmt, RO_NULL);
+ /* PR83660: We mark this as having side effects so that
+ downstream in fold_build_cleanup_point_expr () it will get a
+ CLEANUP_POINT_EXPR. If it does not we can run into an ICE
+ later in gimplify_cleanup_point_expr (). Potentially this
+ causes missed optimization because the actually is no side
+ effect. */
+ if (c_dialect_cxx ())
+ TREE_SIDE_EFFECTS (stmt) = 1;
+
return stmt;
}
Index: testsuite/gcc.target/powerpc/pr83660.C
===================================================================
--- testsuite/gcc.target/powerpc/pr83660.C (nonexistent)
+++ testsuite/gcc.target/powerpc/pr83660.C (working copy)
@@ -0,0 +1,14 @@
+/* PR target/83660 */
+/* { dg-do compile } */
+/* { dg-options "-mcpu=power7" } */
+
+#include <altivec.h>
+
+typedef __vector unsigned int uvec32_t __attribute__((__aligned__(16)));
+
+unsigned get_word(uvec32_t v)
+{
+ return ({const unsigned _B1 = 32;
+ vec_extract((uvec32_t)v, 2);});
+}
+
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] rs6000 PR83660 fix ICE with vec_extract
2018-04-13 20:37 [PATCH] rs6000 PR83660 fix ICE with vec_extract Aaron Sawdey
@ 2018-04-13 23:25 ` Segher Boessenkool
2018-04-23 16:42 ` Aaron Sawdey
1 sibling, 0 replies; 4+ messages in thread
From: Segher Boessenkool @ 2018-04-13 23:25 UTC (permalink / raw)
To: Aaron Sawdey; +Cc: gcc-patches, David Edelsohn
Hi!
On Fri, Apr 13, 2018 at 03:37:08PM -0500, Aaron Sawdey wrote:
> Per the discussion on the 83660, I've come to a minimal patch to
> prevent this. Basically marking the vec_extract tree as having side
> effects later makes sure that it gets all the cleanup points it needs
> so that gimplify_cleanup_point_expr () is happy. Also because
> vec_insert puts a MODIFY_EXPR in there, it has side effects and this
> problem will not occur.
>
> Doing bootstrap/regtest on ppc64le with -mcpu=power7 since that is
> where this issue arises. OK for trunk if everything passes?
> --- testsuite/gcc.target/powerpc/pr83660.C (nonexistent)
> +++ testsuite/gcc.target/powerpc/pr83660.C (working copy)
> @@ -0,0 +1,14 @@
> +/* PR target/83660 */
> +/* { dg-do compile } */
> +/* { dg-options "-mcpu=power7" } */
You'll need to skip this test if the user overrides -mcpu=; see the many
other testcases that do this.
With that fixed, okay for trunk. Thanks!
Segher
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] rs6000 PR83660 fix ICE with vec_extract
2018-04-13 20:37 [PATCH] rs6000 PR83660 fix ICE with vec_extract Aaron Sawdey
2018-04-13 23:25 ` Segher Boessenkool
@ 2018-04-23 16:42 ` Aaron Sawdey
2018-04-23 17:19 ` Segher Boessenkool
1 sibling, 1 reply; 4+ messages in thread
From: Aaron Sawdey @ 2018-04-23 16:42 UTC (permalink / raw)
To: gcc-patches; +Cc: Segher Boessenkool, David Edelsohn
This also affects gcc 7 and is fixed by the same patch. I've tested the
backport to 7 on ppc64le and it causes no new fails. OK for backport to
7 (and 6 if it's also needed there)?
Thanks,
Aaron
On Fri, 2018-04-13 at 15:37 -0500, Aaron Sawdey wrote:
> Per the discussion on the 83660, I've come to a minimal patch to
> prevent this. Basically marking the vec_extract tree as having side
> effects later makes sure that it gets all the cleanup points it needs
> so that gimplify_cleanup_point_expr () is happy. Also because
> vec_insert puts a MODIFY_EXPR in there, it has side effects and this
> problem will not occur.
>
> Doing bootstrap/regtest on ppc64le with -mcpu=power7 since that is
> where this issue arises. OK for trunk if everything passes?
>
> Thanks,
> Aaron
>
>
> 2018-04-13 Aaron Sawdey <acsawdey@linux.ibm.com>
>
> PR target/83660
> * config/rs6000/rs6000-c.c
> (altivec_resolve_overloaded_builtin): Mark
> vec_extract expression as having side effects to make sure it
> gets
> a cleanup point.
>
> 2018-04-13 Aaron Sawdey <acsawdey@linux.ibm.com>
>
> PR target/83660
> * gcc.target/powerpc/pr83660.C: New test.
>
--
Aaron Sawdey, Ph.D. acsawdey@linux.vnet.ibm.com
050-2/C113 (507) 253-7520 home: 507/263-0782
IBM Linux Technology Center - PPC Toolchain
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rs6000 PR83660 fix ICE with vec_extract
2018-04-23 16:42 ` Aaron Sawdey
@ 2018-04-23 17:19 ` Segher Boessenkool
0 siblings, 0 replies; 4+ messages in thread
From: Segher Boessenkool @ 2018-04-23 17:19 UTC (permalink / raw)
To: Aaron Sawdey; +Cc: gcc-patches, David Edelsohn
On Mon, Apr 23, 2018 at 11:36:20AM -0500, Aaron Sawdey wrote:
> This also affects gcc 7 and is fixed by the same patch. I've tested the
> backport to 7 on ppc64le and it causes no new fails. OK for backport to
> 7 (and 6 if it's also needed there)?
Yes please (for both). Thanks!
Segher
> On Fri, 2018-04-13 at 15:37 -0500, Aaron Sawdey wrote:
> > Per the discussion on the 83660, I've come to a minimal patch to
> > prevent this. Basically marking the vec_extract tree as having side
> > effects later makes sure that it gets all the cleanup points it needs
> > so that gimplify_cleanup_point_expr () is happy. Also because
> > vec_insert puts a MODIFY_EXPR in there, it has side effects and this
> > problem will not occur.
> >
> > Doing bootstrap/regtest on ppc64le with -mcpu=power7 since that is
> > where this issue arises. OK for trunk if everything passes?
> >
> > Thanks,
> > Aaron
> >
> >
> > 2018-04-13 Aaron Sawdey <acsawdey@linux.ibm.com>
> >
> > PR target/83660
> > * config/rs6000/rs6000-c.c
> > (altivec_resolve_overloaded_builtin): Mark
> > vec_extract expression as having side effects to make sure it
> > gets
> > a cleanup point.
> >
> > 2018-04-13 Aaron Sawdey <acsawdey@linux.ibm.com>
> >
> > PR target/83660
> > * gcc.target/powerpc/pr83660.C: New test.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-23 16:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-13 20:37 [PATCH] rs6000 PR83660 fix ICE with vec_extract Aaron Sawdey
2018-04-13 23:25 ` Segher Boessenkool
2018-04-23 16:42 ` Aaron Sawdey
2018-04-23 17:19 ` Segher Boessenkool
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).