public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] varasm: Fix up ICE in narrowing_initializer_constant_valid_p [PR105998]
@ 2022-06-17  8:10 Jakub Jelinek
  2022-06-17  8:37 ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-06-17  8:10 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi!

The following testcase ICEs because there is NON_LVALUE_EXPR (location
wrapper) around a VAR_DECL and has TYPE_MODE V2SImode and
SCALAR_INT_TYPE_MODE on that ICEs.  Or for -m32 -march=i386 TYPE_MODE
is DImode, but SCALAR_INT_TYPE_MODE still uses the raw V2SImode and ICEs
too.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok
for trunk?

2022-06-17  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/105998
	* varasm.cc (narrowing_initializer_constant_valid_p): Check
	SCALAR_INT_MODE_P instead of INTEGRAL_MODE_P, also break on
	VECTOR_TYPE_P.

	* c-c++-common/pr105998.c: New test.

--- gcc/varasm.cc.jj	2022-06-06 12:18:12.792812888 +0200
+++ gcc/varasm.cc	2022-06-17 09:49:21.918029072 +0200
@@ -4716,7 +4716,8 @@ narrowing_initializer_constant_valid_p (
     {
       tree inner = TREE_OPERAND (op0, 0);
       if (inner == error_mark_node
-	  || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
+	  || VECTOR_TYPE_P (TREE_TYPE (inner))
+	  || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
 	  || (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (op0)))
 	      > GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (inner)))))
 	break;
@@ -4728,7 +4729,8 @@ narrowing_initializer_constant_valid_p (
     {
       tree inner = TREE_OPERAND (op1, 0);
       if (inner == error_mark_node
-	  || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
+	  || VECTOR_TYPE_P (TREE_TYPE (inner))
+	  || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
 	  || (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (op1)))
 	      > GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (inner)))))
 	break;
--- gcc/testsuite/c-c++-common/pr105998.c.jj	2022-06-16 16:37:00.094926684 +0200
+++ gcc/testsuite/c-c++-common/pr105998.c	2022-06-16 16:36:30.155322215 +0200
@@ -0,0 +1,12 @@
+/* PR middle-end/105998 */
+
+typedef int __attribute__((__vector_size__ (sizeof (long long)))) V;
+
+V v;
+
+long long
+foo (void)
+{
+  long long l = (long long) ((0 | v) - ((V) { } == 0));
+  return l;
+}

	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] varasm: Fix up ICE in narrowing_initializer_constant_valid_p [PR105998]
  2022-06-17  8:10 [PATCH] varasm: Fix up ICE in narrowing_initializer_constant_valid_p [PR105998] Jakub Jelinek
@ 2022-06-17  8:37 ` Richard Biener
  2022-06-17  9:19   ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2022-06-17  8:37 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches



> Am 17.06.2022 um 10:11 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> Hi!
> 
> The following testcase ICEs because there is NON_LVALUE_EXPR (location
> wrapper) around a VAR_DECL and has TYPE_MODE V2SImode and
> SCALAR_INT_TYPE_MODE on that ICEs.  Or for -m32 -march=i386 TYPE_MODE
> is DImode, but SCALAR_INT_TYPE_MODE still uses the raw V2SImode and ICEs
> too.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok
> for trunk?
> 
> 2022-06-17  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR middle-end/105998
>    * varasm.cc (narrowing_initializer_constant_valid_p): Check
>    SCALAR_INT_MODE_P instead of INTEGRAL_MODE_P, also break on
>    VECTOR_TYPE_P.
> 
>    * c-c++-common/pr105998.c: New test.
> 
> --- gcc/varasm.cc.jj    2022-06-06 12:18:12.792812888 +0200
> +++ gcc/varasm.cc    2022-06-17 09:49:21.918029072 +0200
> @@ -4716,7 +4716,8 @@ narrowing_initializer_constant_valid_p (
>     {
>       tree inner = TREE_OPERAND (op0, 0);
>       if (inner == error_mark_node
> -      || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
> +      || VECTOR_TYPE_P (TREE_TYPE (inner))

Do we really want to allow all integer modes here regardless of a composite type (record type for example)?  I’d say !INTEGRAL_TYPE_P would match the rest better.  OTOH if we want to allow integer modes I fail to see why to exclude vector types (but not complex, etc)

> +      || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
>      || (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (op0)))
>          > GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (inner)))))
>    break;
> @@ -4728,7 +4729,8 @@ narrowing_initializer_constant_valid_p (
>     {
>       tree inner = TREE_OPERAND (op1, 0);
>       if (inner == error_mark_node
> -      || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
> +      || VECTOR_TYPE_P (TREE_TYPE (inner))
> +      || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
>      || (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (op1)))
>          > GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (inner)))))
>    break;
> --- gcc/testsuite/c-c++-common/pr105998.c.jj    2022-06-16 16:37:00.094926684 +0200
> +++ gcc/testsuite/c-c++-common/pr105998.c    2022-06-16 16:36:30.155322215 +0200
> @@ -0,0 +1,12 @@
> +/* PR middle-end/105998 */
> +
> +typedef int __attribute__((__vector_size__ (sizeof (long long)))) V;
> +
> +V v;
> +
> +long long
> +foo (void)
> +{
> +  long long l = (long long) ((0 | v) - ((V) { } == 0));
> +  return l;
> +}
> 
>    Jakub
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] varasm: Fix up ICE in narrowing_initializer_constant_valid_p [PR105998]
  2022-06-17  8:37 ` Richard Biener
