diff mbox series

PR rtl-optimization/96791 Check precision of partial modes

Message ID 20200909182723.46246-1-acsawdey@linux.ibm.com
State New
Headers show
Series PR rtl-optimization/96791 Check precision of partial modes | expand

Commit Message

Aaron Sawdey Sept. 9, 2020, 6:27 p.m. UTC
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(-)

Comments

Richard Sandiford Sept. 10, 2020, 9:32 a.m. UTC | #1
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
Richard Biener Sept. 10, 2020, 9:36 a.m. UTC | #2
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
>
Aaron Sawdey Sept. 10, 2020, 2:22 p.m. UTC | #3
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
>>
Richard Biener Sept. 10, 2020, 2:33 p.m. UTC | #4
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
> >>
>
Segher Boessenkool Sept. 10, 2020, 3:10 p.m. UTC | #5
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
Aaron Sawdey Sept. 10, 2020, 4:11 p.m. UTC | #6
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
Richard Biener Sept. 11, 2020, 6:07 a.m. UTC | #7
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
Segher Boessenkool Sept. 11, 2020, 2:16 p.m. UTC | #8
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
Richard Biener Sept. 14, 2020, 7:46 a.m. UTC | #9
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
Segher Boessenkool Sept. 14, 2020, 3:47 p.m. UTC | #10
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
Aaron Sawdey Oct. 5, 2020, 2:39 p.m. UTC | #11
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
>
Aaron Sawdey Nov. 2, 2020, 8:20 p.m. UTC | #12
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
diff mbox series

Patch

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))
     {