[PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Sudi Das]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

Hi

This patch fixes the test case armv8_2-fp16-move-1.c for arm-none-linux-gnueabihf where 2 of the scan-assembler directives were failing. We now generate less vmov between core and VFP registers. Thus changing those directives to reflect that.

Is this ok for trunk?
If yes could someone commit it on my behalf?

Sudi


*** gcc/testsuite/ChangeLog ***

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

	* gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov scan-assembler
	directives.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch7679.diff --]
[-- Type: text/x-patch; name="patch7679.diff", Size: 857 bytes --]

diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
index bb4e68f..0ed8560 100644
--- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
+++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
@@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
 /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
 /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
 
-/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 } }  */
-/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }  */
+/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }  */
+/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } }  */
 
 int
 test_compare_1 (__fp16 a, __fp16 b)

Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Kyrill Tkachov]

Hi Sudi,

On 16/11/17 16:37, Sudi Das wrote:
> Hi
>
> This patch fixes the test case armv8_2-fp16-move-1.c for 
> arm-none-linux-gnueabihf where 2 of the scan-assembler directives were 
> failing. We now generate less vmov between core and VFP registers. 
> Thus changing those directives to reflect that.
>
> Is this ok for trunk?
> If yes could someone commit it on my behalf?
>
> Sudi
>
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-11-16  Sudakshina Das  <sudi.das@arm.com>
>
>         * gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov scan-assembler
>         directives.
>

diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
index bb4e68f..0ed8560 100644
--- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
+++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
@@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
  /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
  /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
  
-/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 } }  */
-/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }  */
+/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }  */
+/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } }  */
  
Some of the moves between core and fp registers were the result of inefficient codegen and in hindsight
scanning for them was not very useful. Now that we emit only the required ones I think scanning for the plain
vmovs between two S-registers doesn't test anything useful.
So can you please just remove the second scan-assembler directive here?

Thanks,
Kyrill

Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Sudi Das]

[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]


Hi Kyrill

Thanks I have made the change.

Sudi



From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
Sent: Thursday, November 16, 2017 5:03 PM
To: Sudi Das; gcc-patches@gcc.gnu.org
Cc: nd; Ramana Radhakrishnan; Richard Earnshaw
Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
  

Hi Sudi,

On 16/11/17 16:37, Sudi Das wrote:
> Hi
>
> This patch fixes the test case armv8_2-fp16-move-1.c for 
> arm-none-linux-gnueabihf where 2 of the scan-assembler directives were 
> failing. We now generate less vmov between core and VFP registers. 
> Thus changing those directives to reflect that.
>
> Is this ok for trunk?
> If yes could someone commit it on my behalf?
>
> Sudi
>
>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-11-16  Sudakshina Das  <sudi.das@arm.com>
>
>         * gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov scan-assembler
>         directives.
>

diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
index bb4e68f..0ed8560 100644
--- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
+++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
@@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
  /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
  /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
  
-/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 } }  */
-/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }  */
+/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }  */
+/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } }  */
  
Some of the moves between core and fp registers were the result of inefficient codegen and in hindsight
scanning for them was not very useful. Now that we emit only the required ones I think scanning for the plain
vmovs between two S-registers doesn't test anything useful.
So can you please just remove the second scan-assembler directive here?

Thanks,
Kyrill

    

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch7679.diff --]
[-- Type: text/x-patch; name="patch7679.diff", Size: 784 bytes --]

diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
index bb4e68f..8c0a53c 100644
--- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
+++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
@@ -101,8 +101,7 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
 /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
 /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
 
-/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 } }  */
-/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }  */
+/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }  */
 
 int
 test_compare_1 (__fp16 a, __fp16 b)

Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Kyrill Tkachov]

On 17/11/17 10:45, Sudi Das wrote:
> Hi Kyrill
>
> Thanks I have made the change.

Thanks Sudi, I've committed this on your behalf with r254863.

Kyrill