@ 2022-06-17  9:19   ` Jakub Jelinek
  2022-06-17 15:22     ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-06-17  9:19 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On Fri, Jun 17, 2022 at 10:37:45AM +0200, Richard Biener wrote:
> > --- gcc/varasm.cc.jj    2022-06-06 12:18:12.792812888 +0200
> > +++ gcc/varasm.cc    2022-06-17 09:49:21.918029072 +0200
> > @@ -4716,7 +4716,8 @@ narrowing_initializer_constant_valid_p (
> >     {
> >       tree inner = TREE_OPERAND (op0, 0);
> >       if (inner == error_mark_node
> > -      || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
> > +      || VECTOR_TYPE_P (TREE_TYPE (inner))
> 
> Do we really want to allow all integer modes here regardless of a
> composite type (record type for example)?  I’d say !INTEGRAL_TYPE_P would
> match the rest better.  OTOH if we want to allow integer modes I fail to
> see why to exclude vector types (but not complex, etc)

I've excluded VECTOR_TYPE_P because those are the only types for which
TYPE_MODE can be different from the raw type mode (so, SCALAR_INT_MODE_P
was true but SCALAR_INT_TYPE_MODE still ICEd).

Checking for INTEGRAL_TYPE_P seems reasonable to me though,
and I'd say we also want to check the outer type too because nothing
really checks it (at least for the first iteration, 2nd and further
get it from checking of inner in the previous iteration).

So like this if it passes bootstrap/regtest?

2022-06-17  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/105998
	* varasm.cc (narrowing_initializer_constant_valid_p): Check
	SCALAR_INT_MODE_P instead of INTEGRAL_MODE_P, also break on
	! INTEGRAL_TYPE_P and do the same check also on op{0,1}'s type.

	* c-c++-common/pr105998.c: New test.

--- gcc/varasm.cc.jj	2022-06-17 11:07:57.883679019 +0200
+++ gcc/varasm.cc	2022-06-17 11:10:09.190932417 +0200
@@ -4716,7 +4716,10 @@ narrowing_initializer_constant_valid_p (
     {
       tree inner = TREE_OPERAND (op0, 0);
       if (inner == error_mark_node
-	  || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
+	  || ! INTEGRAL_TYPE_P (TREE_TYPE (op0))
+	  || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (op0)))
+	  || ! INTEGRAL_TYPE_P (TREE_TYPE (inner))
+	  || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
 	  || (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (op0)))
 	      > GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (inner)))))
 	break;
@@ -4728,7 +4731,10 @@ narrowing_initializer_constant_valid_p (
     {
       tree inner = TREE_OPERAND (op1, 0);
       if (inner == error_mark_node
-	  || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
+	  || ! INTEGRAL_TYPE_P (TREE_TYPE (op1))
+	  || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (op1)))
+	  || ! INTEGRAL_TYPE_P (TREE_TYPE (inner))
+	  || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
 	  || (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (op1)))
 	      > GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (inner)))))
 	break;
--- gcc/testsuite/c-c++-common/pr105998.c.jj	2022-06-17 11:09:11.196703834 +0200
+++ gcc/testsuite/c-c++-common/pr105998.c	2022-06-17 11:09:11.196703834 +0200
@@ -0,0 +1,12 @@
+/* PR middle-end/105998 */
+
+typedef int __attribute__((__vector_size__ (sizeof (long long)))) V;
+
+V v;
+
+long long
+foo (void)
+{
+  long long l = (long long) ((0 | v) - ((V) { } == 0));
+  return l;
+}


	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] varasm: Fix up ICE in narrowing_initializer_constant_valid_p [PR105998]
  2022-06-17  9:19   ` Jakub Jelinek
