[PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

simplify_subreg can return VOIDmode const_int operand and will
cause ICE in simplify_gen_subreg when this operand is passed to it.

The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
We can't process const_int operand any further, since outermode
is not an integer mode here.

2023-02-16  Uroš Bizjak  <ubizjak@gmail.com>

gcc/ChangeLog:

    PR target/108805
    * simplify_rtx.cc (simplify_context::simplify_subreg): Prevent
    VOIDmode const_int result from simplify_subreg from entering
    simplify_gen_subreg.

gcc/testsuite/ChangeLog:

    PR target/108805
    * gcc.dg/pr108805.c: New test.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

OK for master and release branches?

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 1174 bytes --]

diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index 0a1dd88b0a8..281bc418df0 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -7664,7 +7664,7 @@ simplify_context::simplify_subreg (machine_mode outermode, rtx op,
 			    0).exists (&int_outermode))
     {
       rtx tem = simplify_subreg (int_outermode, op, innermode, byte);
-      if (tem)
+      if (tem && GET_MODE (tem) != VOIDmode)
 	return simplify_gen_subreg (outermode, tem, GET_MODE (tem), 0);
     }
 
diff --git a/gcc/testsuite/gcc.dg/pr108805.c b/gcc/testsuite/gcc.dg/pr108805.c
new file mode 100644
index 00000000000..280d3f5c377
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr108805.c
@@ -0,0 +1,20 @@
+/* { dg-do compile { target longlong64 } } */
+/* { dg-options "-O" } */
+/* { dg-additional-options "-msse2" { target x86_64-*-* i?86-*-* } } */
+
+typedef __INT8_TYPE__ __attribute__((__vector_size__ (4))) U;
+typedef __INT32_TYPE__ __attribute__((__vector_size__ (4))) V;
+typedef __UINT64_TYPE__ __attribute__((__vector_size__ (8))) W;
+
+int i;
+U h;
+W g;
+
+U
+foo (void)
+{
+  W w = i != g;
+  V v = __builtin_convertvector (i | w >> 2, V);
+  U u = (U) v[0] + h;
+  return u;
+}

Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 1292 bytes --]

On Thu, 16 Feb 2023, Uros Bizjak wrote:

> simplify_subreg can return VOIDmode const_int operand and will
> cause ICE in simplify_gen_subreg when this operand is passed to it.
> 
> The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
> We can't process const_int operand any further, since outermode
> is not an integer mode here.

But if it's a CONST_INT then we know it's of int_outermode, no? That is,
doesn't simplify_subreg (mode, ...) always return something in 'mode'
and thus we can always pass just 'mode' as third argument to the
following simplify_gen_subreg call?

Richard.

> 2023-02-16  Uroš Bizjak  <ubizjak@gmail.com>
> 
> gcc/ChangeLog:
> 
>     PR target/108805
>     * simplify_rtx.cc (simplify_context::simplify_subreg): Prevent
>     VOIDmode const_int result from simplify_subreg from entering
>     simplify_gen_subreg.
> 
> gcc/testsuite/ChangeLog:
> 
>     PR target/108805
>     * gcc.dg/pr108805.c: New test.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> 
> OK for master and release branches?
> 
> Uros.
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On Fri, Feb 17, 2023 at 8:38 AM Richard Biener <rguenther@suse.de> wrote:
>
> On Thu, 16 Feb 2023, Uros Bizjak wrote:
>
> > simplify_subreg can return VOIDmode const_int operand and will
> > cause ICE in simplify_gen_subreg when this operand is passed to it.
> >
> > The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
> > We can't process const_int operand any further, since outermode
> > is not an integer mode here.
>
> But if it's a CONST_INT then we know it's of int_outermode, no? That is,
> doesn't simplify_subreg (mode, ...) always return something in 'mode'
> and thus we can always pass just 'mode' as third argument to the
> following simplify_gen_subreg call?

You are right. I am testing the attached patch that works too.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 538 bytes --]

diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index 0a1dd88b0a8..3955929bb70 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -7665,7 +7665,7 @@ simplify_context::simplify_subreg (machine_mode outermode, rtx op,
     {
       rtx tem = simplify_subreg (int_outermode, op, innermode, byte);
       if (tem)
-	return simplify_gen_subreg (outermode, tem, GET_MODE (tem), 0);
+	return simplify_gen_subreg (outermode, tem, int_outermode, 0);
     }
 
   /* If OP is a vector comparison and the subreg is not changing the

Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Richard Biener]

On Fri, 17 Feb 2023, Uros Bizjak wrote:

> On Fri, Feb 17, 2023 at 8:38 AM Richard Biener <rguenther@suse.de> wrote:
> >
> > On Thu, 16 Feb 2023, Uros Bizjak wrote:
> >
> > > simplify_subreg can return VOIDmode const_int operand and will
> > > cause ICE in simplify_gen_subreg when this operand is passed to it.
> > >
> > > The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
> > > We can't process const_int operand any further, since outermode
> > > is not an integer mode here.
> >
> > But if it's a CONST_INT then we know it's of int_outermode, no? That is,
> > doesn't simplify_subreg (mode, ...) always return something in 'mode'
> > and thus we can always pass just 'mode' as third argument to the
> > following simplify_gen_subreg call?
> 
> You are right. I am testing the attached patch that works too.

OK if that works.

Thanks,
Richard.

Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Uros Bizjak]

On Fri, Feb 17, 2023 at 12:31 PM Richard Biener <rguenther@suse.de> wrote:
>
> On Fri, 17 Feb 2023, Uros Bizjak wrote:
>
> > On Fri, Feb 17, 2023 at 8:38 AM Richard Biener <rguenther@suse.de> wrote:
> > >
> > > On Thu, 16 Feb 2023, Uros Bizjak wrote:
> > >
> > > > simplify_subreg can return VOIDmode const_int operand and will
> > > > cause ICE in simplify_gen_subreg when this operand is passed to it.
> > > >
> > > > The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
> > > > We can't process const_int operand any further, since outermode
> > > > is not an integer mode here.
> > >
> > > But if it's a CONST_INT then we know it's of int_outermode, no? That is,
> > > doesn't simplify_subreg (mode, ...) always return something in 'mode'
> > > and thus we can always pass just 'mode' as third argument to the
> > > following simplify_gen_subreg call?
> >
> > You are right. I am testing the attached patch that works too.
>
> OK if that works.

Thanks, pushed with the following ChangeLog:

simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

simplify_subreg can return VOIDmode const_int operand and will
cause ICE in simplify_gen_subreg when this operand is passed to it.

The patch uses int_outermode instead of GET_MODE of temporary as the
innermode argument of simplify_gen_subreg.

2023-02-17  Uroš Bizjak  <ubizjak@gmail.com>

gcc/ChangeLog:

    PR target/108805
    * simplify_rtx.cc (simplify_context::simplify_subreg): Use
    int_outermode instead of GET_MODE (tem) to prevent
    VOIDmode from entering simplify_gen_subreg.

Uros.


gcc/testsuite/ChangeLog:

    PR target/108805
    * gcc.dg/pr108805.c: New test.

Uros.

Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Richard Sandiford]

Uros Bizjak <ubizjak@gmail.com> writes:
> On Fri, Feb 17, 2023 at 8:38 AM Richard Biener <rguenther@suse.de> wrote:
>>
>> On Thu, 16 Feb 2023, Uros Bizjak wrote:
>>
>> > simplify_subreg can return VOIDmode const_int operand and will
>> > cause ICE in simplify_gen_subreg when this operand is passed to it.
>> >
>> > The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
>> > We can't process const_int operand any further, since outermode
>> > is not an integer mode here.
>>
>> But if it's a CONST_INT then we know it's of int_outermode, no? That is,
>> doesn't simplify_subreg (mode, ...) always return something in 'mode'
>> and thus we can always pass just 'mode' as third argument to the
>> following simplify_gen_subreg call?
>
> You are right. I am testing the attached patch that works too.

Thanks for this, it's the correct fix.  But as noted in
https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610920.html,
the final 0 is also wrong for big-endian.  Andre?

Richard

>
> Uros.
>
> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
> index 0a1dd88b0a8..3955929bb70 100644
> --- a/gcc/simplify-rtx.cc
> +++ b/gcc/simplify-rtx.cc
> @@ -7665,7 +7665,7 @@ simplify_context::simplify_subreg (machine_mode outermode, rtx op,
>      {
>        rtx tem = simplify_subreg (int_outermode, op, innermode, byte);
>        if (tem)
> -	return simplify_gen_subreg (outermode, tem, GET_MODE (tem), 0);
> +	return simplify_gen_subreg (outermode, tem, int_outermode, 0);
>      }
>  
>    /* If OP is a vector comparison and the subreg is not changing the

Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Andre Vieira (lists)]

Hey both,

Sorry about that, don't know how I missed those. Just running a test on 
that now and will commit when it's done. I assume the comment and 0 -> 
byte change can be seen as obvious, especially since it was supposed to 
be in my original patch...

On 27/02/2023 15:46, Richard Sandiford wrote:
> Uros Bizjak <ubizjak@gmail.com> writes:
>> On Fri, Feb 17, 2023 at 8:38 AM Richard Biener <rguenther@suse.de> wrote:
>>>
>>> On Thu, 16 Feb 2023, Uros Bizjak wrote:
>>>
>>>> simplify_subreg can return VOIDmode const_int operand and will
>>>> cause ICE in simplify_gen_subreg when this operand is passed to it.
>>>>
>>>> The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
>>>> We can't process const_int operand any further, since outermode
>>>> is not an integer mode here.
>>>
>>> But if it's a CONST_INT then we know it's of int_outermode, no? That is,
>>> doesn't simplify_subreg (mode, ...) always return something in 'mode'
>>> and thus we can always pass just 'mode' as third argument to the
>>> following simplify_gen_subreg call?
>>
>> You are right. I am testing the attached patch that works too.
> 
> Thanks for this, it's the correct fix.  But as noted in
> https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610920.html,
> the final 0 is also wrong for big-endian.  Andre?
> 
> Richard
> 
>>
>> Uros.
>>
>> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
>> index 0a1dd88b0a8..3955929bb70 100644
>> --- a/gcc/simplify-rtx.cc
>> +++ b/gcc/simplify-rtx.cc
>> @@ -7665,7 +7665,7 @@ simplify_context::simplify_subreg (machine_mode outermode, rtx op,
>>       {
>>         rtx tem = simplify_subreg (int_outermode, op, innermode, byte);
>>         if (tem)
>> -	return simplify_gen_subreg (outermode, tem, GET_MODE (tem), 0);
>> +	return simplify_gen_subreg (outermode, tem, int_outermode, 0);
>>       }
>>   
>>     /* If OP is a vector comparison and the subreg is not changing the

Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Richard Sandiford]

"Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
> Hey both,
>
> Sorry about that, don't know how I missed those. Just running a test on 
> that now and will commit when it's done. I assume the comment and 0 -> 
> byte change can be seen as obvious, especially since it was supposed to 
> be in my original patch...

Thanks.  And yeah, agree it counts as obvious.

Richard

> On 27/02/2023 15:46, Richard Sandiford wrote:
>> Uros Bizjak <ubizjak@gmail.com> writes:
>>> On Fri, Feb 17, 2023 at 8:38 AM Richard Biener <rguenther@suse.de> wrote:
>>>>
>>>> On Thu, 16 Feb 2023, Uros Bizjak wrote:
>>>>
>>>>> simplify_subreg can return VOIDmode const_int operand and will
>>>>> cause ICE in simplify_gen_subreg when this operand is passed to it.
>>>>>
>>>>> The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
>>>>> We can't process const_int operand any further, since outermode
>>>>> is not an integer mode here.
>>>>
>>>> But if it's a CONST_INT then we know it's of int_outermode, no? That is,
>>>> doesn't simplify_subreg (mode, ...) always return something in 'mode'
>>>> and thus we can always pass just 'mode' as third argument to the
>>>> following simplify_gen_subreg call?
>>>
>>> You are right. I am testing the attached patch that works too.
>> 
>> Thanks for this, it's the correct fix.  But as noted in
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610920.html,
>> the final 0 is also wrong for big-endian.  Andre?
>> 
>> Richard
>> 
>>>
>>> Uros.
>>>
>>> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
>>> index 0a1dd88b0a8..3955929bb70 100644
>>> --- a/gcc/simplify-rtx.cc
>>> +++ b/gcc/simplify-rtx.cc
>>> @@ -7665,7 +7665,7 @@ simplify_context::simplify_subreg (machine_mode outermode, rtx op,
>>>       {
>>>         rtx tem = simplify_subreg (int_outermode, op, innermode, byte);
>>>         if (tem)
>>> -	return simplify_gen_subreg (outermode, tem, GET_MODE (tem), 0);
>>> +	return simplify_gen_subreg (outermode, tem, int_outermode, 0);
>>>       }
>>>   
>>>     /* If OP is a vector comparison and the subreg is not changing the

Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]

[Andre Vieira (lists)]

[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]

Committed attached patch.

On 02/03/2023 10:13, Richard Sandiford wrote:
> "Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
>> Hey both,
>>
>> Sorry about that, don't know how I missed those. Just running a test on
>> that now and will commit when it's done. I assume the comment and 0 ->
>> byte change can be seen as obvious, especially since it was supposed to
>> be in my original patch...
> 
> Thanks.  And yeah, agree it counts as obvious.
> 
> Richard
> 
>> On 27/02/2023 15:46, Richard Sandiford wrote:
>>> Uros Bizjak <ubizjak@gmail.com> writes:
>>>> On Fri, Feb 17, 2023 at 8:38 AM Richard Biener <rguenther@suse.de> wrote:
>>>>>
>>>>> On Thu, 16 Feb 2023, Uros Bizjak wrote:
>>>>>
>>>>>> simplify_subreg can return VOIDmode const_int operand and will
>>>>>> cause ICE in simplify_gen_subreg when this operand is passed to it.
>>>>>>
>>>>>> The patch prevents VOIDmode temporary from entering simplify_gen_subreg.
>>>>>> We can't process const_int operand any further, since outermode
>>>>>> is not an integer mode here.
>>>>>
>>>>> But if it's a CONST_INT then we know it's of int_outermode, no? That is,
>>>>> doesn't simplify_subreg (mode, ...) always return something in 'mode'
>>>>> and thus we can always pass just 'mode' as third argument to the
>>>>> following simplify_gen_subreg call?
>>>>
>>>> You are right. I am testing the attached patch that works too.
>>>
>>> Thanks for this, it's the correct fix.  But as noted in
>>> https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610920.html,
>>> the final 0 is also wrong for big-endian.  Andre?
>>>
>>> Richard
>>>
>>>>
>>>> Uros.
>>>>
>>>> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
>>>> index 0a1dd88b0a8..3955929bb70 100644
>>>> --- a/gcc/simplify-rtx.cc
>>>> +++ b/gcc/simplify-rtx.cc
>>>> @@ -7665,7 +7665,7 @@ simplify_context::simplify_subreg (machine_mode outermode, rtx op,
>>>>        {
>>>>          rtx tem = simplify_subreg (int_outermode, op, innermode, byte);
>>>>          if (tem)
>>>> -	return simplify_gen_subreg (outermode, tem, GET_MODE (tem), 0);
>>>> +	return simplify_gen_subreg (outermode, tem, int_outermode, 0);
>>>>        }
>>>>    
>>>>      /* If OP is a vector comparison and the subreg is not changing the

[-- Attachment #2: simplify_subreg_fix.patch --]
[-- Type: text/plain, Size: 1342 bytes --]

diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index 2c82256af664bf2bc43172fc8fb4dfb2849e64b1..3b33afa24617f3185872ddc43284e4c9cd073510 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -7667,10 +7667,10 @@ simplify_context::simplify_subreg (machine_mode outermode, rtx op,
 	}
     }
 
-  /* Try simplifying a SUBREG expression of a non-integer OUTERMODE by using a
-     NEW_OUTERMODE of the same size instead, other simplifications rely on
-     integer to integer subregs and we'd potentially miss out on optimizations
-     otherwise.  */
+  /* If the outer mode is not integral, try taking a subreg with the equivalent
+     integer outer mode and then bitcasting the result.
+     Other simplifications rely on integer to integer subregs and we'd
+     potentially miss out on optimizations otherwise.  */
   if (known_gt (GET_MODE_SIZE (innermode),
 		GET_MODE_SIZE (outermode))
       && SCALAR_INT_MODE_P (innermode)
@@ -7680,7 +7680,7 @@ simplify_context::simplify_subreg (machine_mode outermode, rtx op,
     {
       rtx tem = simplify_subreg (int_outermode, op, innermode, byte);
       if (tem)
-	return simplify_gen_subreg (outermode, tem, int_outermode, 0);
+	return simplify_gen_subreg (outermode, tem, int_outermode, byte);
     }
 
   /* If OP is a vector comparison and the subreg is not changing the