> Sudi
>
>
>
> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
> Sent: Thursday, November 16, 2017 5:03 PM
> To: Sudi Das; gcc-patches@gcc.gnu.org
> Cc: nd; Ramana Radhakrishnan; Richard Earnshaw
> Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
>    
>
> Hi Sudi,
>
> On 16/11/17 16:37, Sudi Das wrote:
>> Hi
>>
>> This patch fixes the test case armv8_2-fp16-move-1.c for
>> arm-none-linux-gnueabihf where 2 of the scan-assembler directives were
>> failing. We now generate less vmov between core and VFP registers.
>> Thus changing those directives to reflect that.
>>
>> Is this ok for trunk?
>> If yes could someone commit it on my behalf?
>>
>> Sudi
>>
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2017-11-16  Sudakshina Das  <sudi.das@arm.com>
>>
>>           * gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov scan-assembler
>>           directives.
>>
> diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
> index bb4e68f..0ed8560 100644
> --- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
> +++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
> @@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
>    /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
>    /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+, s[0-9]+} 1 } }  */
>    
> -/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 } }  */
> -/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }  */
> +/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }  */
> +/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } }  */
>    
> Some of the moves between core and fp registers were the result of inefficient codegen and in hindsight
> scanning for them was not very useful. Now that we emit only the required ones I think scanning for the plain
> vmovs between two S-registers doesn't test anything useful.
> So can you please just remove the second scan-assembler directive here?
>
> Thanks,
> Kyrill
>
>      

Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Christophe Lyon]

Hi,

On 17 November 2017 at 12:12, Kyrill  Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
>
> On 17/11/17 10:45, Sudi Das wrote:
>>
>> Hi Kyrill
>>
>> Thanks I have made the change.
>
>
> Thanks Sudi, I've committed this on your behalf with r254863.
>
> Kyrill
>
>
>> Sudi
>>
>>
>>
>> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
>> Sent: Thursday, November 16, 2017 5:03 PM
>> To: Sudi Das; gcc-patches@gcc.gnu.org
>> Cc: nd; Ramana Radhakrishnan; Richard Earnshaw
>> Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
>>
>> Hi Sudi,
>>
>> On 16/11/17 16:37, Sudi Das wrote:
>>>
>>> Hi
>>>
>>> This patch fixes the test case armv8_2-fp16-move-1.c for
>>> arm-none-linux-gnueabihf where 2 of the scan-assembler directives were
>>> failing. We now generate less vmov between core and VFP registers.
>>> Thus changing those directives to reflect that.
>>>
>>> Is this ok for trunk?
>>> If yes could someone commit it on my behalf?
>>>
>>> Sudi
>>>
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2017-11-16  Sudakshina Das  <sudi.das@arm.com>
>>>
>>>           * gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov
>>> scan-assembler
>>>           directives.
>>>
>> diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>> b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>> index bb4e68f..0ed8560 100644
>> --- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>> +++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>> @@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
>>    /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+,
>> s[0-9]+} 1 } }  */
>>    /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+,
>> s[0-9]+} 1 } }  */
>>    -/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 }
>> }  */
>> -/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }
>> */
>> +/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }
>> */
>> +/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } }  */
>>    Some of the moves between core and fp registers were the result of
>> inefficient codegen and in hindsight
>> scanning for them was not very useful. Now that we emit only the required
>> ones I think scanning for the plain
>> vmovs between two S-registers doesn't test anything useful.
>> So can you please just remove the second scan-assembler directive here?
>>

You are probably already aware of that: the tests fail on
arm-none-linux-gnueabi/arm-none-eabi
FAIL: gcc.target/arm/armv8_2-fp16-move-1.c scan-assembler-times
vmov\\.f16\\ts[0-9]+, r[0-9]+ 2 (found 38 times)

but this is not a regression, the previous version of the test had the
same problem.

Christophe

>> Thanks,
>> Kyrill
>>
>>
>
>

Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Kyrill Tkachov]

