Re: [PATCH][ARM][gcc-7] Fix wrong code by arm_final_prescan with fp16 move instructions

[Sudakshina Das]

Hi Christophe

On 12/12/17 09:59, Christophe Lyon wrote:

Hi,



On 11 December 2017 at 18:12, Sudakshina Das <sudi....@arm.com> wrote:

On 30/11/17 16:01, Sudakshina Das wrote:

Hi

This patch is the fix for gcc-7 for the same issue as mentioned in:
https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02209.html


For the following test case:
__fp16
test_select (__fp16 a, __fp16 b, __fp16 c)
{
    return (a < b) ? b : c;
}

when compiled with -O2 -mfpu=fp-armv8 -march=armv8.2-a+fp16 -marm
-mfloat-abi=hard generates wrong code:

test_select:
      @ args = 0, pretend = 0, frame = 0
      @ frame_needed = 0, uses_anonymous_args = 0
      @ link register save eliminated.
      vcvtb.f32.f16    s0, s0
      vcvtb.f32.f16    s15, s1
      vmov.f16    r3, s2    @ __fp16
      vcmpe.f32    s0, s15
      vmrs    APSR_nzcv, FPSCR
          // <------ No conditional branch
      vmov.f16    r3, s1    @ __fp16
.L1:
      vmov.f16    s0, r3    @ __fp16
      bx    lr

There should have been a conditional branch there to skip one of the
VMOVs.
This patch fixes this problem by making *movhf_vfp_fp16 unconditional.

Testing done: Add a new test case and checked for regressions on
bootstrapped arm-none-linux-gnueabihf.

Is this ok for gcc-7?

Sudi

ChangeLog entry are as follow:

*** gcc/ChangeLog ***

2017-11-30  Sudakshina Das  <sudi....@arm.com>

      * config/arm/vfp.md (*movhf_vfp_fp16): Add conds attribute.

*** gcc/testsuite/ChangeLog ***

2017-11-30  Sudakshina Das  <sudi....@arm.com>

      * gcc.target/arm/armv8_2-fp16-move-2.c: New test.

As per the trunk thread for this
(https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02209.html) committed as
r255536 on gcc-7-branch for the backport.

I've noticed that this backport fails on arm-none-linux-gnueabi and
arm-none-eabi.
I suspect this is partly due to the fact that I use a "recent"
dejagnu, and has to do with whether
dg-add-options are appended or pre-pended.
I'm seeing a compilation line with:

-mfpu=fp-armv8 -march=armv8.2-a+fp16 -marm -mfloat-abi=hard
-mfpu=fp-armv8 -mfloat-abi=softfp -march=armv8.2-a+fp16

leading to:
FAIL: gcc.target/arm/armv8_2-fp16-move-2.c scan-assembler bpl

I'm not sure why this works on trunk, but there I have only:
-marm -mfloat-abi=softfp -march=armv8.2-a+fp16

Maybe this has to do with the new way cpu/fpu options are parsed on trunk.

Sorry for this. I will try to investigate.

Thanks
Sudi

Christophe

Thanks
Sudi