* [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] @ 2023-02-16 17:39 Uros Bizjak 2023-02-17 7:38 ` Richard Biener 0 siblings, 1 reply; 9+ messages in thread From: Uros Bizjak @ 2023-02-16 17:39 UTC (permalink / raw) To: gcc-patches; +Cc: Richard Sandiford, Richard Biener [-- 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; +} ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] 2023-02-16 17:39 [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] Uros Bizjak @ 2023-02-17 7:38 ` Richard Biener 2023-02-17 8:51 ` Uros Bizjak 0 siblings, 1 reply; 9+ messages in thread From: Richard Biener @ 2023-02-17 7:38 UTC (permalink / raw) To: Uros Bizjak; +Cc: gcc-patches, Richard Sandiford [-- 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) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] 2023-02-17 7:38 ` Richard Biener @ 2023-02-17 8:51 ` Uros Bizjak 2023-02-17 11:31 ` Richard Biener 2023-02-27 15:46 ` Richard Sandiford 0 siblings, 2 replies; 9+ messages in thread From: Uros Bizjak @ 2023-02-17 8:51 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, Richard Sandiford [-- 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] 2023-02-17 8:51 ` Uros Bizjak @ 2023-02-17 11:31 ` Richard Biener 2023-02-17 15:05 ` Uros Bizjak 2023-02-27 15:46 ` Richard Sandiford 1 sibling, 1 reply; 9+ messages in thread From: Richard Biener @ 2023-02-17 11:31 UTC (permalink / raw) To: Uros Bizjak; +Cc: gcc-patches, Richard Sandiford 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] 2023-02-17 11:31 ` Richard Biener @ 2023-02-17 15:05 ` Uros Bizjak 0 siblings, 0 replies; 9+ messages in thread From: Uros Bizjak @ 2023-02-17 15:05 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, Richard Sandiford 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] 2023-02-17 8:51 ` Uros Bizjak 2023-02-17 11:31 ` Richard Biener @ 2023-02-27 15:46 ` Richard Sandiford 2023-03-02 10:08 ` Andre Vieira (lists) 1 sibling, 1 reply; 9+ messages in thread From: Richard Sandiford @ 2023-02-27 15:46 UTC (permalink / raw) To: Uros Bizjak; +Cc: Richard Biener, gcc-patches, andre.simoesdiasvieira 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] 2023-02-27 15:46 ` Richard Sandiford @ 2023-03-02 10:08 ` Andre Vieira (lists) 2023-03-02 10:13 ` Richard Sandiford 0 siblings, 1 reply; 9+ messages in thread From: Andre Vieira (lists) @ 2023-03-02 10:08 UTC (permalink / raw) To: Uros Bizjak, Richard Biener, gcc-patches, richard.sandiford 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] 2023-03-02 10:08 ` Andre Vieira (lists) @ 2023-03-02 10:13 ` Richard Sandiford 2023-03-02 14:15 ` Andre Vieira (lists) 0 siblings, 1 reply; 9+ messages in thread From: Richard Sandiford @ 2023-03-02 10:13 UTC (permalink / raw) To: Andre Vieira (lists); +Cc: Uros Bizjak, Richard Biener, gcc-patches "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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] 2023-03-02 10:13 ` Richard Sandiford @ 2023-03-02 14:15 ` Andre Vieira (lists) 0 siblings, 0 replies; 9+ messages in thread From: Andre Vieira (lists) @ 2023-03-02 14:15 UTC (permalink / raw) To: Uros Bizjak, Richard Biener, gcc-patches, richard.sandiford [-- 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-03-02 14:15 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-16 17:39 [PATCH] simplify-rtx: Fix VOIDmode operand handling in simplify_subreg [PR108805] Uros Bizjak 2023-02-17 7:38 ` Richard Biener 2023-02-17 8:51 ` Uros Bizjak 2023-02-17 11:31 ` Richard Biener 2023-02-17 15:05 ` Uros Bizjak 2023-02-27 15:46 ` Richard Sandiford 2023-03-02 10:08 ` Andre Vieira (lists) 2023-03-02 10:13 ` Richard Sandiford 2023-03-02 14:15 ` Andre Vieira (lists)
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).