On 20/11/17 14:14, Christophe Lyon wrote:
> Hi,
>
> On 17 November 2017 at 12:12, Kyrill  Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
>> On 17/11/17 10:45, Sudi Das wrote:
>>> Hi Kyrill
>>>
>>> Thanks I have made the change.
>>
>> Thanks Sudi, I've committed this on your behalf with r254863.
>>
>> Kyrill
>>
>>
>>> Sudi
>>>
>>>
>>>
>>> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
>>> Sent: Thursday, November 16, 2017 5:03 PM
>>> To: Sudi Das; gcc-patches@gcc.gnu.org
>>> Cc: nd; Ramana Radhakrishnan; Richard Earnshaw
>>> Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
>>>
>>> Hi Sudi,
>>>
>>> On 16/11/17 16:37, Sudi Das wrote:
>>>> Hi
>>>>
>>>> This patch fixes the test case armv8_2-fp16-move-1.c for
>>>> arm-none-linux-gnueabihf where 2 of the scan-assembler directives were
>>>> failing. We now generate less vmov between core and VFP registers.
>>>> Thus changing those directives to reflect that.
>>>>
>>>> Is this ok for trunk?
>>>> If yes could someone commit it on my behalf?
>>>>
>>>> Sudi
>>>>
>>>>
>>>> *** gcc/testsuite/ChangeLog ***
>>>>
>>>> 2017-11-16  Sudakshina Das  <sudi.das@arm.com>
>>>>
>>>>            * gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov
>>>> scan-assembler
>>>>            directives.
>>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>> b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>> index bb4e68f..0ed8560 100644
>>> --- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>> +++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>> @@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
>>>     /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+,
>>> s[0-9]+} 1 } }  */
>>>     /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+,
>>> s[0-9]+} 1 } }  */
>>>     -/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 }
>>> }  */
>>> -/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }
>>> */
>>> +/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }
>>> */
>>> +/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } }  */
>>>     Some of the moves between core and fp registers were the result of
>>> inefficient codegen and in hindsight
>>> scanning for them was not very useful. Now that we emit only the required
>>> ones I think scanning for the plain
>>> vmovs between two S-registers doesn't test anything useful.
>>> So can you please just remove the second scan-assembler directive here?
>>>
> You are probably already aware of that: the tests fail on
> arm-none-linux-gnueabi/arm-none-eabi
> FAIL: gcc.target/arm/armv8_2-fp16-move-1.c scan-assembler-times
> vmov\\.f16\\ts[0-9]+, r[0-9]+ 2 (found 38 times)
>
> but this is not a regression, the previous version of the test had the
> same problem.

Grrr, that's because the softfp ABI necessitates moves between core and 
FP registers,
so scanning for a particular number of vmovs between them is just not 
gonna be stable across
soft-float ABIs. At this point I'd just remove the scan for vmovs 
completely as it doesn't
seem to check anything useful.

A patch to remove that scan for VMOV is pre-approved.

Thanks for reminding me of this Christophe, softfp tends to slip my mind :(
Kyrill

> Christophe
>
>>> Thanks,
>>> Kyrill
>>>
>>>
>>

Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Sudi Das]

[-- Attachment #1: Type: text/plain, Size: 4216 bytes --]


Hi Kyrill and Christophe


In case of soft fp testing, there are other assembly directives apart from the vmov one which are also failing. The directives probably make more sense in the hard fp context so instead of removing the vmov, I have added the -mfloat-abi=hard option.

Is this ok for trunk? If yes could someone post it on my behalf? 

Sudi


*** gcc/testsuite/ChangeLog ***

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

	* gcc.target/arm/armv8_2-fp16-move-1.c: Add -mfloat-abi=hard option.




From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
Sent: Monday, November 20, 2017 2:20 PM
To: Christophe Lyon
Cc: Sudi Das; gcc-patches@gcc.gnu.org; nd; Ramana Radhakrishnan; Richard Earnshaw
Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
  


On 20/11/17 14:14, Christophe Lyon wrote:
> Hi,
>
> On 17 November 2017 at 12:12, Kyrill  Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
>> On 17/11/17 10:45, Sudi Das wrote:
>>> Hi Kyrill
>>>
>>> Thanks I have made the change.
>>
>> Thanks Sudi, I've committed this on your behalf with r254863.
>>
>> Kyrill
>>
>>
>>> Sudi
>>>
>>>
>>>
>>> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
>>> Sent: Thursday, November 16, 2017 5:03 PM
>>> To: Sudi Das; gcc-patches@gcc.gnu.org
>>> Cc: nd; Ramana Radhakrishnan; Richard Earnshaw
>>> Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
>>>
>>> Hi Sudi,
>>>
>>> On 16/11/17 16:37, Sudi Das wrote:
>>>> Hi
>>>>
>>>> This patch fixes the test case armv8_2-fp16-move-1.c for
>>>> arm-none-linux-gnueabihf where 2 of the scan-assembler directives were
>>>> failing. We now generate less vmov between core and VFP registers.
>>>> Thus changing those directives to reflect that.
>>>>
>>>> Is this ok for trunk?
>>>> If yes could someone commit it on my behalf?
>>>>
>>>> Sudi
>>>>
>>>>
>>>> *** gcc/testsuite/ChangeLog ***
>>>>
>>>> 2017-11-16  Sudakshina Das  <sudi.das@arm.com>
>>>>
>>>>            * gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov
>>>> scan-assembler
>>>>            directives.
>>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>> b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>> index bb4e68f..0ed8560 100644
>>> --- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>> +++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>> @@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
>>>     /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+,
>>> s[0-9]+} 1 } }  */
>>>     /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+,
>>> s[0-9]+} 1 } }  */
>>>     -/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 }
>>> }  */
>>> -/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }
>>> */
>>> +/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }
>>> */
>>> +/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } }  */
>>>     Some of the moves between core and fp registers were the result of
>>> inefficient codegen and in hindsight
>>> scanning for them was not very useful. Now that we emit only the required
>>> ones I think scanning for the plain
>>> vmovs between two S-registers doesn't test anything useful.
>>> So can you please just remove the second scan-assembler directive here?
>>>
> You are probably already aware of that: the tests fail on
> arm-none-linux-gnueabi/arm-none-eabi
> FAIL: gcc.target/arm/armv8_2-fp16-move-1.c scan-assembler-times
> vmov\\.f16\\ts[0-9]+, r[0-9]+ 2 (found 38 times)
>
> but this is not a regression, the previous version of the test had the
> same problem.

