Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]
Committed attached patch.
On 02/03/2023 10:13, Richard Sandiford wrote:
"Andre Vieira (lists)" 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 writes:
On Fri, Feb 17, 2023 at 8:38 AM Richard Biener 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 thediff --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
Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805]
"Andre Vieira (lists)" 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 writes:
>>> On Fri, Feb 17, 2023 at 8:38 AM Richard Biener 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]
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 writes:
On Fri, Feb 17, 2023 at 8:38 AM Richard Biener 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]
Uros Bizjak writes:
> On Fri, Feb 17, 2023 at 8:38 AM Richard Biener 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]
On Fri, Feb 17, 2023 at 12:31 PM Richard Biener wrote: > > On Fri, 17 Feb 2023, Uros Bizjak wrote: > > > On Fri, Feb 17, 2023 at 8:38 AM Richard Biener 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 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]
On Fri, 17 Feb 2023, Uros Bizjak wrote: > On Fri, Feb 17, 2023 at 8:38 AM Richard Biener 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]
On Fri, Feb 17, 2023 at 8:38 AM Richard Biener 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.
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]
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
>
> 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
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
[PATCH] 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 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
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.
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 (_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 000..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;
+}
