| Message ID | patch-16595-tamar@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | middle-end: replace GET_MODE_WIDER_MODE with GET_MODE_NEXT_MODE | expand |
Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > Hi All, > > After the fix to the addsub patch yesterday for bootstrap I had only regtested on x86. > While looking today it seemed the new tests were failing, this was caused > by a change in the behavior of the GET_MODE_WIDER_MODE macro on trunk. > > This patch fixes that issue. Sorry for the mess, have rebased all branches now. > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > Ok for master? > > Thanks, > Tamar > > gcc/ChangeLog: > > * match.pd: Replace GET_MODE_WIDER_MODE with > GET_MODE_NEXT_MODE. > > --- inline copy of patch -- > diff --git a/gcc/match.pd b/gcc/match.pd > index 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0e35dd58dc47092 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -7997,7 +7997,7 @@ and, > machine_mode wide_mode; > } > (if (sel.series_p (0, 2, 0, 2) > - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) > + && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode) > && VECTOR_MODE_P (wide_mode) > && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 > == GET_MODE_UNIT_BITSIZE (wide_mode))) Does anything guarantee that the next mode will be the right one? It think it would be safer to replace the last three && conditions with: && GET_MODE_2XWIDER_MODE (GET_MODE_INNER (vec_mode)).exists (&wide_elt_mode) && multiple_p (GET_MODE_NUNITS (vec_mode), 2, &wide_nunits) && related_vector_mode (vec_mode, wide_elt_mode, wide_nunits).exists (&wide_mode) Thanks, Richard
> -----Original Message----- > From: Richard Sandiford <richard.sandiford@arm.com> > Sent: Tuesday, November 15, 2022 11:59 AM > To: Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> > Cc: Tamar Christina <Tamar.Christina@arm.com>; nd <nd@arm.com>; > rguenther@suse.de; jlaw@ventanamicro.com > Subject: Re: [PATCH]middle-end: replace GET_MODE_WIDER_MODE with > GET_MODE_NEXT_MODE > > Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > Hi All, > > > > After the fix to the addsub patch yesterday for bootstrap I had only > regtested on x86. > > While looking today it seemed the new tests were failing, this was > > caused by a change in the behavior of the GET_MODE_WIDER_MODE > macro on trunk. > > > > This patch fixes that issue. Sorry for the mess, have rebased all branches > now. > > > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > > > > Ok for master? > > > > Thanks, > > Tamar > > > > gcc/ChangeLog: > > > > * match.pd: Replace GET_MODE_WIDER_MODE with > > GET_MODE_NEXT_MODE. > > > > --- inline copy of patch -- > > diff --git a/gcc/match.pd b/gcc/match.pd index > > > 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0 > e3 > > 5dd58dc47092 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -7997,7 +7997,7 @@ and, > > machine_mode wide_mode; > > } > > (if (sel.series_p (0, 2, 0, 2) > > - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) > > + && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode) > > && VECTOR_MODE_P (wide_mode) > > && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 > > == GET_MODE_UNIT_BITSIZE (wide_mode))) > > Does anything guarantee that the next mode will be the right one? > It think it would be safer to replace the last three && conditions with: > > && GET_MODE_2XWIDER_MODE (GET_MODE_INNER (vec_mode)).exists > (&wide_elt_mode) > && multiple_p (GET_MODE_NUNITS (vec_mode), 2, &wide_nunits) > && related_vector_mode (vec_mode, wide_elt_mode, > wide_nunits).exists (&wide_mode) I see, respun patch accordingly. Ok for master? --- inline copy of patch --- diff --git a/gcc/match.pd b/gcc/match.pd index 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..82f05bbc912e4f80f3984d930c4a8dcb010136e1 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7995,12 +7995,15 @@ and, vec_perm_indices sel (builder, 2, nelts); machine_mode vec_mode = TYPE_MODE (type); machine_mode wide_mode; + scalar_mode wide_elt_mode; + poly_uint64 wide_nunits; + scalar_mode inner_mode = GET_MODE_INNER (vec_mode); } (if (sel.series_p (0, 2, 0, 2) - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) - && VECTOR_MODE_P (wide_mode) - && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 - == GET_MODE_UNIT_BITSIZE (wide_mode))) + && GET_MODE_2XWIDER_MODE (inner_mode).exists (&wide_elt_mode) + && multiple_p (GET_MODE_NUNITS (vec_mode), 2, &wide_nunits) + && related_vector_mode (vec_mode, wide_elt_mode, + wide_nunits).exists (&wide_mode)) (with { tree stype
Tamar Christina <Tamar.Christina@arm.com> writes: >> -----Original Message----- >> From: Richard Sandiford <richard.sandiford@arm.com> >> Sent: Tuesday, November 15, 2022 11:59 AM >> To: Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> >> Cc: Tamar Christina <Tamar.Christina@arm.com>; nd <nd@arm.com>; >> rguenther@suse.de; jlaw@ventanamicro.com >> Subject: Re: [PATCH]middle-end: replace GET_MODE_WIDER_MODE with >> GET_MODE_NEXT_MODE >> >> Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> > Hi All, >> > >> > After the fix to the addsub patch yesterday for bootstrap I had only >> regtested on x86. >> > While looking today it seemed the new tests were failing, this was >> > caused by a change in the behavior of the GET_MODE_WIDER_MODE >> macro on trunk. >> > >> > This patch fixes that issue. Sorry for the mess, have rebased all branches >> now. >> > >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. >> > >> > Ok for master? >> > >> > Thanks, >> > Tamar >> > >> > gcc/ChangeLog: >> > >> > * match.pd: Replace GET_MODE_WIDER_MODE with >> > GET_MODE_NEXT_MODE. >> > >> > --- inline copy of patch -- >> > diff --git a/gcc/match.pd b/gcc/match.pd index >> > >> 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0 >> e3 >> > 5dd58dc47092 100644 >> > --- a/gcc/match.pd >> > +++ b/gcc/match.pd >> > @@ -7997,7 +7997,7 @@ and, >> > machine_mode wide_mode; >> > } >> > (if (sel.series_p (0, 2, 0, 2) >> > - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) >> > + && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode) >> > && VECTOR_MODE_P (wide_mode) >> > && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 >> > == GET_MODE_UNIT_BITSIZE (wide_mode))) >> >> Does anything guarantee that the next mode will be the right one? >> It think it would be safer to replace the last three && conditions with: >> >> && GET_MODE_2XWIDER_MODE (GET_MODE_INNER (vec_mode)).exists >> (&wide_elt_mode) >> && multiple_p (GET_MODE_NUNITS (vec_mode), 2, &wide_nunits) >> && related_vector_mode (vec_mode, wide_elt_mode, >> wide_nunits).exists (&wide_mode) > > I see, respun patch accordingly. LGTM, but I'm nervous when it comes to match.pd stuff so I'd prefer Richi or Jeff to have the final say. Thanks, Richard > > Ok for master? > > --- inline copy of patch --- > > diff --git a/gcc/match.pd b/gcc/match.pd > index 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..82f05bbc912e4f80f3984d930c4a8dcb010136e1 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -7995,12 +7995,15 @@ and, > vec_perm_indices sel (builder, 2, nelts); > machine_mode vec_mode = TYPE_MODE (type); > machine_mode wide_mode; > + scalar_mode wide_elt_mode; > + poly_uint64 wide_nunits; > + scalar_mode inner_mode = GET_MODE_INNER (vec_mode); > } > (if (sel.series_p (0, 2, 0, 2) > - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) > - && VECTOR_MODE_P (wide_mode) > - && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 > - == GET_MODE_UNIT_BITSIZE (wide_mode))) > + && GET_MODE_2XWIDER_MODE (inner_mode).exists (&wide_elt_mode) > + && multiple_p (GET_MODE_NUNITS (vec_mode), 2, &wide_nunits) > + && related_vector_mode (vec_mode, wide_elt_mode, > + wide_nunits).exists (&wide_mode)) > (with > { > tree stype
On 11/15/22 07:54, Richard Sandiford via Gcc-patches wrote: > Tamar Christina <Tamar.Christina@arm.com> writes: >>> -----Original Message----- >>> From: Richard Sandiford <richard.sandiford@arm.com> >>> Sent: Tuesday, November 15, 2022 11:59 AM >>> To: Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> >>> Cc: Tamar Christina <Tamar.Christina@arm.com>; nd <nd@arm.com>; >>> rguenther@suse.de; jlaw@ventanamicro.com >>> Subject: Re: [PATCH]middle-end: replace GET_MODE_WIDER_MODE with >>> GET_MODE_NEXT_MODE >>> >>> Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >>>> Hi All, >>>> >>>> After the fix to the addsub patch yesterday for bootstrap I had only >>> regtested on x86. >>>> While looking today it seemed the new tests were failing, this was >>>> caused by a change in the behavior of the GET_MODE_WIDER_MODE >>> macro on trunk. >>>> This patch fixes that issue. Sorry for the mess, have rebased all branches >>> now. >>>> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. >>>> >>>> Ok for master? >>>> >>>> Thanks, >>>> Tamar >>>> >>>> gcc/ChangeLog: >>>> >>>> * match.pd: Replace GET_MODE_WIDER_MODE with >>>> GET_MODE_NEXT_MODE. >>>> >>>> --- inline copy of patch -- >>>> diff --git a/gcc/match.pd b/gcc/match.pd index >>>> >>> 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0 >>> e3 >>>> 5dd58dc47092 100644 >>>> --- a/gcc/match.pd >>>> +++ b/gcc/match.pd >>>> @@ -7997,7 +7997,7 @@ and, >>>> machine_mode wide_mode; >>>> } >>>> (if (sel.series_p (0, 2, 0, 2) >>>> - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) >>>> + && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode) >>>> && VECTOR_MODE_P (wide_mode) >>>> && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 >>>> == GET_MODE_UNIT_BITSIZE (wide_mode))) >>> Does anything guarantee that the next mode will be the right one? >>> It think it would be safer to replace the last three && conditions with: >>> >>> && GET_MODE_2XWIDER_MODE (GET_MODE_INNER (vec_mode)).exists >>> (&wide_elt_mode) >>> && multiple_p (GET_MODE_NUNITS (vec_mode), 2, &wide_nunits) >>> && related_vector_mode (vec_mode, wide_elt_mode, >>> wide_nunits).exists (&wide_mode) >> I see, respun patch accordingly. > LGTM, but I'm nervous when it comes to match.pd stuff so I'd prefer > Richi or Jeff to have the final say. It's just a matter of finding that 2X wider mode to make the transformation possible. So I don't see any concerns here. jeff
On Tue, 15 Nov 2022, Richard Sandiford wrote: > Tamar Christina <Tamar.Christina@arm.com> writes: > >> -----Original Message----- > >> From: Richard Sandiford <richard.sandiford@arm.com> > >> Sent: Tuesday, November 15, 2022 11:59 AM > >> To: Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> > >> Cc: Tamar Christina <Tamar.Christina@arm.com>; nd <nd@arm.com>; > >> rguenther@suse.de; jlaw@ventanamicro.com > >> Subject: Re: [PATCH]middle-end: replace GET_MODE_WIDER_MODE with > >> GET_MODE_NEXT_MODE > >> > >> Tamar Christina via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > >> > Hi All, > >> > > >> > After the fix to the addsub patch yesterday for bootstrap I had only > >> regtested on x86. > >> > While looking today it seemed the new tests were failing, this was > >> > caused by a change in the behavior of the GET_MODE_WIDER_MODE > >> macro on trunk. > >> > > >> > This patch fixes that issue. Sorry for the mess, have rebased all branches > >> now. > >> > > >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. > >> > > >> > Ok for master? > >> > > >> > Thanks, > >> > Tamar > >> > > >> > gcc/ChangeLog: > >> > > >> > * match.pd: Replace GET_MODE_WIDER_MODE with > >> > GET_MODE_NEXT_MODE. > >> > > >> > --- inline copy of patch -- > >> > diff --git a/gcc/match.pd b/gcc/match.pd index > >> > > >> 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0 > >> e3 > >> > 5dd58dc47092 100644 > >> > --- a/gcc/match.pd > >> > +++ b/gcc/match.pd > >> > @@ -7997,7 +7997,7 @@ and, > >> > machine_mode wide_mode; > >> > } > >> > (if (sel.series_p (0, 2, 0, 2) > >> > - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) > >> > + && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode) > >> > && VECTOR_MODE_P (wide_mode) > >> > && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 > >> > == GET_MODE_UNIT_BITSIZE (wide_mode))) > >> > >> Does anything guarantee that the next mode will be the right one? > >> It think it would be safer to replace the last three && conditions with: > >> > >> && GET_MODE_2XWIDER_MODE (GET_MODE_INNER (vec_mode)).exists > >> (&wide_elt_mode) > >> && multiple_p (GET_MODE_NUNITS (vec_mode), 2, &wide_nunits) > >> && related_vector_mode (vec_mode, wide_elt_mode, > >> wide_nunits).exists (&wide_mode) > > > > I see, respun patch accordingly. > > LGTM, but I'm nervous when it comes to match.pd stuff so I'd prefer > Richi or Jeff to have the final say. I see nothing wrong here, so OK. Richard. > Thanks, > Richard > > > > > Ok for master? > > > > --- inline copy of patch --- > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > index 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..82f05bbc912e4f80f3984d930c4a8dcb010136e1 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -7995,12 +7995,15 @@ and, > > vec_perm_indices sel (builder, 2, nelts); > > machine_mode vec_mode = TYPE_MODE (type); > > machine_mode wide_mode; > > + scalar_mode wide_elt_mode; > > + poly_uint64 wide_nunits; > > + scalar_mode inner_mode = GET_MODE_INNER (vec_mode); > > } > > (if (sel.series_p (0, 2, 0, 2) > > - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) > > - && VECTOR_MODE_P (wide_mode) > > - && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 > > - == GET_MODE_UNIT_BITSIZE (wide_mode))) > > + && GET_MODE_2XWIDER_MODE (inner_mode).exists (&wide_elt_mode) > > + && multiple_p (GET_MODE_NUNITS (vec_mode), 2, &wide_nunits) > > + && related_vector_mode (vec_mode, wide_elt_mode, > > + wide_nunits).exists (&wide_mode)) > > (with > > { > > tree stype >
--- a/gcc/match.pd +++ b/gcc/match.pd @@ -7997,7 +7997,7 @@ and, machine_mode wide_mode; } (if (sel.series_p (0, 2, 0, 2) - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) + && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode) && VECTOR_MODE_P (wide_mode) && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 == GET_MODE_UNIT_BITSIZE (wide_mode)))
Hi All, After the fix to the addsub patch yesterday for bootstrap I had only regtested on x86. While looking today it seemed the new tests were failing, this was caused by a change in the behavior of the GET_MODE_WIDER_MODE macro on trunk. This patch fixes that issue. Sorry for the mess, have rebased all branches now. Bootstrapped Regtested on aarch64-none-linux-gnu and no issues. Ok for master? Thanks, Tamar gcc/ChangeLog: * match.pd: Replace GET_MODE_WIDER_MODE with GET_MODE_NEXT_MODE. --- inline copy of patch -- diff --git a/gcc/match.pd b/gcc/match.pd index 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0e35dd58dc47092 100644 -- diff --git a/gcc/match.pd b/gcc/match.pd index 1b0ab7cf60fa4772fbe8304c622b0b8fab1bdefa..28191a992039c6f3a1dab5f7c0e35dd58dc47092 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7997,7 +7997,7 @@ and, machine_mode wide_mode; } (if (sel.series_p (0, 2, 0, 2) - && GET_MODE_WIDER_MODE (vec_mode).exists (&wide_mode) + && GET_MODE_NEXT_MODE (vec_mode).exists (&wide_mode) && VECTOR_MODE_P (wide_mode) && (GET_MODE_UNIT_BITSIZE (vec_mode) * 2 == GET_MODE_UNIT_BITSIZE (wide_mode)))