Grrr, that's because the softfp ABI necessitates moves between core and 
FP registers,
so scanning for a particular number of vmovs between them is just not 
gonna be stable across
soft-float ABIs. At this point I'd just remove the scan for vmovs 
completely as it doesn't
seem to check anything useful.

A patch to remove that scan for VMOV is pre-approved.

Thanks for reminding me of this Christophe, softfp tends to slip my mind :(
Kyrill

> Christophe
>
>>> Thanks,
>>> Kyrill
>>>
>>>
>>

    

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-fp16-move-1.diff --]
[-- Type: text/x-patch; name="patch-fp16-move-1.diff", Size: 502 bytes --]

diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
index 8c0a53c..167286d 100644
--- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
+++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
@@ -1,6 +1,6 @@
 /* { dg-do compile }  */
 /* { dg-require-effective-target arm_v8_2a_fp16_scalar_ok }  */
-/* { dg-options "-O2" }  */
+/* { dg-options "-O2 -mfloat-abi=hard" }  */
 /* { dg-add-options arm_v8_2a_fp16_scalar }  */
 
 __fp16

Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Kyrill Tkachov]

On 22/11/17 11:25, Sudi Das wrote:
> Hi Kyrill and Christophe
>
>
> In case of soft fp testing, there are other assembly directives apart from the vmov one which are also failing. The directives probably make more sense in the hard fp context so instead of removing the vmov, I have added the -mfloat-abi=hard option.
>
> Is this ok for trunk? If yes could someone post it on my behalf?
>
> Sudi

Thanks Sudi,

You're right, this whole test isn't written with softfp in mind. We 
might as well restrict it to -mfloat-abi=hard.
I've committed the patch with r255061.

Would you like to get commit access to the SVN repo?
If you complete the form at https://sourceware.org/cgi-bin/pdw/ps_form.cgi
using my email address as the approver we can get that sorted out :)

Kyrill

