diff mbox series

varasm: Fix up ICE in narrowing_initializer_constant_valid_p [PR105998]

Message ID Yqw3Eeqv3q/7HR3s@tucnak
State New
Headers
Series varasm: Fix up ICE in narrowing_initializer_constant_valid_p [PR105998] |

Commit Message

Jakub Jelinek June 17, 2022, 8:10 a.m. UTC 
  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.


	Jakub

Comments

Richard Biener June 17, 2022, 8:37 a.m. UTC | #1 
> 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
>
Jakub Jelinek June 17, 2022, 9:19 a.m. UTC | #2 
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
Richard Biener June 17, 2022, 3:22 p.m. UTC | #3 
> 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
>
diff mbox series

Patch

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