diff mbox series

[ARM] Fix can_change_mode_class for big-endian

Message ID 20180619141354.GA28904@arm.com
State New
Headers show
Series [ARM] Fix can_change_mode_class for big-endian | expand

Commit Message

Tamar Christina June 19, 2018, 2:14 p.m. UTC 
Hi All,

This patch requires https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01145.html to work,
it has been accepted once already but caused a regression on certain configuratoins.
I am re-submitting it with the required mid-end change and requesting a back-port.

--- original patch.

Taking the subreg of a vector mode on big-endian may result in an infinite
recursion and eventually a segfault once we run out of stack space.

As an example, taking a subreg of V4HF to SImode we end up in the following
loop on big-endian:

#861 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
#862 0x0000000000882a90 in emit_move_multi_word src/gcc/gcc/expr.c:3621
#863 0x000000000087eea1 in emit_move_insn_1 src/gcc/gcc/expr.c:3698
#864 0x000000000087f350 in emit_move_insn src/gcc/gcc/expr.c:3757
#865 0x000000000085e326 in copy_to_reg src/gcc/gcc/explow.c:603
#866 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787

The reason is that operand_subword_force will always fail. When the value is in
a register that can't be accessed as a multi word the code tries to create a new
psuedo register and emit the value to it. Eventually you end up in simplify_gen_subreg
which calls validate_subreg.

validate_subreg will however always fail because of the REG_CAN_CHANGE_MODE_P check.

On little endian this check always returns true. On big-endian this check is supposed
to prevent values that have a size larger than word size, due to those being stored in
VFP registers.

However we are only interested in a subreg of the vector mode, so we should be checking
the unit size, not the size of the entire mode. Doing this fixes the problem.

Regtested on armeb-none-eabi and no regressions.
Bootstrapped on arm-none-linux-gnueabihf and no issues.

Ok for trunk? and for backport to GCC 8?

Thanks,
Tamar

gcc/
2018-06-19  Tamar Christina  <tamar.christina@arm.com>

	PR target/84711
	* config/arm/arm.c (arm_can_change_mode_class): Use GET_MODE_UNIT_SIZE
	instead of GET_MODE_SIZE when comparing Units.
	
gcc/testsuite/
2018-06-19  Tamar Christina  <tamar.christina@arm.com>

	PR target/84711
	* gcc.target/arm/big-endian-subreg.c: New.

--

Comments

Kyrill Tkachov June 20, 2018, 8:43 a.m. UTC | #1 
Hi Tamar,

