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

["Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com>]

[-- 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