Hi,

On 04/08/17 09:58, Jiong Wang wrote:
> Change
> 
>   "adreq lr,X(ff_h264_idct_add_neon) +CONFIG_THUMB"
> 
> Into:
> 
> .eqv ff_h264_idct_add_neon_without_func_type, X(ff_h264_idct_add_neon)
> adreq lr,  ff_h264_idct_add_neon_without_func_type +CONFIG_THUMB
> 
> might be a solution.  The idea is we use .eqv to remove the function
> attribute, so the assembler won't set LSB in any case.

This was the commit which introduced the +CONFIG_THUMB stuff:
https://github.com/FFmpeg/FFmpeg/commit/8986fddc2bab92bd7d77a123ac70c4fb70c96c7c

On the technical side, does having the LSB clear when executing a blx
instruction cause a mode change out of Thumb, or does it retain the
mode? I think all the code in that file is compiled in the same mode, so
if the mode is retained then simply dropping +CONFIG_THUMB might work.

Would it be possible for you or someone with better ARM assembly
experience to submit the fixes upstream? It would help greatly.

On Debian, there is #870676 open about NEON code on ARM. We could "fix"
this bug and that one by disabling NEON but it would be nice if we
didn't have to do that.

Thanks,
James

> On 04/08/17 12:39, Jiong Wang wrote:
>> Hi,
>>
>>   This issue is caused by a recent change in ARM assembler included
>> since Binutils 2.29.
>>
>>   The details of that change can be found at
>> https://sourceware.org/bugzilla/show_bug.cgi?id=21458
>>
>>   The semantics of ADR has changed.  In general, the address generated
>> by ADR will guarantee the LSB be set if it's a thumb function address.
>>
>>    I noticed h264idct_neon.S is using something like:
>>
>>      adreq lr,X(ff_h264_idct_add_neon) +CONFIG_THUMB
>>
>>    As ADR now will set the LSB automatically, you don't need
>> CONFIG_THUMB any more.
>>
>>    I think h264idct_neon.S needs to be updated, and the modification
>> should make sure it works with both old Binutils and the new one.
>>
>> Regards,
>> Jiong

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to