* [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
@ 2020-09-09 18:27 Aaron Sawdey
2020-09-10 9:32 ` Richard Sandiford
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Aaron Sawdey @ 2020-09-09 18:27 UTC (permalink / raw)
To: gcc-patches; +Cc: segher, wschmidt, richard.sandiford, jakub, Aaron Sawdey
Now that the documentation for partial modes says they have a known
number of bits of precision, would it make sense for extract_low_bits to
check this before attempting to extract the bits?
This would solve the problem we have been having with POImode and
extract_low_bits -- DSE tries to use it to extract part of a POImode
register used in a previous store. We do not want to supply any patterns
to make POImode (or OImode) used like a regular integer mode.
This patch adds such a check, and sets the precision of POImode to one
bit, which resolves the problems of PR/96791 for ppc64 target.
Bootstrap passes on ppc64le and x86_64.
Thanks,
Aaron
gcc/ChangeLog:
* config/rs6000/rs6000-modes.def (POImode): Change precision.
* expmed.c (extract_low_bits): Check precision.
---
gcc/config/rs6000/rs6000-modes.def | 2 +-
gcc/expmed.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/gcc/config/rs6000/rs6000-modes.def b/gcc/config/rs6000/rs6000-modes.def
index ddb218b3fba..aa7d60dd835 100644
--- a/gcc/config/rs6000/rs6000-modes.def
+++ b/gcc/config/rs6000/rs6000-modes.def
@@ -90,5 +90,5 @@ INT_MODE (OI, 32);
INT_MODE (XI, 64);
/* Modes used by __vector_pair and __vector_quad. */
-PARTIAL_INT_MODE (OI, 256, POI); /* __vector_pair. */
+PARTIAL_INT_MODE (OI, 1, POI); /* __vector_pair. */
PARTIAL_INT_MODE (XI, 512, PXI); /* __vector_quad. */
diff --git a/gcc/expmed.c b/gcc/expmed.c
index d34f0fb0b54..23ca181afa6 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -2396,6 +2396,9 @@ extract_low_bits (machine_mode mode, machine_mode src_mode, rtx src)
if (GET_MODE_CLASS (mode) == MODE_CC || GET_MODE_CLASS (src_mode) == MODE_CC)
return NULL_RTX;
+ if (known_lt (GET_MODE_PRECISION (src_mode), GET_MODE_BITSIZE (mode)))
+ return NULL_RTX;
+
if (known_eq (GET_MODE_BITSIZE (mode), GET_MODE_BITSIZE (src_mode))
&& targetm.modes_tieable_p (mode, src_mode))
{
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-09 18:27 [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes Aaron Sawdey
@ 2020-09-10 9:32 ` Richard Sandiford
2020-09-10 9:36 ` Richard Biener
2020-10-05 14:39 ` Ping: " Aaron Sawdey
2 siblings, 0 replies; 13+ messages in thread
From: Richard Sandiford @ 2020-09-10 9:32 UTC (permalink / raw)
To: Aaron Sawdey; +Cc: gcc-patches, segher, wschmidt, jakub
Aaron Sawdey <acsawdey@linux.ibm.com> writes:
> Now that the documentation for partial modes says they have a known
> number of bits of precision, would it make sense for extract_low_bits to
> check this before attempting to extract the bits?
>
> This would solve the problem we have been having with POImode and
> extract_low_bits -- DSE tries to use it to extract part of a POImode
> register used in a previous store. We do not want to supply any patterns
> to make POImode (or OImode) used like a regular integer mode.
>
> This patch adds such a check, and sets the precision of POImode to one
> bit, which resolves the problems of PR/96791 for ppc64 target.
>
> Bootstrap passes on ppc64le and x86_64.
>
> Thanks,
> Aaron
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000-modes.def (POImode): Change precision.
> * expmed.c (extract_low_bits): Check precision.
> ---
> gcc/config/rs6000/rs6000-modes.def | 2 +-
> gcc/expmed.c | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/rs6000/rs6000-modes.def b/gcc/config/rs6000/rs6000-modes.def
> index ddb218b3fba..aa7d60dd835 100644
> --- a/gcc/config/rs6000/rs6000-modes.def
> +++ b/gcc/config/rs6000/rs6000-modes.def
> @@ -90,5 +90,5 @@ INT_MODE (OI, 32);
> INT_MODE (XI, 64);
>
> /* Modes used by __vector_pair and __vector_quad. */
> -PARTIAL_INT_MODE (OI, 256, POI); /* __vector_pair. */
> +PARTIAL_INT_MODE (OI, 1, POI); /* __vector_pair. */
> PARTIAL_INT_MODE (XI, 512, PXI); /* __vector_quad. */
> diff --git a/gcc/expmed.c b/gcc/expmed.c
> index d34f0fb0b54..23ca181afa6 100644
> --- a/gcc/expmed.c
> +++ b/gcc/expmed.c
> @@ -2396,6 +2396,9 @@ extract_low_bits (machine_mode mode, machine_mode src_mode, rtx src)
> if (GET_MODE_CLASS (mode) == MODE_CC || GET_MODE_CLASS (src_mode) == MODE_CC)
> return NULL_RTX;
>
> + if (known_lt (GET_MODE_PRECISION (src_mode), GET_MODE_BITSIZE (mode)))
> + return NULL_RTX;
extract_low_bits has defined semantics when MODE is wider than SRC_MODE:
- when MODE is wider than SRC_MODE, the extraction involves
a zero extension
Also, I'd be worried that setting the precision to 1 (although a nice
hack for some things) could lead to miscompilation, since it would be
telling target-independent code that only the low bit of the mode is
significant.
Thanks,
Richard
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-09 18:27 [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes Aaron Sawdey
2020-09-10 9:32 ` Richard Sandiford
@ 2020-09-10 9:36 ` Richard Biener
2020-09-10 14:22 ` Aaron Sawdey
2020-10-05 14:39 ` Ping: " Aaron Sawdey
2 siblings, 1 reply; 13+ messages in thread
From: Richard Biener @ 2020-09-10 9:36 UTC (permalink / raw)
To: Aaron Sawdey; +Cc: GCC Patches, Jakub Jelinek, Bill Schmidt, Segher Boessenkool
On Wed, Sep 9, 2020 at 8:28 PM Aaron Sawdey via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Now that the documentation for partial modes says they have a known
> number of bits of precision, would it make sense for extract_low_bits to
> check this before attempting to extract the bits?
>
> This would solve the problem we have been having with POImode and
> extract_low_bits -- DSE tries to use it to extract part of a POImode
> register used in a previous store. We do not want to supply any patterns
> to make POImode (or OImode) used like a regular integer mode.
>
> This patch adds such a check, and sets the precision of POImode to one
> bit, which resolves the problems of PR/96791 for ppc64 target.
How many bits are you actually storing in POImode? If you say it's
precision is 1 then the middle-end might be tempted to ignore any
changes to the upper bits. You now probably say "but we don't have
any such interesting operation done on POImode" but still ... it feels
like a hack.
Richard.
> Bootstrap passes on ppc64le and x86_64.
>
> Thanks,
> Aaron
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000-modes.def (POImode): Change precision.
> * expmed.c (extract_low_bits): Check precision.
> ---
> gcc/config/rs6000/rs6000-modes.def | 2 +-
> gcc/expmed.c | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/rs6000/rs6000-modes.def b/gcc/config/rs6000/rs6000-modes.def
> index ddb218b3fba..aa7d60dd835 100644
> --- a/gcc/config/rs6000/rs6000-modes.def
> +++ b/gcc/config/rs6000/rs6000-modes.def
> @@ -90,5 +90,5 @@ INT_MODE (OI, 32);
> INT_MODE (XI, 64);
>
> /* Modes used by __vector_pair and __vector_quad. */
> -PARTIAL_INT_MODE (OI, 256, POI); /* __vector_pair. */
> +PARTIAL_INT_MODE (OI, 1, POI); /* __vector_pair. */
> PARTIAL_INT_MODE (XI, 512, PXI); /* __vector_quad. */
> diff --git a/gcc/expmed.c b/gcc/expmed.c
> index d34f0fb0b54..23ca181afa6 100644
> --- a/gcc/expmed.c
> +++ b/gcc/expmed.c
> @@ -2396,6 +2396,9 @@ extract_low_bits (machine_mode mode, machine_mode src_mode, rtx src)
> if (GET_MODE_CLASS (mode) == MODE_CC || GET_MODE_CLASS (src_mode) == MODE_CC)
> return NULL_RTX;
>
> + if (known_lt (GET_MODE_PRECISION (src_mode), GET_MODE_BITSIZE (mode)))
> + return NULL_RTX;
> +
> if (known_eq (GET_MODE_BITSIZE (mode), GET_MODE_BITSIZE (src_mode))
> && targetm.modes_tieable_p (mode, src_mode))
> {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-10 9:36 ` Richard Biener
@ 2020-09-10 14:22 ` Aaron Sawdey
2020-09-10 14:33 ` Richard Biener
0 siblings, 1 reply; 13+ messages in thread
From: Aaron Sawdey @ 2020-09-10 14:22 UTC (permalink / raw)
To: Richard Biener
Cc: gcc-patches, Jakub Jelinek, Bill Schmidt, Segher Boessenkool
If it feels like a hack, that would because it is a hack.
What I’d really like to discuss is how to accomplish the real goal: keep anything from trying to do other operations (zero/sign extend for one) to POImode.
Is there an existing mechanism for this?
Thanks,
Aaron
Aaron Sawdey, Ph.D. sawdey@linux.ibm.com
IBM Linux on POWER Toolchain
> On Sep 10, 2020, at 4:36 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Wed, Sep 9, 2020 at 8:28 PM Aaron Sawdey via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Now that the documentation for partial modes says they have a known
>> number of bits of precision, would it make sense for extract_low_bits to
>> check this before attempting to extract the bits?
>>
>> This would solve the problem we have been having with POImode and
>> extract_low_bits -- DSE tries to use it to extract part of a POImode
>> register used in a previous store. We do not want to supply any patterns
>> to make POImode (or OImode) used like a regular integer mode.
>>
>> This patch adds such a check, and sets the precision of POImode to one
>> bit, which resolves the problems of PR/96791 for ppc64 target.
>
> How many bits are you actually storing in POImode? If you say it's
> precision is 1 then the middle-end might be tempted to ignore any
> changes to the upper bits. You now probably say "but we don't have
> any such interesting operation done on POImode" but still ... it feels
> like a hack.
>
> Richard.
>
>> Bootstrap passes on ppc64le and x86_64.
>>
>> Thanks,
>> Aaron
>>
>> gcc/ChangeLog:
>>
>> * config/rs6000/rs6000-modes.def (POImode): Change precision.
>> * expmed.c (extract_low_bits): Check precision.
>> ---
>> gcc/config/rs6000/rs6000-modes.def | 2 +-
>> gcc/expmed.c | 3 +++
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/config/rs6000/rs6000-modes.def b/gcc/config/rs6000/rs6000-modes.def
>> index ddb218b3fba..aa7d60dd835 100644
>> --- a/gcc/config/rs6000/rs6000-modes.def
>> +++ b/gcc/config/rs6000/rs6000-modes.def
>> @@ -90,5 +90,5 @@ INT_MODE (OI, 32);
>> INT_MODE (XI, 64);
>>
>> /* Modes used by __vector_pair and __vector_quad. */
>> -PARTIAL_INT_MODE (OI, 256, POI); /* __vector_pair. */
>> +PARTIAL_INT_MODE (OI, 1, POI); /* __vector_pair. */
>> PARTIAL_INT_MODE (XI, 512, PXI); /* __vector_quad. */
>> diff --git a/gcc/expmed.c b/gcc/expmed.c
>> index d34f0fb0b54..23ca181afa6 100644
>> --- a/gcc/expmed.c
>> +++ b/gcc/expmed.c
>> @@ -2396,6 +2396,9 @@ extract_low_bits (machine_mode mode, machine_mode src_mode, rtx src)
>> if (GET_MODE_CLASS (mode) == MODE_CC || GET_MODE_CLASS (src_mode) == MODE_CC)
>> return NULL_RTX;
>>
>> + if (known_lt (GET_MODE_PRECISION (src_mode), GET_MODE_BITSIZE (mode)))
>> + return NULL_RTX;
>> +
>> if (known_eq (GET_MODE_BITSIZE (mode), GET_MODE_BITSIZE (src_mode))
>> && targetm.modes_tieable_p (mode, src_mode))
>> {
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-10 14:22 ` Aaron Sawdey
@ 2020-09-10 14:33 ` Richard Biener
2020-09-10 15:10 ` Segher Boessenkool
0 siblings, 1 reply; 13+ messages in thread
From: Richard Biener @ 2020-09-10 14:33 UTC (permalink / raw)
To: Aaron Sawdey; +Cc: gcc-patches, Jakub Jelinek, Bill Schmidt, Segher Boessenkool
On Thu, Sep 10, 2020 at 4:22 PM Aaron Sawdey <acsawdey@linux.ibm.com> wrote:
>
> If it feels like a hack, that would because it is a hack.
>
> What I’d really like to discuss is how to accomplish the real goal: keep anything from trying to do other operations (zero/sign extend for one) to POImode.
>
> Is there an existing mechanism for this?
Not that I know, but somehow x86 gets away with OImode and XImode without
providing too many patterns for those.
Richard.
> Thanks,
> Aaron
>
> Aaron Sawdey, Ph.D. sawdey@linux.ibm.com
> IBM Linux on POWER Toolchain
>
>
> > On Sep 10, 2020, at 4:36 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Wed, Sep 9, 2020 at 8:28 PM Aaron Sawdey via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> Now that the documentation for partial modes says they have a known
> >> number of bits of precision, would it make sense for extract_low_bits to
> >> check this before attempting to extract the bits?
> >>
> >> This would solve the problem we have been having with POImode and
> >> extract_low_bits -- DSE tries to use it to extract part of a POImode
> >> register used in a previous store. We do not want to supply any patterns
> >> to make POImode (or OImode) used like a regular integer mode.
> >>
> >> This patch adds such a check, and sets the precision of POImode to one
> >> bit, which resolves the problems of PR/96791 for ppc64 target.
> >
> > How many bits are you actually storing in POImode? If you say it's
> > precision is 1 then the middle-end might be tempted to ignore any
> > changes to the upper bits. You now probably say "but we don't have
> > any such interesting operation done on POImode" but still ... it feels
> > like a hack.
> >
> > Richard.
> >
> >> Bootstrap passes on ppc64le and x86_64.
> >>
> >> Thanks,
> >> Aaron
> >>
> >> gcc/ChangeLog:
> >>
> >> * config/rs6000/rs6000-modes.def (POImode): Change precision.
> >> * expmed.c (extract_low_bits): Check precision.
> >> ---
> >> gcc/config/rs6000/rs6000-modes.def | 2 +-
> >> gcc/expmed.c | 3 +++
> >> 2 files changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/gcc/config/rs6000/rs6000-modes.def b/gcc/config/rs6000/rs6000-modes.def
> >> index ddb218b3fba..aa7d60dd835 100644
> >> --- a/gcc/config/rs6000/rs6000-modes.def
> >> +++ b/gcc/config/rs6000/rs6000-modes.def
> >> @@ -90,5 +90,5 @@ INT_MODE (OI, 32);
> >> INT_MODE (XI, 64);
> >>
> >> /* Modes used by __vector_pair and __vector_quad. */
> >> -PARTIAL_INT_MODE (OI, 256, POI); /* __vector_pair. */
> >> +PARTIAL_INT_MODE (OI, 1, POI); /* __vector_pair. */
> >> PARTIAL_INT_MODE (XI, 512, PXI); /* __vector_quad. */
> >> diff --git a/gcc/expmed.c b/gcc/expmed.c
> >> index d34f0fb0b54..23ca181afa6 100644
> >> --- a/gcc/expmed.c
> >> +++ b/gcc/expmed.c
> >> @@ -2396,6 +2396,9 @@ extract_low_bits (machine_mode mode, machine_mode src_mode, rtx src)
> >> if (GET_MODE_CLASS (mode) == MODE_CC || GET_MODE_CLASS (src_mode) == MODE_CC)
> >> return NULL_RTX;
> >>
> >> + if (known_lt (GET_MODE_PRECISION (src_mode), GET_MODE_BITSIZE (mode)))
> >> + return NULL_RTX;
> >> +
> >> if (known_eq (GET_MODE_BITSIZE (mode), GET_MODE_BITSIZE (src_mode))
> >> && targetm.modes_tieable_p (mode, src_mode))
> >> {
> >> --
> >> 2.17.1
> >>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-10 14:33 ` Richard Biener
@ 2020-09-10 15:10 ` Segher Boessenkool
2020-09-10 16:11 ` Aaron Sawdey
2020-09-11 6:07 ` Richard Biener
0 siblings, 2 replies; 13+ messages in thread
From: Segher Boessenkool @ 2020-09-10 15:10 UTC (permalink / raw)
To: Richard Biener; +Cc: Aaron Sawdey, gcc-patches, Jakub Jelinek, Bill Schmidt
Hi!
On Thu, Sep 10, 2020 at 04:33:30PM +0200, Richard Biener wrote:
> On Thu, Sep 10, 2020 at 4:22 PM Aaron Sawdey <acsawdey@linux.ibm.com> wrote:
> > If it feels like a hack, that would because it is a hack.
> >
> > What I’d really like to discuss is how to accomplish the real goal: keep anything from trying to do other operations (zero/sign extend for one) to POImode.
> >
> > Is there an existing mechanism for this?
>
> Not that I know, but somehow x86 gets away with OImode and XImode without
> providing too many patterns for those.
What we were seeing is DSE (of all things!) tries to extract a DImode
from a POImode (and expects that insn to exist!) That is no good.
Segher
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-10 15:10 ` Segher Boessenkool
@ 2020-09-10 16:11 ` Aaron Sawdey
2020-09-11 6:07 ` Richard Biener
1 sibling, 0 replies; 13+ messages in thread
From: Aaron Sawdey @ 2020-09-10 16:11 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Richard Biener, gcc-patches, Jakub Jelinek, Bill Schmidt
So, would it be legitimate for extract_low_bits to query if the truncate pattern it will likely use is actually available?
Aaron Sawdey, Ph.D. sawdey@linux.ibm.com
IBM Linux on POWER Toolchain
> On Sep 10, 2020, at 10:10 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Thu, Sep 10, 2020 at 04:33:30PM +0200, Richard Biener wrote:
>> On Thu, Sep 10, 2020 at 4:22 PM Aaron Sawdey <acsawdey@linux.ibm.com> wrote:
>>> If it feels like a hack, that would because it is a hack.
>>>
>>> What I’d really like to discuss is how to accomplish the real goal: keep anything from trying to do other operations (zero/sign extend for one) to POImode.
>>>
>>> Is there an existing mechanism for this?
>>
>> Not that I know, but somehow x86 gets away with OImode and XImode without
>> providing too many patterns for those.
>
> What we were seeing is DSE (of all things!) tries to extract a DImode
> from a POImode (and expects that insn to exist!) That is no good.
>
>
> Segher
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-10 15:10 ` Segher Boessenkool
2020-09-10 16:11 ` Aaron Sawdey
@ 2020-09-11 6:07 ` Richard Biener
2020-09-11 14:16 ` Segher Boessenkool
1 sibling, 1 reply; 13+ messages in thread
From: Richard Biener @ 2020-09-11 6:07 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Aaron Sawdey, gcc-patches, Jakub Jelinek, Bill Schmidt
On Thu, Sep 10, 2020 at 5:12 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> Hi!
>
> On Thu, Sep 10, 2020 at 04:33:30PM +0200, Richard Biener wrote:
> > On Thu, Sep 10, 2020 at 4:22 PM Aaron Sawdey <acsawdey@linux.ibm.com> wrote:
> > > If it feels like a hack, that would because it is a hack.
> > >
> > > What I’d really like to discuss is how to accomplish the real goal: keep anything from trying to do other operations (zero/sign extend for one) to POImode.
> > >
> > > Is there an existing mechanism for this?
> >
> > Not that I know, but somehow x86 gets away with OImode and XImode without
> > providing too many patterns for those.
>
> What we were seeing is DSE (of all things!) tries to extract a DImode
> from a POImode (and expects that insn to exist!) That is no good.
Maybe. I don't know what kind of operations have to exist if a mode
is present and what not.
But are you sure this will be the only case you'll run into?
Richard.
>
> Segher
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-11 6:07 ` Richard Biener
@ 2020-09-11 14:16 ` Segher Boessenkool
2020-09-14 7:46 ` Richard Biener
0 siblings, 1 reply; 13+ messages in thread
From: Segher Boessenkool @ 2020-09-11 14:16 UTC (permalink / raw)
To: Richard Biener; +Cc: Aaron Sawdey, gcc-patches, Jakub Jelinek, Bill Schmidt
On Fri, Sep 11, 2020 at 08:07:39AM +0200, Richard Biener wrote:
> On Thu, Sep 10, 2020 at 5:12 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Thu, Sep 10, 2020 at 04:33:30PM +0200, Richard Biener wrote:
> > > On Thu, Sep 10, 2020 at 4:22 PM Aaron Sawdey <acsawdey@linux.ibm.com> wrote:
> > > > If it feels like a hack, that would because it is a hack.
> > > >
> > > > What I’d really like to discuss is how to accomplish the real goal: keep anything from trying to do other operations (zero/sign extend for one) to POImode.
> > > >
> > > > Is there an existing mechanism for this?
> > >
> > > Not that I know, but somehow x86 gets away with OImode and XImode without
> > > providing too many patterns for those.
> >
> > What we were seeing is DSE (of all things!) tries to extract a DImode
> > from a POImode (and expects that insn to exist!) That is no good.
>
> Maybe. I don't know what kind of operations have to exist if a mode
> is present and what not.
> But are you sure this will be the only case you'll run into?
No, I am not sure if there are bugs related to this elsewhere in the
compiler :-)
Until 2014 (and documented just days ago ;-) ) all bits of a partial
integer mode were considered unknown. I have looked at a lot of it in
our code the past weeks, and we still treat it like that in most places.
We now see bootstrap problems if we use POImode in some contexts (that's
this PR96791). POImode can only live in pairs of VSX registers; taking
a subreg of POImode that would not be valid on one VSX register is not
okay.
Maybe we are missing some hooks or macros?
Segher
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-11 14:16 ` Segher Boessenkool
@ 2020-09-14 7:46 ` Richard Biener
2020-09-14 15:47 ` Segher Boessenkool
0 siblings, 1 reply; 13+ messages in thread
From: Richard Biener @ 2020-09-14 7:46 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Aaron Sawdey, gcc-patches, Jakub Jelinek, Bill Schmidt
On Fri, Sep 11, 2020 at 4:18 PM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Fri, Sep 11, 2020 at 08:07:39AM +0200, Richard Biener wrote:
> > On Thu, Sep 10, 2020 at 5:12 PM Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> > > On Thu, Sep 10, 2020 at 04:33:30PM +0200, Richard Biener wrote:
> > > > On Thu, Sep 10, 2020 at 4:22 PM Aaron Sawdey <acsawdey@linux.ibm.com> wrote:
> > > > > If it feels like a hack, that would because it is a hack.
> > > > >
> > > > > What I’d really like to discuss is how to accomplish the real goal: keep anything from trying to do other operations (zero/sign extend for one) to POImode.
> > > > >
> > > > > Is there an existing mechanism for this?
> > > >
> > > > Not that I know, but somehow x86 gets away with OImode and XImode without
> > > > providing too many patterns for those.
> > >
> > > What we were seeing is DSE (of all things!) tries to extract a DImode
> > > from a POImode (and expects that insn to exist!) That is no good.
> >
> > Maybe. I don't know what kind of operations have to exist if a mode
> > is present and what not.
> > But are you sure this will be the only case you'll run into?
>
> No, I am not sure if there are bugs related to this elsewhere in the
> compiler :-)
>
> Until 2014 (and documented just days ago ;-) ) all bits of a partial
> integer mode were considered unknown.
All bits or all bits outside of its precision? I hope the latter ;)
> I have looked at a lot of it in
> our code the past weeks, and we still treat it like that in most places.
>
> We now see bootstrap problems if we use POImode in some contexts (that's
> this PR96791). POImode can only live in pairs of VSX registers; taking
> a subreg of POImode that would not be valid on one VSX register is not
> okay.
I guess the same applies to i?86 DImode living in two gpr regs. Or any
multi-reg pseudo. It certainly shouldn't be dependent on whether we're
dealing with a partial integer mode or not.
> Maybe we are missing some hooks or macros?
So this problem must be "solved" in some way already. How do we asses
subreg validity? Through recog in the end?
Richard.
>
>
> Segher
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-14 7:46 ` Richard Biener
@ 2020-09-14 15:47 ` Segher Boessenkool
2020-11-02 20:20 ` Aaron Sawdey
0 siblings, 1 reply; 13+ messages in thread
From: Segher Boessenkool @ 2020-09-14 15:47 UTC (permalink / raw)
To: Richard Biener; +Cc: Aaron Sawdey, gcc-patches, Jakub Jelinek, Bill Schmidt
On Mon, Sep 14, 2020 at 09:46:11AM +0200, Richard Biener wrote:
> On Fri, Sep 11, 2020 at 4:18 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > Until 2014 (and documented just days ago ;-) ) all bits of a partial
> > integer mode were considered unknown.
>
> All bits or all bits outside of its precision? I hope the latter ;)
All bits. Many things in GCC still follow that older definition, btw.
> > I have looked at a lot of it in
> > our code the past weeks, and we still treat it like that in most places.
Oh I said that already, heh.
> > We now see bootstrap problems if we use POImode in some contexts (that's
> > this PR96791). POImode can only live in pairs of VSX registers; taking
> > a subreg of POImode that would not be valid on one VSX register is not
> > okay.
>
> I guess the same applies to i?86 DImode living in two gpr regs. Or any
> multi-reg pseudo. It certainly shouldn't be dependent on whether we're
> dealing with a partial integer mode or not.
If some mode can be in GPRs, then taking subregs of it works fine.
> > Maybe we are missing some hooks or macros?
>
> So this problem must be "solved" in some way already. How do we asses
> subreg validity? Through recog in the end?
No, we ICE. See the PR? (PR96791).
Segher
^ permalink raw reply [flat|nested] 13+ messages in thread
* Ping: [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-09 18:27 [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes Aaron Sawdey
2020-09-10 9:32 ` Richard Sandiford
2020-09-10 9:36 ` Richard Biener
@ 2020-10-05 14:39 ` Aaron Sawdey
2 siblings, 0 replies; 13+ messages in thread
From: Aaron Sawdey @ 2020-10-05 14:39 UTC (permalink / raw)
To: gcc-patches
Cc: Segher Boessenkool, Bill Schmidt, Richard Sandiford,
Jakub Jelinek, Richard Biener
Not exactly a patch ping, but I was hoping we could re-engage the discussion on this and figure out how we can make POImode work for powerpc.
How does x86 solve this? There was some suggestion that it has some similar situations?
Thanks,
Aaron Sawdey, Ph.D. sawdey@linux.ibm.com
IBM Linux on POWER Toolchain
> On Sep 9, 2020, at 1:27 PM, Aaron Sawdey <acsawdey@linux.ibm.com> wrote:
>
> Now that the documentation for partial modes says they have a known
> number of bits of precision, would it make sense for extract_low_bits to
> check this before attempting to extract the bits?
>
> This would solve the problem we have been having with POImode and
> extract_low_bits -- DSE tries to use it to extract part of a POImode
> register used in a previous store. We do not want to supply any patterns
> to make POImode (or OImode) used like a regular integer mode.
>
> This patch adds such a check, and sets the precision of POImode to one
> bit, which resolves the problems of PR/96791 for ppc64 target.
>
> Bootstrap passes on ppc64le and x86_64.
>
> Thanks,
> Aaron
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000-modes.def (POImode): Change precision.
> * expmed.c (extract_low_bits): Check precision.
> ---
> gcc/config/rs6000/rs6000-modes.def | 2 +-
> gcc/expmed.c | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/rs6000/rs6000-modes.def b/gcc/config/rs6000/rs6000-modes.def
> index ddb218b3fba..aa7d60dd835 100644
> --- a/gcc/config/rs6000/rs6000-modes.def
> +++ b/gcc/config/rs6000/rs6000-modes.def
> @@ -90,5 +90,5 @@ INT_MODE (OI, 32);
> INT_MODE (XI, 64);
>
> /* Modes used by __vector_pair and __vector_quad. */
> -PARTIAL_INT_MODE (OI, 256, POI); /* __vector_pair. */
> +PARTIAL_INT_MODE (OI, 1, POI); /* __vector_pair. */
> PARTIAL_INT_MODE (XI, 512, PXI); /* __vector_quad. */
> diff --git a/gcc/expmed.c b/gcc/expmed.c
> index d34f0fb0b54..23ca181afa6 100644
> --- a/gcc/expmed.c
> +++ b/gcc/expmed.c
> @@ -2396,6 +2396,9 @@ extract_low_bits (machine_mode mode, machine_mode src_mode, rtx src)
> if (GET_MODE_CLASS (mode) == MODE_CC || GET_MODE_CLASS (src_mode) == MODE_CC)
> return NULL_RTX;
>
> + if (known_lt (GET_MODE_PRECISION (src_mode), GET_MODE_BITSIZE (mode)))
> + return NULL_RTX;
> +
> if (known_eq (GET_MODE_BITSIZE (mode), GET_MODE_BITSIZE (src_mode))
> && targetm.modes_tieable_p (mode, src_mode))
> {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes
2020-09-14 15:47 ` Segher Boessenkool
@ 2020-11-02 20:20 ` Aaron Sawdey
0 siblings, 0 replies; 13+ messages in thread
From: Aaron Sawdey @ 2020-11-02 20:20 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Richard Biener, gcc-patches, Jakub Jelinek, Bill Schmidt,
Richard Sandiford
Ping.
So, this has sat for a while and it’s getting close to the end of stage1 now. I don’t see that we're any closer to a solution that allows us to use POImode without risking this ICE. I had to disable the use of VSX vector pair loads/stores in inline expansion of memcpy/memmove do avoid it. There is no solution like that for the MMA builtins that use POImode and are (in theory) exposed to the same problem.
So I ask again, how can we tell extract_low_bits() that POImode is off limits to its prying fingers?
Thanks,
Aaron
Aaron Sawdey, Ph.D. sawdey@linux.ibm.com
IBM Linux on POWER Toolchain
> On Sep 14, 2020, at 10:47 AM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>
> On Mon, Sep 14, 2020 at 09:46:11AM +0200, Richard Biener wrote:
>> On Fri, Sep 11, 2020 at 4:18 PM Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>>> Until 2014 (and documented just days ago ;-) ) all bits of a partial
>>> integer mode were considered unknown.
>>
>> All bits or all bits outside of its precision? I hope the latter ;)
>
> All bits. Many things in GCC still follow that older definition, btw.
>
>>> I have looked at a lot of it in
>>> our code the past weeks, and we still treat it like that in most places.
>
> Oh I said that already, heh.
>
>>> We now see bootstrap problems if we use POImode in some contexts (that's
>>> this PR96791). POImode can only live in pairs of VSX registers; taking
>>> a subreg of POImode that would not be valid on one VSX register is not
>>> okay.
>>
>> I guess the same applies to i?86 DImode living in two gpr regs. Or any
>> multi-reg pseudo. It certainly shouldn't be dependent on whether we're
>> dealing with a partial integer mode or not.
>
> If some mode can be in GPRs, then taking subregs of it works fine.
>
>>> Maybe we are missing some hooks or macros?
>>
>> So this problem must be "solved" in some way already. How do we asses
>> subreg validity? Through recog in the end?
>
> No, we ICE. See the PR? (PR96791).
>
>
> Segher
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-11-02 20:20 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09 18:27 [PATCH] [PATCH] PR rtl-optimization/96791 Check precision of partial modes Aaron Sawdey
2020-09-10 9:32 ` Richard Sandiford
2020-09-10 9:36 ` Richard Biener
2020-09-10 14:22 ` Aaron Sawdey
2020-09-10 14:33 ` Richard Biener
2020-09-10 15:10 ` Segher Boessenkool
2020-09-10 16:11 ` Aaron Sawdey
2020-09-11 6:07 ` Richard Biener
2020-09-11 14:16 ` Segher Boessenkool
2020-09-14 7:46 ` Richard Biener
2020-09-14 15:47 ` Segher Boessenkool
2020-11-02 20:20 ` Aaron Sawdey
2020-10-05 14:39 ` Ping: " Aaron Sawdey
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).