>
> *** gcc/testsuite/ChangeLog ***
>
> 2017-11-22  Sudakshina Das  <sudi.das@arm.com>
>
> 	* gcc.target/arm/armv8_2-fp16-move-1.c: Add -mfloat-abi=hard option.
>
>
>
>
> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
> Sent: Monday, November 20, 2017 2:20 PM
> To: Christophe Lyon
> Cc: Sudi Das; gcc-patches@gcc.gnu.org; nd; Ramana Radhakrishnan; Richard Earnshaw
> Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
>    
>
>
> On 20/11/17 14:14, Christophe Lyon wrote:
>> Hi,
>>
>> On 17 November 2017 at 12:12, Kyrill  Tkachov
>> <kyrylo.tkachov@foss.arm.com> wrote:
>>> On 17/11/17 10:45, Sudi Das wrote:
>>>> Hi Kyrill
>>>>
>>>> Thanks I have made the change.
>>> Thanks Sudi, I've committed this on your behalf with r254863.
>>>
>>> Kyrill
>>>
>>>
>>>> Sudi
>>>>
>>>>
>>>>
>>>> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
>>>> Sent: Thursday, November 16, 2017 5:03 PM
>>>> To: Sudi Das; gcc-patches@gcc.gnu.org
>>>> Cc: nd; Ramana Radhakrishnan; Richard Earnshaw
>>>> Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
>>>>
>>>> Hi Sudi,
>>>>
>>>> On 16/11/17 16:37, Sudi Das wrote:
>>>>> Hi
>>>>>
>>>>> This patch fixes the test case armv8_2-fp16-move-1.c for
>>>>> arm-none-linux-gnueabihf where 2 of the scan-assembler directives were
>>>>> failing. We now generate less vmov between core and VFP registers.
>>>>> Thus changing those directives to reflect that.
>>>>>
>>>>> Is this ok for trunk?
>>>>> If yes could someone commit it on my behalf?
>>>>>
>>>>> Sudi
>>>>>
>>>>>
>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>
>>>>> 2017-11-16  Sudakshina Das  <sudi.das@arm.com>
>>>>>
>>>>>              * gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov
>>>>> scan-assembler
>>>>>              directives.
>>>>>
>>>> diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>>> b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>>> index bb4e68f..0ed8560 100644
>>>> --- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>>> +++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>>> @@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
>>>>       /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, s[0-9]+,
>>>> s[0-9]+} 1 } }  */
>>>>       /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, s[0-9]+,
>>>> s[0-9]+} 1 } }  */
>>>>       -/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 4 }
>>>> }  */
>>>> -/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, s[0-9]+} 4 } }
>>>> */
>>>> +/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, r[0-9]+} 2 } }
>>>> */
>>>> +/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } }  */
>>>>       Some of the moves between core and fp registers were the result of
>>>> inefficient codegen and in hindsight
>>>> scanning for them was not very useful. Now that we emit only the required
>>>> ones I think scanning for the plain
>>>> vmovs between two S-registers doesn't test anything useful.
>>>> So can you please just remove the second scan-assembler directive here?
>>>>
>> You are probably already aware of that: the tests fail on
>> arm-none-linux-gnueabi/arm-none-eabi
>> FAIL: gcc.target/arm/armv8_2-fp16-move-1.c scan-assembler-times
>> vmov\\.f16\\ts[0-9]+, r[0-9]+ 2 (found 38 times)
>>
>> but this is not a regression, the previous version of the test had the
>> same problem.
> Grrr, that's because the softfp ABI necessitates moves between core and
> FP registers,
> so scanning for a particular number of vmovs between them is just not
> gonna be stable across
> soft-float ABIs. At this point I'd just remove the scan for vmovs
> completely as it doesn't
> seem to check anything useful.
>
> A patch to remove that scan for VMOV is pre-approved.
>
> Thanks for reminding me of this Christophe, softfp tends to slip my mind :(
> Kyrill
>
>> Christophe
>>
>>>> Thanks,
>>>> Kyrill
>>>>
>>>>
>      

Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c

[Sudakshina Das]

On 22/11/17 15:21, Kyrill Tkachov wrote:
>
> On 22/11/17 11:25, Sudi Das wrote:
>> Hi Kyrill and Christophe
>>
>>
>> In case of soft fp testing, there are other assembly directives apart 
>> from the vmov one which are also failing. The directives probably 
>> make more sense in the hard fp context so instead of removing the 
>> vmov, I have added the -mfloat-abi=hard option.
>>
>> Is this ok for trunk? If yes could someone post it on my behalf?
>>
>> Sudi
>
> Thanks Sudi,
>
> You're right, this whole test isn't written with softfp in mind. We 
> might as well restrict it to -mfloat-abi=hard.
> I've committed the patch with r255061.
>
Hi Kyriil

Thanks for the commit!

> Would you like to get commit access to the SVN repo?
> If you complete the form at 
> https://sourceware.org/cgi-bin/pdw/ps_form.cgi
> using my email address as the approver we can get that sorted out :)
>

Thanks again for the invite. I have filled out the form! :)

Sudi