@ 2022-06-17 15:22     ` Richard Biener
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2022-06-17 15:22 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches



> Am 17.06.2022 um 11:20 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> On Fri, Jun 17, 2022 at 10:37:45AM +0200, Richard Biener wrote:
>>> --- gcc/varasm.cc.jj    2022-06-06 12:18:12.792812888 +0200
>>> +++ gcc/varasm.cc    2022-06-17 09:49:21.918029072 +0200
>>> @@ -4716,7 +4716,8 @@ narrowing_initializer_constant_valid_p (
>>>    {
>>>      tree inner = TREE_OPERAND (op0, 0);
>>>      if (inner == error_mark_node
>>> -      || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
>>> +      || VECTOR_TYPE_P (TREE_TYPE (inner))
>> 
>> Do we really want to allow all integer modes here regardless of a
>> composite type (record type for example)?  I’d say !INTEGRAL_TYPE_P would
>> match the rest better.  OTOH if we want to allow integer modes I fail to
>> see why to exclude vector types (but not complex, etc)
> 
> I've excluded VECTOR_TYPE_P because those are the only types for which
> TYPE_MODE can be different from the raw type mode (so, SCALAR_INT_MODE_P
> was true but SCALAR_INT_TYPE_MODE still ICEd).
> 
> Checking for INTEGRAL_TYPE_P seems reasonable to me though,
> and I'd say we also want to check the outer type too because nothing
> really checks it (at least for the first iteration, 2nd and further
> get it from checking of inner in the previous iteration).
> 
> So like this if it passes bootstrap/regtest?

Ok.

Richard 
> 2022-06-17  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR middle-end/105998
>    * varasm.cc (narrowing_initializer_constant_valid_p): Check
>    SCALAR_INT_MODE_P instead of INTEGRAL_MODE_P, also break on
>    ! INTEGRAL_TYPE_P and do the same check also on op{0,1}'s type.
> 
>    * c-c++-common/pr105998.c: New test.
> 
> --- gcc/varasm.cc.jj    2022-06-17 11:07:57.883679019 +0200
> +++ gcc/varasm.cc    2022-06-17 11:10:09.190932417 +0200
> @@ -4716,7 +4716,10 @@ narrowing_initializer_constant_valid_p (
>     {
>       tree inner = TREE_OPERAND (op0, 0);
>       if (inner == error_mark_node
> -      || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
> +      || ! INTEGRAL_TYPE_P (TREE_TYPE (op0))
> +      || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (op0)))
> +      || ! INTEGRAL_TYPE_P (TREE_TYPE (inner))
> +      || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
>      || (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (op0)))
>          > GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (inner)))))
>    break;
> @@ -4728,7 +4731,10 @@ narrowing_initializer_constant_valid_p (
>     {
>       tree inner = TREE_OPERAND (op1, 0);
>       if (inner == error_mark_node
> -      || ! INTEGRAL_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
> +      || ! INTEGRAL_TYPE_P (TREE_TYPE (op1))
> +      || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (op1)))
> +      || ! INTEGRAL_TYPE_P (TREE_TYPE (inner))
> +      || ! SCALAR_INT_MODE_P (TYPE_MODE (TREE_TYPE (inner)))
>      || (GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (op1)))
>          > GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (TREE_TYPE (inner)))))
>    break;
> --- gcc/testsuite/c-c++-common/pr105998.c.jj    2022-06-17 11:09:11.196703834 +0200
> +++ gcc/testsuite/c-c++-common/pr105998.c    2022-06-17 11:09:11.196703834 +0200
> @@ -0,0 +1,12 @@
> +/* PR middle-end/105998 */
> +
> +typedef int __attribute__((__vector_size__ (sizeof (long long)))) V;
> +
> +V v;
> +
> +long long
> +foo (void)
> +{
> +  long long l = (long long) ((0 | v) - ((V) { } == 0));
> +  return l;
> +}
> 
> 
>    Jakub
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-06-17 15:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-17  8:10 [PATCH] varasm: Fix up ICE in narrowing_initializer_constant_valid_p [PR105998] Jakub Jelinek
2022-06-17  8:37 ` Richard Biener
2022-06-17  9:19   ` Jakub Jelinek
2022-06-17 15:22     ` Richard Biener

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).