On 19/06/18 15:14, Tamar Christina wrote:
> Hi All,
>
> This patch requires https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01145.html to work,
> it has been accepted once already but caused a regression on certain configuratoins.
> I am re-submitting it with the required mid-end change and requesting a back-port.
>
> --- original patch.
>
> Taking the subreg of a vector mode on big-endian may result in an infinite
> recursion and eventually a segfault once we run out of stack space.
>
> As an example, taking a subreg of V4HF to SImode we end up in the following
> loop on big-endian:
>
> #861 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
> #862 0x0000000000882a90 in emit_move_multi_word src/gcc/gcc/expr.c:3621
> #863 0x000000000087eea1 in emit_move_insn_1 src/gcc/gcc/expr.c:3698
> #864 0x000000000087f350 in emit_move_insn src/gcc/gcc/expr.c:3757
> #865 0x000000000085e326 in copy_to_reg src/gcc/gcc/explow.c:603
> #866 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
>
> The reason is that operand_subword_force will always fail. When the value is in
> a register that can't be accessed as a multi word the code tries to create a new
> psuedo register and emit the value to it. Eventually you end up in simplify_gen_subreg
> which calls validate_subreg.
>
> validate_subreg will however always fail because of the REG_CAN_CHANGE_MODE_P check.
>
> On little endian this check always returns true. On big-endian this check is supposed
> to prevent values that have a size larger than word size, due to those being stored in
> VFP registers.
>
> However we are only interested in a subreg of the vector mode, so we should be checking
> the unit size, not the size of the entire mode. Doing this fixes the problem.
>
> Regtested on armeb-none-eabi and no regressions.
> Bootstrapped on arm-none-linux-gnueabihf and no issues.
>
> Ok for trunk? and for backport to GCC 8?
>
> Thanks,
> Tamar
>
> gcc/
> 2018-06-19  Tamar Christina  <tamar.christina@arm.com>
>
>         PR target/84711
>         * config/arm/arm.c (arm_can_change_mode_class): Use GET_MODE_UNIT_SIZE
>         instead of GET_MODE_SIZE when comparing Units.
>
> gcc/testsuite/
> 2018-06-19  Tamar Christina  <tamar.christina@arm.com>
>
>         PR target/84711
>         * gcc.target/arm/big-endian-subreg.c: New.
>
> -- 


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 90d62e699bce9594879be2e3016c9b36c7e064c8..703632240822e762a906570964b949c783df56f3 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -31508,8 +31508,8 @@ arm_can_change_mode_class (machine_mode from, machine_mode to,
  {
    if (TARGET_BIG_END
        && !(GET_MODE_SIZE (from) == 16 && GET_MODE_SIZE (to) == 8)
-      && (GET_MODE_SIZE (from) > UNITS_PER_WORD
-	  || GET_MODE_SIZE (to) > UNITS_PER_WORD)
+      && (GET_MODE_UNIT_SIZE (from) > UNITS_PER_WORD
+	  || GET_MODE_UNIT_SIZE (to) > UNITS_PER_WORD)

Does GET_MODE_UNIT_SIZE do what you want? Its documentation in rtl.texi says:
"Returns the size in bytes of the subunits of a datum of mode @var{m}.
  This is the same as @code{GET_MODE_SIZE} except in the case of complex
  modes.  For them, the unit size is the size of the real or imaginary
  part."

Does it also do the right thing for vector modes (GET_MODE_SIZE (GET_MODE_INNER (mode))) ?
If so, this patch is ok, but we'll need to update the documentation to make it more explicit.

Thanks for the patch,
Kyrill
Tamar Christina June 20, 2018, 10:33 a.m. UTC | #2 
Hi Kyrill,

Many thanks for the review!

The 06/20/2018 09:43, Kyrill Tkachov wrote:
> Hi Tamar,
> 
> On 19/06/18 15:14, Tamar Christina wrote:
> > Hi All,
> >
> > This patch requires https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01145.html to work,
> > it has been accepted once already but caused a regression on certain configuratoins.
> > I am re-submitting it with the required mid-end change and requesting a back-port.
> >
> > --- original patch.
> >
> > Taking the subreg of a vector mode on big-endian may result in an infinite
> > recursion and eventually a segfault once we run out of stack space.
> >
> > As an example, taking a subreg of V4HF to SImode we end up in the following
> > loop on big-endian:
> >
> > #861 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
> > #862 0x0000000000882a90 in emit_move_multi_word src/gcc/gcc/expr.c:3621
> > #863 0x000000000087eea1 in emit_move_insn_1 src/gcc/gcc/expr.c:3698
> > #864 0x000000000087f350 in emit_move_insn src/gcc/gcc/expr.c:3757
> > #865 0x000000000085e326 in copy_to_reg src/gcc/gcc/explow.c:603
> > #866 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
> >
> > The reason is that operand_subword_force will always fail. When the value is in
> > a register that can't be accessed as a multi word the code tries to create a new
> > psuedo register and emit the value to it. Eventually you end up in simplify_gen_subreg
> > which calls validate_subreg.
> >
> > validate_subreg will however always fail because of the REG_CAN_CHANGE_MODE_P check.
> >
> > On little endian this check always returns true. On big-endian this check is supposed
> > to prevent values that have a size larger than word size, due to those being stored in
> > VFP registers.
> >
> > However we are only interested in a subreg of the vector mode, so we should be checking
> > the unit size, not the size of the entire mode. Doing this fixes the problem.
> >
> > Regtested on armeb-none-eabi and no regressions.
> > Bootstrapped on arm-none-linux-gnueabihf and no issues.
> >
> > Ok for trunk? and for backport to GCC 8?
> >
> > Thanks,
> > Tamar
> >
> > gcc/
> > 2018-06-19  Tamar Christina  <tamar.christina@arm.com>
> >
> >         PR target/84711
> >         * config/arm/arm.c (arm_can_change_mode_class): Use GET_MODE_UNIT_SIZE
> >         instead of GET_MODE_SIZE when comparing Units.
> >
> > gcc/testsuite/
> > 2018-06-19  Tamar Christina  <tamar.christina@arm.com>
> >
> >         PR target/84711
> >         * gcc.target/arm/big-endian-subreg.c: New.
> >
> > -- 
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 90d62e699bce9594879be2e3016c9b36c7e064c8..703632240822e762a906570964b949c783df56f3 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -31508,8 +31508,8 @@ arm_can_change_mode_class (machine_mode from, machine_mode to,
>   {
>     if (TARGET_BIG_END
>         && !(GET_MODE_SIZE (from) == 16 && GET_MODE_SIZE (to) == 8)
> -      && (GET_MODE_SIZE (from) > UNITS_PER_WORD
> -	  || GET_MODE_SIZE (to) > UNITS_PER_WORD)
> +      && (GET_MODE_UNIT_SIZE (from) > UNITS_PER_WORD
> +	  || GET_MODE_UNIT_SIZE (to) > UNITS_PER_WORD)
> 
> Does GET_MODE_UNIT_SIZE do what you want? Its documentation in rtl.texi says:
> "Returns the size in bytes of the subunits of a datum of mode @var{m}.
>   This is the same as @code{GET_MODE_SIZE} except in the case of complex
>   modes.  For them, the unit size is the size of the real or imaginary
>   part."
> 
> Does it also do the right thing for vector modes (GET_MODE_SIZE (GET_MODE_INNER (mode))) ?
> If so, this patch is ok, but we'll need to update the documentation to make it more explicit.

I don't know what the documentation is trying to say here, but the key is the first part:

"returns the size in bytes of the subunits of a datum of mode m"

MODE: V4SI, GET_MODE_UNIT_SIZE: 4, GET_MODE_SIZE: 16

So GET_MODE_UNIT_SIZE (m) * GET_MODE_NUNITS(m) == GET_MODE_SIZE (m)

or GET_MODE_UNIT_SIZE (m) == GET_MODE_SIZE (GET_MODE_INNER (mode)).

From this the only time GET_MODE_UNIT_SIZE is equal to GET_MODE_SIZE is on
non-vector modes of V1 vector modes.

Kind Regards,
Tamar

> 
> Thanks for the patch,
> Kyrill
> 
> 
> 

--
Kyrill Tkachov June 20, 2018, 1:35 p.m. UTC | #3 
On 20/06/18 11:33, Tamar Christina wrote:
> Hi Kyrill,
>
> Many thanks for the review!
>
> The 06/20/2018 09:43, Kyrill Tkachov wrote:
>> Hi Tamar,
>>
>> On 19/06/18 15:14, Tamar Christina wrote:
>>> Hi All,
>>>
>>> This patch requires https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01145.html to work,
>>> it has been accepted once already but caused a regression on certain configuratoins.
>>> I am re-submitting it with the required mid-end change and requesting a back-port.
>>>
>>> --- original patch.
>>>
>>> Taking the subreg of a vector mode on big-endian may result in an infinite
>>> recursion and eventually a segfault once we run out of stack space.
>>>
>>> As an example, taking a subreg of V4HF to SImode we end up in the following
>>> loop on big-endian:
>>>
>>> #861 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
>>> #862 0x0000000000882a90 in emit_move_multi_word src/gcc/gcc/expr.c:3621
>>> #863 0x000000000087eea1 in emit_move_insn_1 src/gcc/gcc/expr.c:3698
>>> #864 0x000000000087f350 in emit_move_insn src/gcc/gcc/expr.c:3757
>>> #865 0x000000000085e326 in copy_to_reg src/gcc/gcc/explow.c:603
>>> #866 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
>>>
>>> The reason is that operand_subword_force will always fail. When the value is in
>>> a register that can't be accessed as a multi word the code tries to create a new
>>> psuedo register and emit the value to it. Eventually you end up in simplify_gen_subreg
>>> which calls validate_subreg.
>>>
>>> validate_subreg will however always fail because of the REG_CAN_CHANGE_MODE_P check.
>>>
>>> On little endian this check always returns true. On big-endian this check is supposed
>>> to prevent values that have a size larger than word size, due to those being stored in
>>> VFP registers.
>>>
>>> However we are only interested in a subreg of the vector mode, so we should be checking
>>> the unit size, not the size of the entire mode. Doing this fixes the problem.
>>>
>>> Regtested on armeb-none-eabi and no regressions.
>>> Bootstrapped on arm-none-linux-gnueabihf and no issues.
>>>
>>> Ok for trunk? and for backport to GCC 8?
>>>
>>> Thanks,
>>> Tamar
>>>
>>> gcc/
>>> 2018-06-19  Tamar Christina  <tamar.christina@arm.com>
>>>
>>>          PR target/84711
>>>          * config/arm/arm.c (arm_can_change_mode_class): Use GET_MODE_UNIT_SIZE
>>>          instead of GET_MODE_SIZE when comparing Units.
>>>
>>> gcc/testsuite/
>>> 2018-06-19  Tamar Christina  <tamar.christina@arm.com>
>>>
>>>          PR target/84711
>>>          * gcc.target/arm/big-endian-subreg.c: New.
>>>
>>> -- 
>>
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index 90d62e699bce9594879be2e3016c9b36c7e064c8..703632240822e762a906570964b949c783df56f3 100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -31508,8 +31508,8 @@ arm_can_change_mode_class (machine_mode from, machine_mode to,
>>    {
>>      if (TARGET_BIG_END
>>          && !(GET_MODE_SIZE (from) == 16 && GET_MODE_SIZE (to) == 8)
>> -      && (GET_MODE_SIZE (from) > UNITS_PER_WORD
>> -	  || GET_MODE_SIZE (to) > UNITS_PER_WORD)
>> +      && (GET_MODE_UNIT_SIZE (from) > UNITS_PER_WORD
>> +	  || GET_MODE_UNIT_SIZE (to) > UNITS_PER_WORD)
>>
>> Does GET_MODE_UNIT_SIZE do what you want? Its documentation in rtl.texi says:
>> "Returns the size in bytes of the subunits of a datum of mode @var{m}.
>>    This is the same as @code{GET_MODE_SIZE} except in the case of complex
>>    modes.  For them, the unit size is the size of the real or imaginary
>>    part."
>>
>> Does it also do the right thing for vector modes (GET_MODE_SIZE (GET_MODE_INNER (mode))) ?
>> If so, this patch is ok, but we'll need to update the documentation to make it more explicit.
> I don't know what the documentation is trying to say here, but the key is the first part:
>
> "returns the size in bytes of the subunits of a datum of mode m"
>
> MODE: V4SI, GET_MODE_UNIT_SIZE: 4, GET_MODE_SIZE: 16
>
> So GET_MODE_UNIT_SIZE (m) * GET_MODE_NUNITS(m) == GET_MODE_SIZE (m)
>
> or GET_MODE_UNIT_SIZE (m) == GET_MODE_SIZE (GET_MODE_INNER (mode)).
>
>  From this the only time GET_MODE_UNIT_SIZE is equal to GET_MODE_SIZE is on
> non-vector modes of V1 vector modes.

Right, this is what I thought, but the documentation is not as clear as it could be.
Your patch is ok for trunk.

Thanks,
Kyrill

> Kind Regards,
> Tamar
>
>> Thanks for the patch,
>> Kyrill
>>
>>
>>
Christophe Lyon July 6, 2018, 7:58 a.m. UTC | #4 
Hi Tamar,

On Wed, 20 Jun 2018 at 15:35, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
>
>
> On 20/06/18 11:33, Tamar Christina wrote:
> > Hi Kyrill,
> >
> > Many thanks for the review!
> >
> > The 06/20/2018 09:43, Kyrill Tkachov wrote:
> >> Hi Tamar,
> >>
> >> On 19/06/18 15:14, Tamar Christina wrote:
> >>> Hi All,
> >>>
> >>> This patch requires https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01145.html to work,
> >>> it has been accepted once already but caused a regression on certain configuratoins.
> >>> I am re-submitting it with the required mid-end change and requesting a back-port.
> >>>
> >>> --- original patch.
> >>>
> >>> Taking the subreg of a vector mode on big-endian may result in an infinite
> >>> recursion and eventually a segfault once we run out of stack space.
> >>>
> >>> As an example, taking a subreg of V4HF to SImode we end up in the following
> >>> loop on big-endian:
> >>>
> >>> #861 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
> >>> #862 0x0000000000882a90 in emit_move_multi_word src/gcc/gcc/expr.c:3621
> >>> #863 0x000000000087eea1 in emit_move_insn_1 src/gcc/gcc/expr.c:3698
> >>> #864 0x000000000087f350 in emit_move_insn src/gcc/gcc/expr.c:3757
> >>> #865 0x000000000085e326 in copy_to_reg src/gcc/gcc/explow.c:603
> >>> #866 0x00000000008462e9 in operand_subword_force src/gcc/gcc/emit-rtl.c:1787
> >>>
> >>> The reason is that operand_subword_force will always fail. When the value is in
> >>> a register that can't be accessed as a multi word the code tries to create a new
> >>> psuedo register and emit the value to it. Eventually you end up in simplify_gen_subreg
> >>> which calls validate_subreg.
> >>>
> >>> validate_subreg will however always fail because of the REG_CAN_CHANGE_MODE_P check.
> >>>
> >>> On little endian this check always returns true. On big-endian this check is supposed
> >>> to prevent values that have a size larger than word size, due to those being stored in
> >>> VFP registers.
> >>>
> >>> However we are only interested in a subreg of the vector mode, so we should be checking
> >>> the unit size, not the size of the entire mode. Doing this fixes the problem.
> >>>
> >>> Regtested on armeb-none-eabi and no regressions.
> >>> Bootstrapped on arm-none-linux-gnueabihf and no issues.
> >>>
> >>> Ok for trunk? and for backport to GCC 8?
> >>>
> >>> Thanks,
> >>> Tamar
> >>>
> >>> gcc/
> >>> 2018-06-19  Tamar Christina  <tamar.christina@arm.com>
> >>>
> >>>          PR target/84711
> >>>          * config/arm/arm.c (arm_can_change_mode_class): Use GET_MODE_UNIT_SIZE
> >>>          instead of GET_MODE_SIZE when comparing Units.
> >>>
> >>> gcc/testsuite/
> >>> 2018-06-19  Tamar Christina  <tamar.christina@arm.com>
> >>>
> >>>          PR target/84711
> >>>          * gcc.target/arm/big-endian-subreg.c: New.
> >>>
> >>> --
> >>
> >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> >> index 90d62e699bce9594879be2e3016c9b36c7e064c8..703632240822e762a906570964b949c783df56f3 100644
> >> --- a/gcc/config/arm/arm.c
> >> +++ b/gcc/config/arm/arm.c
> >> @@ -31508,8 +31508,8 @@ arm_can_change_mode_class (machine_mode from, machine_mode to,
> >>    {
> >>      if (TARGET_BIG_END
> >>          && !(GET_MODE_SIZE (from) == 16 && GET_MODE_SIZE (to) == 8)
> >> -      && (GET_MODE_SIZE (from) > UNITS_PER_WORD
> >> -      || GET_MODE_SIZE (to) > UNITS_PER_WORD)
> >> +      && (GET_MODE_UNIT_SIZE (from) > UNITS_PER_WORD
> >> +      || GET_MODE_UNIT_SIZE (to) > UNITS_PER_WORD)
> >>

After this commit (r262436), I have noticed regressions on
armeb-none-linux-gnueabihf
--with-cpu cortex-a9
--with-fpu neon-fp16
FAIL: gcc.dg/vect/vect-nop-move.c execution test
FAIL: g++.dg/torture/vshuf-v2si.C   -O3 -g  execution test
FAIL: g++.dg/torture/vshuf-v4si.C   -O3 -g  execution test
FAIL: g++.dg/torture/vshuf-v8hi.C   -O3 -g  execution test

Can you have a look?

> >> Does GET_MODE_UNIT_SIZE do what you want? Its documentation in rtl.texi says:
> >> "Returns the size in bytes of the subunits of a datum of mode @var{m}.
> >>    This is the same as @code{GET_MODE_SIZE} except in the case of complex
> >>    modes.  For them, the unit size is the size of the real or imaginary
> >>    part."
> >>
> >> Does it also do the right thing for vector modes (GET_MODE_SIZE (GET_MODE_INNER (mode))) ?
> >> If so, this patch is ok, but we'll need to update the documentation to make it more explicit.
> > I don't know what the documentation is trying to say here, but the key is the first part:
> >
> > "returns the size in bytes of the subunits of a datum of mode m"
> >
> > MODE: V4SI, GET_MODE_UNIT_SIZE: 4, GET_MODE_SIZE: 16
> >
> > So GET_MODE_UNIT_SIZE (m) * GET_MODE_NUNITS(m) == GET_MODE_SIZE (m)
> >
> > or GET_MODE_UNIT_SIZE (m) == GET_MODE_SIZE (GET_MODE_INNER (mode)).
> >
> >  From this the only time GET_MODE_UNIT_SIZE is equal to GET_MODE_SIZE is on
> > non-vector modes of V1 vector modes.
>
> Right, this is what I thought, but the documentation is not as clear as it could be.
> Your patch is ok for trunk.
>
> Thanks,
> Kyrill
>
> > Kind Regards,
> > Tamar
> >
> >> Thanks for the patch,
> >> Kyrill
> >>
> >>
> >>
>
Tamar Christina July 6, 2018, 1:24 p.m. UTC | #5 
Hi Christoph,

> 90d62e699bce9594879be2e3016c9b36c7e064c8..703632240822e762a90657096
> > >> 4b949c783df56f3 100644
> > >> --- a/gcc/config/arm/arm.c
> > >> +++ b/gcc/config/arm/arm.c
> > >> @@ -31508,8 +31508,8 @@ arm_can_change_mode_class
> (machine_mode from, machine_mode to,
> > >>    {
> > >>      if (TARGET_BIG_END
> > >>          && !(GET_MODE_SIZE (from) == 16 && GET_MODE_SIZE (to) == 8)
> > >> -      && (GET_MODE_SIZE (from) > UNITS_PER_WORD
> > >> -      || GET_MODE_SIZE (to) > UNITS_PER_WORD)
> > >> +      && (GET_MODE_UNIT_SIZE (from) > UNITS_PER_WORD
> > >> +      || GET_MODE_UNIT_SIZE (to) > UNITS_PER_WORD)
> > >>
> 
> After this commit (r262436), I have noticed regressions on armeb-none-linux-
> gnueabihf --with-cpu cortex-a9 --with-fpu neon-fp16
> FAIL: gcc.dg/vect/vect-nop-move.c execution test
> FAIL: g++.dg/torture/vshuf-v2si.C   -O3 -g  execution test
> FAIL: g++.dg/torture/vshuf-v4si.C   -O3 -g  execution test
> FAIL: g++.dg/torture/vshuf-v8hi.C   -O3 -g  execution test
> 
> Can you have a look?

Sorry! I know you reported these before so I did explicitly test them, but it turns
out a configuration issue in our scripts was causing armeb linux builds to skip
execution tests.  So the execution parts of these tests never showed up and so the
regression was clean.

I know what's wrong and will submit a patch soon.

Thanks,
Tamar

> 
> > >> Does GET_MODE_UNIT_SIZE do what you want? Its documentation in
> rtl.texi says:
> > >> "Returns the size in bytes of the subunits of a datum of mode @var{m}.
> > >>    This is the same as @code{GET_MODE_SIZE} except in the case of
> complex
> > >>    modes.  For them, the unit size is the size of the real or imaginary
> > >>    part."
> > >>
> > >> Does it also do the right thing for vector modes (GET_MODE_SIZE
> (GET_MODE_INNER (mode))) ?
> > >> If so, this patch is ok, but we'll need to update the documentation to
> make it more explicit.
> > > I don't know what the documentation is trying to say here, but the key is
> the first part:
> > >
> > > "returns the size in bytes of the subunits of a datum of mode m"
> > >
> > > MODE: V4SI, GET_MODE_UNIT_SIZE: 4, GET_MODE_SIZE: 16
> > >
> > > So GET_MODE_UNIT_SIZE (m) * GET_MODE_NUNITS(m) ==
> GET_MODE_SIZE (m)
> > >
> > > or GET_MODE_UNIT_SIZE (m) == GET_MODE_SIZE (GET_MODE_INNER
> (mode)).
> > >
> > >  From this the only time GET_MODE_UNIT_SIZE is equal to
> > > GET_MODE_SIZE is on non-vector modes of V1 vector modes.
> >
> > Right, this is what I thought, but the documentation is not as clear as it
> could be.
> > Your patch is ok for trunk.
> >
> > Thanks,
> > Kyrill
> >
> > > Kind Regards,
> > > Tamar
> > >
> > >> Thanks for the patch,
> > >> Kyrill
> > >>
> > >>
> > >>
> >
diff mbox series

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 90d62e699bce9594879be2e3016c9b36c7e064c8..703632240822e762a906570964b949c783df56f3 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -31508,8 +31508,8 @@  arm_can_change_mode_class (machine_mode from, machine_mode to,
 {
   if (TARGET_BIG_END
       && !(GET_MODE_SIZE (from) == 16 && GET_MODE_SIZE (to) == 8)
-      && (GET_MODE_SIZE (from) > UNITS_PER_WORD
-	  || GET_MODE_SIZE (to) > UNITS_PER_WORD)
+      && (GET_MODE_UNIT_SIZE (from) > UNITS_PER_WORD
+	  || GET_MODE_UNIT_SIZE (to) > UNITS_PER_WORD)
       && reg_classes_intersect_p (VFP_REGS, rclass))
     return false;
   return true;
diff --git a/gcc/testsuite/gcc.target/arm/big-endian-subreg.c b/gcc/testsuite/gcc.target/arm/big-endian-subreg.c
new file mode 100644
index 0000000000000000000000000000000000000000..4b1ab122f349e61e296fe3f76030a7a258e55f62
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/big-endian-subreg.c
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-require-effective-target arm_hf_eabi } */
+/* { dg-add-options arm_neon } */
+/* { dg-additional-options "-mfp16-format=ieee -mfloat-abi=hard" } */
+
+typedef __fp16 v4f16
+  __attribute__ ((vector_size (8)));
+
+v4f16 fn1 (v4f16 p)
+{
+  return p;
+}