> Kyrill
>
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2017-11-22  Sudakshina Das  <sudi.das@arm.com>
>>
>>     * gcc.target/arm/armv8_2-fp16-move-1.c: Add -mfloat-abi=hard option.
>>
>>
>>
>>
>> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
>> Sent: Monday, November 20, 2017 2:20 PM
>> To: Christophe Lyon
>> Cc: Sudi Das; gcc-patches@gcc.gnu.org; nd; Ramana Radhakrishnan; 
>> Richard Earnshaw
>> Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
>>
>>
>> On 20/11/17 14:14, Christophe Lyon wrote:
>>> Hi,
>>>
>>> On 17 November 2017 at 12:12, Kyrill  Tkachov
>>> <kyrylo.tkachov@foss.arm.com> wrote:
>>>> On 17/11/17 10:45, Sudi Das wrote:
>>>>> Hi Kyrill
>>>>>
>>>>> Thanks I have made the change.
>>>> Thanks Sudi, I've committed this on your behalf with r254863.
>>>>
>>>> Kyrill
>>>>
>>>>
>>>>> Sudi
>>>>>
>>>>>
>>>>>
>>>>> From: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>
>>>>> Sent: Thursday, November 16, 2017 5:03 PM
>>>>> To: Sudi Das; gcc-patches@gcc.gnu.org
>>>>> Cc: nd; Ramana Radhakrishnan; Richard Earnshaw
>>>>> Subject: Re: [PATCH][ARM] Fix test armv8_2-fp16-move-1.c
>>>>>
>>>>> Hi Sudi,
>>>>>
>>>>> On 16/11/17 16:37, Sudi Das wrote:
>>>>>> Hi
>>>>>>
>>>>>> This patch fixes the test case armv8_2-fp16-move-1.c for
>>>>>> arm-none-linux-gnueabihf where 2 of the scan-assembler directives 
>>>>>> were
>>>>>> failing. We now generate less vmov between core and VFP registers.
>>>>>> Thus changing those directives to reflect that.
>>>>>>
>>>>>> Is this ok for trunk?
>>>>>> If yes could someone commit it on my behalf?
>>>>>>
>>>>>> Sudi
>>>>>>
>>>>>>
>>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>>
>>>>>> 2017-11-16  Sudakshina Das  <sudi.das@arm.com>
>>>>>>
>>>>>>              * gcc.target/arm/armv8_2-fp16-move-1.c: Edit vmov
>>>>>> scan-assembler
>>>>>>              directives.
>>>>>>
>>>>> diff --git a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>>>> b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>>>> index bb4e68f..0ed8560 100644
>>>>> --- a/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>>>> +++ b/gcc/testsuite/gcc.target/arm/armv8_2-fp16-move-1.c
>>>>> @@ -101,8 +101,8 @@ test_select_8 (__fp16 a, __fp16 b, __fp16 c)
>>>>>       /* { dg-final { scan-assembler-times {vselgt\.f16\ts[0-9]+, 
>>>>> s[0-9]+,
>>>>> s[0-9]+} 1 } }  */
>>>>>       /* { dg-final { scan-assembler-times {vselge\.f16\ts[0-9]+, 
>>>>> s[0-9]+,
>>>>> s[0-9]+} 1 } }  */
>>>>>       -/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, 
>>>>> r[0-9]+} 4 }
>>>>> }  */
>>>>> -/* { dg-final { scan-assembler-times {vmov\.f16\tr[0-9]+, 
>>>>> s[0-9]+} 4 } }
>>>>> */
>>>>> +/* { dg-final { scan-assembler-times {vmov\.f16\ts[0-9]+, 
>>>>> r[0-9]+} 2 } }
>>>>> */
>>>>> +/* { dg-final { scan-assembler-times {vmov\ts[0-9]+, s[0-9]+} 4 } 
>>>>> }  */
>>>>>       Some of the moves between core and fp registers were the 
>>>>> result of
>>>>> inefficient codegen and in hindsight
>>>>> scanning for them was not very useful. Now that we emit only the 
>>>>> required
>>>>> ones I think scanning for the plain
>>>>> vmovs between two S-registers doesn't test anything useful.
>>>>> So can you please just remove the second scan-assembler directive 
>>>>> here?
>>>>>
>>> You are probably already aware of that: the tests fail on
>>> arm-none-linux-gnueabi/arm-none-eabi
>>> FAIL: gcc.target/arm/armv8_2-fp16-move-1.c scan-assembler-times
>>> vmov\\.f16\\ts[0-9]+, r[0-9]+ 2 (found 38 times)
>>>
>>> but this is not a regression, the previous version of the test had the
>>> same problem.
>> Grrr, that's because the softfp ABI necessitates moves between core and
>> FP registers,
>> so scanning for a particular number of vmovs between them is just not
>> gonna be stable across
>> soft-float ABIs. At this point I'd just remove the scan for vmovs
>> completely as it doesn't
>> seem to check anything useful.
>>
>> A patch to remove that scan for VMOV is pre-approved.
>>
>> Thanks for reminding me of this Christophe, softfp tends to slip my 
>> mind :(
>> Kyrill
>>
>>> Christophe
>>>
>>>>> Thanks,
>>>>> Kyrill
>>>>>
>>>>>
>