diff mbox

[FFmpeg-devel] swresample/arm: avoid conditional branch to PLT in THUMB-2.

Message ID CAJRD=opgUMe851Ft7aB2quHV_7QzycbJzXNadRAWn3zauQCCyw@mail.gmail.com
State Superseded
Headers show

Commit Message

Rahul Chaudhry April 13, 2018, 8:43 p.m. UTC 
When compiling for THUMB-2, the conditional branch to PLT results in a
R_ARM_THM_JUMP19 relocation. Some linkers don't support this relocation
in THUMB-2 (ld.gold), while others can end up truncating the relocation
to fit (ld.bfd).

Adding an "it eq" before the branch converts it into an unconditional
branch, which uses R_ARM_THM_JUMP24 relocation that has a range of 16MB.

See https://github.com/android-ndk/ndk/issues/337 for background.

The current workaround is to disable neon during gstreamer build,
which is not optimal and can be reverted after this patch.

Rahul

Comments

Michael Niedermayer April 16, 2018, 9:31 a.m. UTC | #1 
On Fri, Apr 13, 2018 at 01:43:44PM -0700, Rahul Chaudhry wrote:
> When compiling for THUMB-2, the conditional branch to PLT results in a
> R_ARM_THM_JUMP19 relocation. Some linkers don't support this relocation
> in THUMB-2 (ld.gold), while others can end up truncating the relocation
> to fit (ld.bfd).

iam not a arm expert but if its needed only for some linkers, shouldnt
it be conditional and only used for these linkers ?
(checking in configure if a linker suppors it)

[...]
Rahul Chaudhry April 16, 2018, 8:13 p.m. UTC | #2 
Hi Michael,

While it is true that some linkers support the conditional branch to PLT in
Thumb mode, it is of very limited use, since it uses an R_ARM_THM_JUMP19
relocation and does not have a good range. If I switch my linker to ld.bfd for
the original issue, it fails for another library with a "relocation truncated
to fit" error message because the PLT entry is out of range of the conditional
branch and cannot be handled by the R_ARM_THM_JUMP19 relocation.

It is cleaner and more stable (for all linkers) to convert the conditional
branch to an unconditional one which uses an R_ARM_THM_JUMP24 relocation and
has a range of 16MB.

Rahul


On Mon, Apr 16, 2018 at 2:31 AM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Fri, Apr 13, 2018 at 01:43:44PM -0700, Rahul Chaudhry wrote:
>> When compiling for THUMB-2, the conditional branch to PLT results in a
>> R_ARM_THM_JUMP19 relocation. Some linkers don't support this relocation
>> in THUMB-2 (ld.gold), while others can end up truncating the relocation
>> to fit (ld.bfd).
>
> iam not a arm expert but if its needed only for some linkers, shouldnt
> it be conditional and only used for these linkers ?
> (checking in configure if a linker suppors it)
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The worst form of inequality is to try to make unequal things equal.
> -- Aristotle
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Michael Niedermayer April 17, 2018, 1:17 p.m. UTC | #3 
On Mon, Apr 16, 2018 at 01:13:21PM -0700, Rahul Chaudhry wrote:
> Hi Michael,
> 
> While it is true that some linkers support the conditional branch to PLT in
> Thumb mode, it is of very limited use, since it uses an R_ARM_THM_JUMP19
> relocation and does not have a good range. If I switch my linker to ld.bfd for
> the original issue, it fails for another library with a "relocation truncated
> to fit" error message because the PLT entry is out of range of the conditional
> branch and cannot be handled by the R_ARM_THM_JUMP19 relocation.
> 
> It is cleaner and more stable (for all linkers) to convert the conditional
> branch to an unconditional one which uses an R_ARM_THM_JUMP24 relocation and
> has a range of 16MB.

why does it go through the PLT at all ?


[...]
diff mbox

Patch

From 8dbb701398cf26a6a2f4686f871c5032dcbf1158 Mon Sep 17 00:00:00 2001
From: Rahul Chaudhry <rahulchaudhry@chromium.org>
Date: Thu, 12 Apr 2018 16:27:31 -0700
Subject: [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.

When compiling for THUMB-2, the conditional branch to PLT results in a
R_ARM_THM_JUMP19 relocation. Some linkers don't support this relocation
in THUMB-2 (ld.gold), while others can end up truncating the relocation
to fit (ld.bfd).

Adding an "it eq" before the branch converts it into an unconditional
branch, which uses R_ARM_THM_JUMP24 relocation that has a range of 16MB.
---
 libswresample/arm/audio_convert_neon.S | 1 +
 1 file changed, 1 insertion(+)

diff --git libswresample/arm/audio_convert_neon.S libswresample/arm/audio_convert_neon.S
index 1f88316ddec838dfe791b08cbe72533207994741..bc933fb4bd00071702f553cc0f3e74797c33ab12 100644
--- libswresample/arm/audio_convert_neon.S
+++ libswresample/arm/audio_convert_neon.S
@@ -134,6 +134,7 @@  function swri_oldapi_conv_fltp_to_s16_nch_neon, export=1
         itt             lt
         ldrlt           r1,  [r1]
         blt             X(swri_oldapi_conv_flt_to_s16_neon)
+        it              eq
         beq             X(swri_oldapi_conv_fltp_to_s16_2ch_neon)
 
         push            {r4-r8, lr}
-- 
2.13.5