[FFmpeg-devel] [PATCH] checkasm/aarch64: fix tests returning a float

[James Almer]

On 6/22/2017 5:35 AM, Matthieu Bouron wrote:
> On Wed, Jun 21, 2017 at 10:19:33PM +0200, Matthieu Bouron wrote:
>> On Wed, Jun 21, 2017 at 04:57:53PM -0300, James Almer wrote:
>>> On 6/19/2017 6:08 AM, Matthieu Bouron wrote:
>>>> Avoids overriding v0 (which containins the result of the tested
>>>> function) in checkasm_call_checked.
>>>>
>>>> Also properly calls checkasm_call_checked.
>>>> ---
>>>>  tests/checkasm/aarch64/checkasm.S | 8 ++++----
>>>>  tests/checkasm/checkasm.h         | 2 ++
>>>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/checkasm.S
>>>> index 53a2a478dc..75a9a56143 100644
>>>> --- a/tests/checkasm/aarch64/checkasm.S
>>>> +++ b/tests/checkasm/aarch64/checkasm.S
>>>> @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1
>>>>      movi        v3.8h,  #0
>>>>  
>>>>  .macro check_reg_neon reg1, reg2
>>>> -    ldr         q0,  [x9], #16
>>>> -    uzp1        v1.2d,  v\reg1\().2d, v\reg2\().2d
>>>> -    eor         v0.16b, v0.16b, v1.16b
>>>> -    orr         v3.16b, v3.16b, v0.16b
>>>> +    ldr         q1,  [x9], #16
>>>> +    uzp1        v2.2d,  v\reg1\().2d, v\reg2\().2d
>>>> +    eor         v1.16b, v1.16b, v2.16b
>>>> +    orr         v3.16b, v3.16b, v1.16b
>>>>  .endm
>>>>      check_reg_neon  8,  9
>>>>      check_reg_neon  10, 11
>>>> diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
>>>> index 695d871220..5249669fba 100644
>>>> --- a/tests/checkasm/checkasm.h
>>>> +++ b/tests/checkasm/checkasm.h
>>>> @@ -145,6 +145,8 @@ void checkasm_stack_clobber(uint64_t clobber, ...);
>>>>  void checkasm_checked_call(void *func, ...);
>>>>  #define declare_new(ret, ...) ret (*checked_call)(void *, int, int, int, int, int, int, int, __VA_ARGS__)\
>>>>                                = (void *)checkasm_checked_call;
>>>> +#define declare_new_float(ret, ...) ret (*checked_call)(void *, int, int, int, int, int, int, int, __VA_ARGS__)\
>>>> +                              = (void *)checkasm_checked_call;
>>>
>>> Isn't this doing the same as the generic "#define declare_new_float"
>>> about 15 lines below? If declare_new_float() is no different than
>>> declare_new() for a given target, then just let the aforementioned check
>>> handle it.
>>
>> You are right (I missed this define), so this chunk is removed from the
>> patch.
>>
>>>
>>>>  #define CLOB (UINT64_C(0xdeadbeefdeadbeef))
>>>>  #define call_new(...) (checkasm_stack_clobber(CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,\
>>>>                                                CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB,CLOB),\
>>>>
>>>
>>> Should be ok either way if tested (The only aarch64 FATE client using
>>> git head hasn't run in twenty days, so no way for me to check if this
>>> fixes the problem anyway).
>>
>> I tested on my Odroid-C2 and fate passes with this patch (otherwise it
>> fails on checkasm float_dsp.scalarproduct_float test).
>>
>> I've attached the updated patch. I'll push it in a few hours if there is
>> no objection.
>>
>> -- 
>> Matthieu B.
> 
>> From ee4bd4fdb9ac01bd331a43222e9029f820bec4e5 Mon Sep 17 00:00:00 2001
>> From: Matthieu Bouron <matthieu.bouron at gmail.com>
>> Date: Mon, 19 Jun 2017 10:55:28 +0200
>> Subject: [PATCH] checkasm/aarch64: fix tests returning a float
>>
>> Avoids overriding v0 (which containins the result of the tested
>> function) in checkasm_call_checked.
>> ---
>>  tests/checkasm/aarch64/checkasm.S | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/checkasm/aarch64/checkasm.S b/tests/checkasm/aarch64/checkasm.S
>> index 53a2a478dc..75a9a56143 100644
>> --- a/tests/checkasm/aarch64/checkasm.S
>> +++ b/tests/checkasm/aarch64/checkasm.S
>> @@ -112,10 +112,10 @@ function checkasm_checked_call, export=1
>>      movi        v3.8h,  #0
>>  
>>  .macro check_reg_neon reg1, reg2
>> -    ldr         q0,  [x9], #16
>> -    uzp1        v1.2d,  v\reg1\().2d, v\reg2\().2d
>> -    eor         v0.16b, v0.16b, v1.16b
>> -    orr         v3.16b, v3.16b, v0.16b
>> +    ldr         q1,  [x9], #16
>> +    uzp1        v2.2d,  v\reg1\().2d, v\reg2\().2d
>> +    eor         v1.16b, v1.16b, v2.16b
>> +    orr         v3.16b, v3.16b, v1.16b
>>  .endm
>>      check_reg_neon  8,  9
>>      check_reg_neon  10, 11
>> -- 
>> 2.13.1
>>
> 
> Patch applied.
> 
> Thanks.

Do you know if this needs to be done for arm as well? Fate is reporting
an error in fate-checkasm-float_dsp.

I added a fate-checkasm-sbrdsp target earlier today, so if that one also
fails on arm then it will mean it's highly likely the function with a
float return value is what's at fault.