Re: [PATCH][GCC][ARM] Fix can_change_mode_class for big-endian

[Christophe Lyon <christophe dot lyon at linaro dot org>]

On 15 March 2018 at 11:19, Kyrill  Tkachov <kyrylo.tkachov@foss.arm.com> wrote:
> Hi Tamar,
>
>
> On 05/03/18 16:51, Tamar Christina wrote:
>>
>> Hi All,
>>
>> 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 7?
>>
>
> Ok for trunk.
> Please wait for a few days before backporting.
>

Hi Tamar,

Strangely I have noticed regressions on armeb, I have updated bugzilla
accordingly.

Thanks,

Christophe

> Thanks,
> Kyrill
>
>
>> Thanks,
>> Tamar
>>
>> gcc/
>> 2018-03-05  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-03-05  Tamar Christina  <tamar.christina@arm.com>
>>
>>         PR target/84711
>>         * gcc.target/arm/big-endian-subreg.c: New.
>>
>> --
>
>