Message ID | 20160303110431.GB10006@redhat.com |
---|---|
State | New |
Headers | show |
On Thu, Mar 03, 2016 at 12:04:31PM +0100, Marek Polacek wrote: > We crashed on the attached testcase because we were trying to apply the > X % -Y -> X % Y transformation even on vectors. That doesn't go well with the > check for !TYPE_UNSIGNED. So I think let's limit the pattern to only work on > integral types. I think TYPE_UNSIGNED is fine, but TYPE_MIN_VALUE is not. > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2016-03-03 Marek Polacek <polacek@redhat.com> > > PR middle-end/70050 > * match.pd (X % -Y): Add INTEGRAL_TYPE_P check. > > * gcc.dg/pr70050.c: New test. Ok, thanks. Jakub
On Thu, 3 Mar 2016, Marek Polacek wrote: > We crashed on the attached testcase because we were trying to apply the > X % -Y -> X % Y transformation even on vectors. That doesn't go well with the > check for !TYPE_UNSIGNED. So I think let's limit the pattern to only work on > integral types. I certainly hope the problem is not with TYPE_UNSIGNED, I think there are many places where we use it on vectors. I would expect the issue to be with TYPE_MIN_VALUE a few lines below. Not that it changes anything to the fix... (we can revisit that if vectors ever get VRP support) > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2016-03-03 Marek Polacek <polacek@redhat.com> > > PR middle-end/70050 > * match.pd (X % -Y): Add INTEGRAL_TYPE_P check. > > * gcc.dg/pr70050.c: New test. > > diff --git gcc/match.pd gcc/match.pd > index 5903782..112deb3 100644 > --- gcc/match.pd > +++ gcc/match.pd > @@ -293,7 +293,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > /* X % -Y is the same as X % Y. */ > (simplify > (trunc_mod @0 (convert? (negate @1))) > - (if (!TYPE_UNSIGNED (type) > + (if (INTEGRAL_TYPE_P (type) > + && !TYPE_UNSIGNED (type) > && !TYPE_OVERFLOW_TRAPS (type) > && tree_nop_conversion_p (type, TREE_TYPE (@1)) > /* Avoid this transformation if X might be INT_MIN or > diff --git gcc/testsuite/gcc.dg/pr70050.c gcc/testsuite/gcc.dg/pr70050.c > index e69de29..610456f 100644 > --- gcc/testsuite/gcc.dg/pr70050.c > +++ gcc/testsuite/gcc.dg/pr70050.c > @@ -0,0 +1,11 @@ > +/* PR middle-end/70025 */ > +/* { dg-do compile } */ > +/* { dg-options "-Wno-psabi" } */ > + > +typedef int v8si __attribute__ ((vector_size (32))); > + > +v8si > +foo (v8si v) > +{ > + return v %= -v; > +}
On Thu, Mar 03, 2016 at 01:56:05PM +0100, Marc Glisse wrote: > On Thu, 3 Mar 2016, Marek Polacek wrote: > > >We crashed on the attached testcase because we were trying to apply the > >X % -Y -> X % Y transformation even on vectors. That doesn't go well with the > >check for !TYPE_UNSIGNED. So I think let's limit the pattern to only work on > >integral types. > > I certainly hope the problem is not with TYPE_UNSIGNED, I think there are > many places where we use it on vectors. I would expect the issue to be with > TYPE_MIN_VALUE a few lines below. Not that it changes anything to the fix... > > (we can revisit that if vectors ever get VRP support) And expr_not_equal_to would need to be tought to use that... Jakub
On Thu, Mar 03, 2016 at 01:56:05PM +0100, Marc Glisse wrote: > On Thu, 3 Mar 2016, Marek Polacek wrote: > > >We crashed on the attached testcase because we were trying to apply the > >X % -Y -> X % Y transformation even on vectors. That doesn't go well with the > >check for !TYPE_UNSIGNED. So I think let's limit the pattern to only work on > >integral types. > > I certainly hope the problem is not with TYPE_UNSIGNED, I think there are > many places where we use it on vectors. I would expect the issue to be with > TYPE_MIN_VALUE a few lines below. Not that it changes anything to the fix... Yes, as Jakub already pointed out. My bad, sorry about that. Marek
Marek Polacek <polacek@redhat.com> writes: > diff --git gcc/testsuite/gcc.dg/pr70050.c gcc/testsuite/gcc.dg/pr70050.c > index e69de29..610456f 100644 > --- gcc/testsuite/gcc.dg/pr70050.c > +++ gcc/testsuite/gcc.dg/pr70050.c > @@ -0,0 +1,11 @@ > +/* PR middle-end/70025 */ > +/* { dg-do compile } */ > +/* { dg-options "-Wno-psabi" } */ > + > +typedef int v8si __attribute__ ((vector_size (32))); > + > +v8si > +foo (v8si v) On powerpc: FAIL: gcc.dg/pr70050.c (test for excess errors) Excess errors: /daten/gcc/gcc-20160307/gcc/testsuite/gcc.dg/pr70050.c:9:1: warning: GCC vector returned by reference: non-standard ABI extension with no compatibility guarantee /daten/gcc/gcc-20160307/gcc/testsuite/gcc.dg/pr70050.c:8:1: warning: GCC vector passed by reference: non-standard ABI extension with no compatibility guarantee Andreas.
diff --git gcc/match.pd gcc/match.pd index 5903782..112deb3 100644 --- gcc/match.pd +++ gcc/match.pd @@ -293,7 +293,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* X % -Y is the same as X % Y. */ (simplify (trunc_mod @0 (convert? (negate @1))) - (if (!TYPE_UNSIGNED (type) + (if (INTEGRAL_TYPE_P (type) + && !TYPE_UNSIGNED (type) && !TYPE_OVERFLOW_TRAPS (type) && tree_nop_conversion_p (type, TREE_TYPE (@1)) /* Avoid this transformation if X might be INT_MIN or diff --git gcc/testsuite/gcc.dg/pr70050.c gcc/testsuite/gcc.dg/pr70050.c index e69de29..610456f 100644 --- gcc/testsuite/gcc.dg/pr70050.c +++ gcc/testsuite/gcc.dg/pr70050.c @@ -0,0 +1,11 @@ +/* PR middle-end/70025 */ +/* { dg-do compile } */ +/* { dg-options "-Wno-psabi" } */ + +typedef int v8si __attribute__ ((vector_size (32))); + +v8si +foo (v8si v) +{ + return v %= -v; +}