diff mbox

Fix ICE with vector types in X % -Y pattern (PR middle-end/70050)

Message ID 20160303110431.GB10006@redhat.com
State New
Headers show

Commit Message

Marek Polacek March 3, 2016, 11:04 a.m. UTC
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.

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.


	Marek

Comments

Jakub Jelinek March 3, 2016, 11:34 a.m. UTC | #1
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
Marc Glisse March 3, 2016, 12:56 p.m. UTC | #2
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;
> +}
Jakub Jelinek March 3, 2016, 12:58 p.m. UTC | #3
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
Marek Polacek March 3, 2016, 12:59 p.m. UTC | #4
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
Andreas Schwab March 8, 2016, 10:16 p.m. UTC | #5
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 mbox

Patch

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;
+}