diff mbox series

[committed] Punt instead of assertion failure when TYPE_MODE is not vmode (PR tree-optimization/92557)

Message ID 20191119085737.GC4650@tucnak
State New
Headers show
Series [committed] Punt instead of assertion failure when TYPE_MODE is not vmode (PR tree-optimization/92557) | expand

Commit Message

Jakub Jelinek Nov. 19, 2019, 8:57 a.m. UTC 
Hi!

This patch restores the previous behavior, there could be many reasons why
TYPE_MODE is BLKmode or some integral mode instead of a vector mode,
unsupported vector mode, lack of available registers etc.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2019-11-19  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/92557
	* omp-low.c (omp_clause_aligned_alignment): Punt if TYPE_MODE is not
	vmode rather than asserting it always is.

	* gcc.dg/gomp/pr92557.c: New test.


	Jakub

Comments

Richard Sandiford Nov. 19, 2019, 12:58 p.m. UTC | #1 
Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> This patch restores the previous behavior, there could be many reasons why
> TYPE_MODE is BLKmode or some integral mode instead of a vector mode,
> unsupported vector mode, lack of available registers etc.

Fow the record, I think the assert was valid and I'd have preferred it
if we'd fixed the targets so that they don't return bogus modes for
preferred_simd_mode.  After all, it's the targets that decide which
vector modes are supported and what can go in registers.

So IMO the assert simply exposed a genuine target bug.  I don't strongly
object though.

Thanks,
Richard

> Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
>
> 2019-11-19  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/92557
> 	* omp-low.c (omp_clause_aligned_alignment): Punt if TYPE_MODE is not
> 	vmode rather than asserting it always is.
>
> 	* gcc.dg/gomp/pr92557.c: New test.
>
> --- gcc/omp-low.c.jj	2019-11-15 00:37:26.359069152 +0100
> +++ gcc/omp-low.c	2019-11-18 16:17:27.767714777 +0100
> @@ -4086,8 +4086,8 @@ omp_clause_aligned_alignment (tree claus
>  	if (type == NULL_TREE || TYPE_MODE (type) != mode)
>  	  continue;
>  	type = build_vector_type_for_mode (type, vmode);
> -	/* The functions above are not allowed to return invalid modes.  */
> -	gcc_assert (TYPE_MODE (type) == vmode);
> +	if (TYPE_MODE (type) != vmode)
> +	  continue;
>  	if (TYPE_ALIGN_UNIT (type) > al)
>  	  al = TYPE_ALIGN_UNIT (type);
>        }
> --- gcc/testsuite/gcc.dg/gomp/pr92557.c.jj	2019-11-18 16:18:18.246961685 +0100
> +++ gcc/testsuite/gcc.dg/gomp/pr92557.c	2019-11-18 16:19:21.531017555 +0100
> @@ -0,0 +1,13 @@
> +/* PR tree-optimization/92557 */
> +/* { dg-do compile } */
> +/* { dg-additional-options "-maltivec" { target powerpc*-*-* } } */
> +
> +void
> +foo (double *p)
> +{
> +  int i;
> +
> +#pragma omp simd aligned (p)
> +  for (i = 0; i < 1; ++i)
> +    p[i] = 7.0;
> +}
>
> 	Jakub
Jakub Jelinek Nov. 19, 2019, 1:11 p.m. UTC | #2 
On Tue, Nov 19, 2019 at 12:58:19PM +0000, Richard Sandiford wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> > This patch restores the previous behavior, there could be many reasons why
> > TYPE_MODE is BLKmode or some integral mode instead of a vector mode,
> > unsupported vector mode, lack of available registers etc.
> 
> Fow the record, I think the assert was valid and I'd have preferred it
> if we'd fixed the targets so that they don't return bogus modes for
> preferred_simd_mode.  After all, it's the targets that decide which
> vector modes are supported and what can go in registers.
> 
> So IMO the assert simply exposed a genuine target bug.  I don't strongly
> object though.

I don't object if it is fixed in the targets, I just don't think
omp_clause_aligned_alignment is the place where such target bugs should be
diagnosed.  If we want to enforce it, we should enforce it in some self-test
(though, that wouldn't cover all possible ISA combinations), or in some
checking function somewhere where it is always triggered.
The vectorizer apparently doesn't enforce that.

	Jakub
Segher Boessenkool Nov. 19, 2019, 2:54 p.m. UTC | #3 
On Tue, Nov 19, 2019 at 12:58:19PM +0000, Richard Sandiford wrote:
> Fow the record, I think the assert was valid and I'd have preferred it
> if we'd fixed the targets so that they don't return bogus modes for
> preferred_simd_mode.  After all, it's the targets that decide which
> vector modes are supported and what can go in registers.
> 
> So IMO the assert simply exposed a genuine target bug.  I don't strongly
> object though.

Why is this a bug?  Saying V2DI is the preferred vector mode for DI does
not imply that we can actually put V2DI in registers, does it?  It isn't
documented like that, in any case.

(Of course it is somewhat silly to return a mode that cannot be used, and
it is likely the target can do better.)


Segher
diff mbox series

Patch

--- gcc/omp-low.c.jj	2019-11-15 00:37:26.359069152 +0100
+++ gcc/omp-low.c	2019-11-18 16:17:27.767714777 +0100
@@ -4086,8 +4086,8 @@  omp_clause_aligned_alignment (tree claus
 	if (type == NULL_TREE || TYPE_MODE (type) != mode)
 	  continue;
 	type = build_vector_type_for_mode (type, vmode);
-	/* The functions above are not allowed to return invalid modes.  */
-	gcc_assert (TYPE_MODE (type) == vmode);
+	if (TYPE_MODE (type) != vmode)
+	  continue;
 	if (TYPE_ALIGN_UNIT (type) > al)
 	  al = TYPE_ALIGN_UNIT (type);
       }
--- gcc/testsuite/gcc.dg/gomp/pr92557.c.jj	2019-11-18 16:18:18.246961685 +0100
+++ gcc/testsuite/gcc.dg/gomp/pr92557.c	2019-11-18 16:19:21.531017555 +0100
@@ -0,0 +1,13 @@ 
+/* PR tree-optimization/92557 */
+/* { dg-do compile } */
+/* { dg-additional-options "-maltivec" { target powerpc*-*-* } } */
+
+void
+foo (double *p)
+{
+  int i;
+
+#pragma omp simd aligned (p)
+  for (i = 0; i < 1; ++i)
+    p[i] = 7.0;
+}