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

[Sudakshina Das <sudi.das@arm.com>]

Hi Christophe

On 12/12/17 09:59, Christophe Lyon wrote:
> Hi,
> 
> 
> 
> On 11 December 2017 at 18:12, Sudakshina Das <sudi.das@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.das@arm.com>
>>>
>>>       * config/arm/vfp.md (*movhf_vfp_fp16): Add conds attribute.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2017-11-30  Sudakshina Das  <sudi.das@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