* [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307)
@ 2016-04-01 15:03 Marek Polacek
2016-04-01 15:54 ` Jeff Law
0 siblings, 1 reply; 9+ messages in thread
From: Marek Polacek @ 2016-04-01 15:03 UTC (permalink / raw)
To: GCC Patches, Joseph Myers
This is another case where a C_MAYBE_CONST_EXPR leaks into the gimplifier.
Starting with r229128 and thus introduction of build_vec_cmp we now create
VEC_COND_EXPR when building a vector comparison. The C_MAYBE_CONST_EXPR
originated in build_compound_literal when creating a COMPOUND_LITERAL_EXPR.
This is then made a part of op0 of a VEC_COND_EXPR. But c_fully_fold_internal
doesn't know what to do with VEC_COND_EXPRs so the C_MAYBE_CONST_EXPR went
unnoticed into fold, oops. The fix here is to teach c_fully_fold_internal
how to handle VEC_COND_EXPRs, which is what this patch attempts to do.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-04-01 Marek Polacek <polacek@redhat.com>
PR c/70307
* c-fold.c (c_fully_fold_internal): Handle VEC_COND_EXPR.
* gcc.dg/torture/pr70307.c: New test.
diff --git gcc/c/c-fold.c gcc/c/c-fold.c
index f07917f..d512824 100644
--- gcc/c/c-fold.c
+++ gcc/c/c-fold.c
@@ -528,6 +528,23 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
*maybe_const_itself &= op2_const_self;
goto out;
+ case VEC_COND_EXPR:
+ orig_op0 = op0 = TREE_OPERAND (expr, 0);
+ op1 = TREE_OPERAND (expr, 1);
+ op2 = TREE_OPERAND (expr, 2);
+ op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
+ maybe_const_itself, for_int_const);
+ STRIP_TYPE_NOPS (op0);
+
+ /* OP1 will be a vector of -1 and OP2 a vector if 0, as created in
+ build_vec_cmp -- no need to fold them. */
+
+ if (op0 != orig_op0)
+ ret = fold_build3_loc (loc, code, TREE_TYPE (expr), op0, op1, op2);
+ else
+ ret = fold (expr);
+ goto out;
+
case EXCESS_PRECISION_EXPR:
/* Each case where an operand with excess precision may be
encountered must remove the EXCESS_PRECISION_EXPR around
diff --git gcc/testsuite/gcc.dg/torture/pr70307.c gcc/testsuite/gcc.dg/torture/pr70307.c
index e69de29..d47c4b6 100644
--- gcc/testsuite/gcc.dg/torture/pr70307.c
+++ gcc/testsuite/gcc.dg/torture/pr70307.c
@@ -0,0 +1,62 @@
+/* PR c/70307 */
+/* { dg-do compile } */
+
+typedef int v4si __attribute__ ((vector_size (16)));
+
+v4si foo (v4si);
+
+v4si
+fn1 (int i)
+{
+ return i <= (v4si){(0, 0)};
+}
+
+v4si
+fn2 (int i)
+{
+ v4si r;
+ r = i <= (v4si){(0, 0)};
+ return r;
+}
+
+v4si
+fn3 (int i)
+{
+ return foo (i <= (v4si){(0, 0)});
+}
+
+v4si
+fn4 (int i)
+{
+ struct S { v4si v; };
+ struct S s = { .v = i <= (v4si){(0, 0)} };
+ return s.v;
+}
+
+v4si
+fn5 (int i)
+{
+ return (v4si){(1, i++)} == (v4si){(0, 0)};
+}
+
+v4si
+fn6 (int i)
+{
+ v4si r;
+ r = (v4si){(1, i++)} == (v4si){(0, 0)};
+ return r;
+}
+
+v4si
+fn7 (int i)
+{
+ return foo ((v4si){(1, i++)} == (v4si){(0, 0)});
+}
+
+v4si
+fn8 (int i)
+{
+ struct S { v4si v; };
+ struct S s = { .v = (v4si){(1, i++)} == (v4si){(0, 0)} };
+ return s.v;
+}
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307)
2016-04-01 15:03 [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307) Marek Polacek
@ 2016-04-01 15:54 ` Jeff Law
2016-04-01 16:02 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Jeff Law @ 2016-04-01 15:54 UTC (permalink / raw)
To: Marek Polacek, GCC Patches, Joseph Myers
On 04/01/2016 09:03 AM, Marek Polacek wrote:
> This is another case where a C_MAYBE_CONST_EXPR leaks into the gimplifier.
> Starting with r229128 and thus introduction of build_vec_cmp we now create
> VEC_COND_EXPR when building a vector comparison. The C_MAYBE_CONST_EXPR
> originated in build_compound_literal when creating a COMPOUND_LITERAL_EXPR.
> This is then made a part of op0 of a VEC_COND_EXPR. But c_fully_fold_internal
> doesn't know what to do with VEC_COND_EXPRs so the C_MAYBE_CONST_EXPR went
> unnoticed into fold, oops. The fix here is to teach c_fully_fold_internal
> how to handle VEC_COND_EXPRs, which is what this patch attempts to do.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-04-01 Marek Polacek <polacek@redhat.com>
>
> PR c/70307
> * c-fold.c (c_fully_fold_internal): Handle VEC_COND_EXPR.
>
> * gcc.dg/torture/pr70307.c: New test.
>
> diff --git gcc/c/c-fold.c gcc/c/c-fold.c
> index f07917f..d512824 100644
> --- gcc/c/c-fold.c
> +++ gcc/c/c-fold.c
> @@ -528,6 +528,23 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
> *maybe_const_itself &= op2_const_self;
> goto out;
>
> + case VEC_COND_EXPR:
> + orig_op0 = op0 = TREE_OPERAND (expr, 0);
> + op1 = TREE_OPERAND (expr, 1);
> + op2 = TREE_OPERAND (expr, 2);
> + op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
> + maybe_const_itself, for_int_const);
> + STRIP_TYPE_NOPS (op0);
> +
> + /* OP1 will be a vector of -1 and OP2 a vector if 0, as created in
> + build_vec_cmp -- no need to fold them. */
Is this worth verifying with a gcc_assert? Your call.
OK with or without the gcc_assert that op1/op2 are constants.
Jeff
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307)
2016-04-01 15:54 ` Jeff Law
@ 2016-04-01 16:02 ` Jakub Jelinek
2016-04-01 16:14 ` Marek Polacek
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2016-04-01 16:02 UTC (permalink / raw)
To: Jeff Law; +Cc: Marek Polacek, GCC Patches, Joseph Myers
On Fri, Apr 01, 2016 at 09:54:39AM -0600, Jeff Law wrote:
> >--- gcc/c/c-fold.c
> >+++ gcc/c/c-fold.c
> >@@ -528,6 +528,23 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
> > *maybe_const_itself &= op2_const_self;
> > goto out;
> >
> >+ case VEC_COND_EXPR:
> >+ orig_op0 = op0 = TREE_OPERAND (expr, 0);
> >+ op1 = TREE_OPERAND (expr, 1);
> >+ op2 = TREE_OPERAND (expr, 2);
> >+ op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
> >+ maybe_const_itself, for_int_const);
> >+ STRIP_TYPE_NOPS (op0);
> >+
> >+ /* OP1 will be a vector of -1 and OP2 a vector if 0, as created in
> >+ build_vec_cmp -- no need to fold them. */
> Is this worth verifying with a gcc_assert? Your call.
>
> OK with or without the gcc_assert that op1/op2 are constants.
I think either we should have an gcc_checking_assert, or can't
we just handle VEC_COND_EXPR like COND_EXPR, with no extra code?
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307)
2016-04-01 16:02 ` Jakub Jelinek
@ 2016-04-01 16:14 ` Marek Polacek
2016-04-01 16:18 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Marek Polacek @ 2016-04-01 16:14 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jeff Law, GCC Patches, Joseph Myers
On Fri, Apr 01, 2016 at 06:02:24PM +0200, Jakub Jelinek wrote:
> On Fri, Apr 01, 2016 at 09:54:39AM -0600, Jeff Law wrote:
> > >--- gcc/c/c-fold.c
> > >+++ gcc/c/c-fold.c
> > >@@ -528,6 +528,23 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
> > > *maybe_const_itself &= op2_const_self;
> > > goto out;
> > >
> > >+ case VEC_COND_EXPR:
> > >+ orig_op0 = op0 = TREE_OPERAND (expr, 0);
> > >+ op1 = TREE_OPERAND (expr, 1);
> > >+ op2 = TREE_OPERAND (expr, 2);
> > >+ op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
> > >+ maybe_const_itself, for_int_const);
> > >+ STRIP_TYPE_NOPS (op0);
> > >+
> > >+ /* OP1 will be a vector of -1 and OP2 a vector if 0, as created in
> > >+ build_vec_cmp -- no need to fold them. */
> > Is this worth verifying with a gcc_assert? Your call.
> >
> > OK with or without the gcc_assert that op1/op2 are constants.
I'm going to add the asserts. I should've add them in the first place.
> I think either we should have an gcc_checking_assert, or can't
> we just handle VEC_COND_EXPR like COND_EXPR, with no extra code?
It works (I seem to remember testing that variant), but the COND_EXPR case
code does a lot of stuff that doesn't seem to make sense for vectors, like
checking for truthvalue_false_node, folding op1/op2, playing games with
maybe_const_* stuff... So I decided to add a new case.
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307)
2016-04-01 16:14 ` Marek Polacek
@ 2016-04-01 16:18 ` Jakub Jelinek
2016-04-01 17:10 ` Marek Polacek
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2016-04-01 16:18 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jeff Law, GCC Patches, Joseph Myers
On Fri, Apr 01, 2016 at 06:14:00PM +0200, Marek Polacek wrote:
> On Fri, Apr 01, 2016 at 06:02:24PM +0200, Jakub Jelinek wrote:
> > On Fri, Apr 01, 2016 at 09:54:39AM -0600, Jeff Law wrote:
> > > >--- gcc/c/c-fold.c
> > > >+++ gcc/c/c-fold.c
> > > >@@ -528,6 +528,23 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
> > > > *maybe_const_itself &= op2_const_self;
> > > > goto out;
> > > >
> > > >+ case VEC_COND_EXPR:
> > > >+ orig_op0 = op0 = TREE_OPERAND (expr, 0);
> > > >+ op1 = TREE_OPERAND (expr, 1);
> > > >+ op2 = TREE_OPERAND (expr, 2);
> > > >+ op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
> > > >+ maybe_const_itself, for_int_const);
> > > >+ STRIP_TYPE_NOPS (op0);
> > > >+
> > > >+ /* OP1 will be a vector of -1 and OP2 a vector if 0, as created in
> > > >+ build_vec_cmp -- no need to fold them. */
> > > Is this worth verifying with a gcc_assert? Your call.
> > >
> > > OK with or without the gcc_assert that op1/op2 are constants.
>
> I'm going to add the asserts. I should've add them in the first place.
>
> > I think either we should have an gcc_checking_assert, or can't
> > we just handle VEC_COND_EXPR like COND_EXPR, with no extra code?
>
> It works (I seem to remember testing that variant), but the COND_EXPR case
> code does a lot of stuff that doesn't seem to make sense for vectors, like
> checking for truthvalue_false_node, folding op1/op2, playing games with
> maybe_const_* stuff... So I decided to add a new case.
Those comparisons with truthvalue_*_node would fail and DTRT.
Or just c_fully_fold_internal all the arguments, be ready for any future
further uses of VEC_COND_EXPR early?
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307)
2016-04-01 16:18 ` Jakub Jelinek
@ 2016-04-01 17:10 ` Marek Polacek
2016-04-01 17:22 ` Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Marek Polacek @ 2016-04-01 17:10 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jeff Law, GCC Patches, Joseph Myers
On Fri, Apr 01, 2016 at 06:17:57PM +0200, Jakub Jelinek wrote:
> Those comparisons with truthvalue_*_node would fail and DTRT.
> Or just c_fully_fold_internal all the arguments, be ready for any future
> further uses of VEC_COND_EXPR early?
..thus revive my earlier version of the patch that does it:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2016-04-01 Marek Polacek <polacek@redhat.com>
PR c/70307
* c-fold.c (c_fully_fold_internal): Handle VEC_COND_EXPR.
* gcc.dg/torture/pr70307.c: New test.
diff --git gcc/c/c-fold.c gcc/c/c-fold.c
index f07917f..6c82f24 100644
--- gcc/c/c-fold.c
+++ gcc/c/c-fold.c
@@ -528,6 +528,26 @@ c_fully_fold_internal (tree expr, bool in_init, bool *maybe_const_operands,
*maybe_const_itself &= op2_const_self;
goto out;
+ case VEC_COND_EXPR:
+ orig_op0 = op0 = TREE_OPERAND (expr, 0);
+ orig_op1 = op1 = TREE_OPERAND (expr, 1);
+ orig_op2 = op2 = TREE_OPERAND (expr, 2);
+ op0 = c_fully_fold_internal (op0, in_init, maybe_const_operands,
+ maybe_const_itself, for_int_const);
+ STRIP_TYPE_NOPS (op0);
+ op1 = c_fully_fold_internal (op1, in_init, maybe_const_operands,
+ maybe_const_itself, for_int_const);
+ STRIP_TYPE_NOPS (op1);
+ op2 = c_fully_fold_internal (op2, in_init, maybe_const_operands,
+ maybe_const_itself, for_int_const);
+ STRIP_TYPE_NOPS (op2);
+
+ if (op0 != orig_op0 || op1 != orig_op1 || op2 != orig_op2)
+ ret = fold_build3_loc (loc, code, TREE_TYPE (expr), op0, op1, op2);
+ else
+ ret = fold (expr);
+ goto out;
+
case EXCESS_PRECISION_EXPR:
/* Each case where an operand with excess precision may be
encountered must remove the EXCESS_PRECISION_EXPR around
diff --git gcc/testsuite/gcc.dg/torture/pr70307.c gcc/testsuite/gcc.dg/torture/pr70307.c
index e69de29..d47c4b6 100644
--- gcc/testsuite/gcc.dg/torture/pr70307.c
+++ gcc/testsuite/gcc.dg/torture/pr70307.c
@@ -0,0 +1,62 @@
+/* PR c/70307 */
+/* { dg-do compile } */
+
+typedef int v4si __attribute__ ((vector_size (16)));
+
+v4si foo (v4si);
+
+v4si
+fn1 (int i)
+{
+ return i <= (v4si){(0, 0)};
+}
+
+v4si
+fn2 (int i)
+{
+ v4si r;
+ r = i <= (v4si){(0, 0)};
+ return r;
+}
+
+v4si
+fn3 (int i)
+{
+ return foo (i <= (v4si){(0, 0)});
+}
+
+v4si
+fn4 (int i)
+{
+ struct S { v4si v; };
+ struct S s = { .v = i <= (v4si){(0, 0)} };
+ return s.v;
+}
+
+v4si
+fn5 (int i)
+{
+ return (v4si){(1, i++)} == (v4si){(0, 0)};
+}
+
+v4si
+fn6 (int i)
+{
+ v4si r;
+ r = (v4si){(1, i++)} == (v4si){(0, 0)};
+ return r;
+}
+
+v4si
+fn7 (int i)
+{
+ return foo ((v4si){(1, i++)} == (v4si){(0, 0)});
+}
+
+v4si
+fn8 (int i)
+{
+ struct S { v4si v; };
+ struct S s = { .v = (v4si){(1, i++)} == (v4si){(0, 0)} };
+ return s.v;
+}
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307)
2016-04-01 17:10 ` Marek Polacek
@ 2016-04-01 17:22 ` Jakub Jelinek
2016-04-01 17:35 ` Marek Polacek
0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2016-04-01 17:22 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jeff Law, GCC Patches, Joseph Myers
On Fri, Apr 01, 2016 at 07:10:06PM +0200, Marek Polacek wrote:
> On Fri, Apr 01, 2016 at 06:17:57PM +0200, Jakub Jelinek wrote:
> > Those comparisons with truthvalue_*_node would fail and DTRT.
> > Or just c_fully_fold_internal all the arguments, be ready for any future
> > further uses of VEC_COND_EXPR early?
>
> ..thus revive my earlier version of the patch that does it:
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-04-01 Marek Polacek <polacek@redhat.com>
>
> PR c/70307
> * c-fold.c (c_fully_fold_internal): Handle VEC_COND_EXPR.
>
> * gcc.dg/torture/pr70307.c: New test.
LGTM, thanks.
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307)
2016-04-01 17:22 ` Jakub Jelinek
@ 2016-04-01 17:35 ` Marek Polacek
2016-04-04 17:23 ` [committed] Fix pr70307.c testcase (PR middle-end/70307) Jakub Jelinek
0 siblings, 1 reply; 9+ messages in thread
From: Marek Polacek @ 2016-04-01 17:35 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jeff Law, GCC Patches, Joseph Myers
On Fri, Apr 01, 2016 at 07:22:24PM +0200, Jakub Jelinek wrote:
> On Fri, Apr 01, 2016 at 07:10:06PM +0200, Marek Polacek wrote:
> > On Fri, Apr 01, 2016 at 06:17:57PM +0200, Jakub Jelinek wrote:
> > > Those comparisons with truthvalue_*_node would fail and DTRT.
> > > Or just c_fully_fold_internal all the arguments, be ready for any future
> > > further uses of VEC_COND_EXPR early?
> >
> > ..thus revive my earlier version of the patch that does it:
> >
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> >
> > 2016-04-01 Marek Polacek <polacek@redhat.com>
> >
> > PR c/70307
> > * c-fold.c (c_fully_fold_internal): Handle VEC_COND_EXPR.
> >
> > * gcc.dg/torture/pr70307.c: New test.
>
> LGTM, thanks.
Thanks. I'll wait till tomorrow to see if Joseph has any comments on this
patch.
Marek
^ permalink raw reply [flat|nested] 9+ messages in thread
* [committed] Fix pr70307.c testcase (PR middle-end/70307)
2016-04-01 17:35 ` Marek Polacek
@ 2016-04-04 17:23 ` Jakub Jelinek
0 siblings, 0 replies; 9+ messages in thread
From: Jakub Jelinek @ 2016-04-04 17:23 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jeff Law, GCC Patches, Joseph Myers
Hi!
I've noticed the new testcase FAILs on i?86-linux and powerpc-linux.
As this is a compile time testcase rather than runtime, I chose to prune
the rs6000 ABI warnings instead of using -w which is otherwise the only way
to disable them :(.
Fixed thusly, committed to trunk.
2016-04-04 Jakub Jelinek <jakub@redhat.com>
PR middle-end/70307
* gcc.dg/torture/pr70307.c: Add -Wno-psabi to dg-options. Prune
rs6000 ABI warnings.
--- gcc/testsuite/gcc.dg/torture/pr70307.c.jj 2016-04-04 12:28:39.000000000 +0200
+++ gcc/testsuite/gcc.dg/torture/pr70307.c 2016-04-04 19:17:13.103432985 +0200
@@ -1,5 +1,6 @@
/* PR c/70307 */
/* { dg-do compile } */
+/* { dg-options "-Wno-psabi" } */
typedef int v4si __attribute__ ((vector_size (16)));
@@ -60,3 +61,7 @@ fn8 (int i)
struct S s = { .v = (v4si){(1, i++)} == (v4si){(0, 0)} };
return s.v;
}
+
+/* Ignore a warning that is irrelevant to the purpose of this test. */
+/* { dg-prune-output "\[^\n\r\]*GCC vector passed by reference\[^\n\r\]*" } */
+/* { dg-prune-output "\[^\n\r\]*GCC vector returned by reference\[^\n\r\]*" } */
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-04-04 17:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-01 15:03 [C PATCH] Fix ICE with VEC_COND_EXPR and compound literals (PR c/70307) Marek Polacek
2016-04-01 15:54 ` Jeff Law
2016-04-01 16:02 ` Jakub Jelinek
2016-04-01 16:14 ` Marek Polacek
2016-04-01 16:18 ` Jakub Jelinek
2016-04-01 17:10 ` Marek Polacek
2016-04-01 17:22 ` Jakub Jelinek
2016-04-01 17:35 ` Marek Polacek
2016-04-04 17:23 ` [committed] Fix pr70307.c testcase (PR middle-end/70307) Jakub Jelinek
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).