Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.
On Fri, Apr 27, 2018 at 02:06:04PM -0700, Rahul Chaudhry wrote: > It was reported on github that this patch causes build errors with xcode: > > https://github.com/FFmpeg/FFmpeg/commit/b22db4f465c9adb2cf1489e04f7b65ef6bb55b8b#commitcomment-28725295 > > The attached patch fixes the build errors by renaming the labels. This time > I've tested it with clang from xcode on a MacBook to verify that the build > works. > > Thanks, > Rahul > > > On Thu, Apr 19, 2018 at 11:06 AM, Michael Niedermayer > wrote: > > On Wed, Apr 18, 2018 at 04:40:28PM -0700, Rahul Chaudhry wrote: > >> On Wed, Apr 18, 2018 at 3:46 PM, Michael Niedermayer > >> wrote: > >> > please make sure this works on apple based arm (unless you know it works) > >> > (ive tested qemu linux based) > >> > > >> > Also please add a commit message > >> > >> If by 'apple based arm' you mean llvm/clang assembler, then yes, I've > >> verified > >> that the assembly works with armv7a-cros-linux-gnueabi-clang (version > >> 7.0.0). > >> > >> Updated patch with new commit message is attached. > >> > >> Thanks, > >> Rahul > > > >> From 2e3318acf266b519e98b680102a07196d6ddbf93 Mon Sep 17 00:00:00 2001 > >> From: Rahul Chaudhry > >> Date: Wed, 18 Apr 2018 16:29:39 -0700 > >> Subject: [PATCH] swresample/arm: remove unintentional relocation. > > > > ok, will apply > > > > thx > From a58b726947cc22081d899894036fa01933dfac0a Mon Sep 17 00:00:00 2001 > From: Rahul Chaudhry > Date: Fri, 27 Apr 2018 13:49:52 -0700 > Subject: [PATCH] swresample/arm: rename labels to fix xcode build error will apply thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.
It was reported on github that this patch causes build errors with xcode:
https://github.com/FFmpeg/FFmpeg/commit/b22db4f465c9adb2cf1489e04f7b65ef6bb55b8b#commitcomment-28725295
The attached patch fixes the build errors by renaming the labels. This time
I've tested it with clang from xcode on a MacBook to verify that the build
works.
Thanks,
Rahul
On Thu, Apr 19, 2018 at 11:06 AM, Michael Niedermayer
wrote:
> On Wed, Apr 18, 2018 at 04:40:28PM -0700, Rahul Chaudhry wrote:
>> On Wed, Apr 18, 2018 at 3:46 PM, Michael Niedermayer
>> wrote:
>> > please make sure this works on apple based arm (unless you know it works)
>> > (ive tested qemu linux based)
>> >
>> > Also please add a commit message
>>
>> If by 'apple based arm' you mean llvm/clang assembler, then yes, I've
>> verified
>> that the assembly works with armv7a-cros-linux-gnueabi-clang (version 7.0.0).
>>
>> Updated patch with new commit message is attached.
>>
>> Thanks,
>> Rahul
>
>> From 2e3318acf266b519e98b680102a07196d6ddbf93 Mon Sep 17 00:00:00 2001
>> From: Rahul Chaudhry
>> Date: Wed, 18 Apr 2018 16:29:39 -0700
>> Subject: [PATCH] swresample/arm: remove unintentional relocation.
>
> ok, will apply
>
> thx
From a58b726947cc22081d899894036fa01933dfac0a Mon Sep 17 00:00:00 2001
From: Rahul Chaudhry
Date: Fri, 27 Apr 2018 13:49:52 -0700
Subject: [PATCH] swresample/arm: rename labels to fix xcode build error
---
libswresample/arm/audio_convert_neon.S | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git libswresample/arm/audio_convert_neon.S
libswresample/arm/audio_convert_neon.S
index
7729514701d3a02d04146f24cd9b12b11548ac64..085d50aafa5cf364e357d251a3986fad55643589
100644
--- libswresample/arm/audio_convert_neon.S
+++ libswresample/arm/audio_convert_neon.S
@@ -22,7 +22,7 @@
#include "libavutil/arm/asm.S"
function swri_oldapi_conv_flt_to_s16_neon, export=1
-_swri_oldapi_conv_flt_to_s16_neon:
+.L_swri_oldapi_conv_flt_to_s16_neon:
subsr2, r2, #8
vld1.32 {q0}, [r1,:128]!
vcvt.s32.f32q8, q0, #31
@@ -67,7 +67,7 @@ _swri_oldapi_conv_flt_to_s16_neon:
endfunc
function swri_oldapi_conv_fltp_to_s16_2ch_neon, export=1
-_swri_oldapi_conv_fltp_to_s16_2ch_neon:
+.L_swri_oldapi_conv_fltp_to_s16_2ch_neon:
ldm r1, {r1, r3}
subsr2, r2, #8
vld1.32 {q0}, [r1,:128]!
@@ -135,8 +135,8 @@ function swri_oldapi_conv_fltp_to_s16_nch_neon, export=1
cmp r3, #2
itt lt
ldrlt r1, [r1]
-blt _swri_oldapi_conv_flt_to_s16_neon
-beq _swri_oldapi_conv_fltp_to_s16_2ch_neon
+blt .L_swri_oldapi_conv_flt_to_s16_neon
+beq .L_swri_oldapi_conv_fltp_to_s16_2ch_neon
push{r4-r8, lr}
cmp r3, #4
--
2.13.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.
On Wed, Apr 18, 2018 at 04:40:28PM -0700, Rahul Chaudhry wrote: > On Wed, Apr 18, 2018 at 3:46 PM, Michael Niedermayer > wrote: > > please make sure this works on apple based arm (unless you know it works) > > (ive tested qemu linux based) > > > > Also please add a commit message > > If by 'apple based arm' you mean llvm/clang assembler, then yes, I've verified > that the assembly works with armv7a-cros-linux-gnueabi-clang (version 7.0.0). > > Updated patch with new commit message is attached. > > Thanks, > Rahul > From 2e3318acf266b519e98b680102a07196d6ddbf93 Mon Sep 17 00:00:00 2001 > From: Rahul Chaudhry > Date: Wed, 18 Apr 2018 16:29:39 -0700 > Subject: [PATCH] swresample/arm: remove unintentional relocation. ok, will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.
On Wed, Apr 18, 2018 at 3:46 PM, Michael Niedermayer
wrote:
> please make sure this works on apple based arm (unless you know it works)
> (ive tested qemu linux based)
>
> Also please add a commit message
If by 'apple based arm' you mean llvm/clang assembler, then yes, I've verified
that the assembly works with armv7a-cros-linux-gnueabi-clang (version 7.0.0).
Updated patch with new commit message is attached.
Thanks,
Rahul
From 2e3318acf266b519e98b680102a07196d6ddbf93 Mon Sep 17 00:00:00 2001
From: Rahul Chaudhry
Date: Wed, 18 Apr 2018 16:29:39 -0700
Subject: [PATCH] swresample/arm: remove unintentional relocation.
Branch to global symbol results in reference to PLT, and when compiling
for THUMB-2 - in a R_ARM_THM_JUMP19 relocation. Some linkers don't
support this relocation (ld.gold), while others can end up truncating
the relocation to fit (ld.bfd).
Convert this branch through PLT into a direct branch that the assembler
can resolve locally.
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:
https://github.com/freedesktop/gstreamer-cerbero/commit/41556c415739fbc3a72c7eaee7e70a565b719b2f
---
libswresample/arm/audio_convert_neon.S | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git libswresample/arm/audio_convert_neon.S
libswresample/arm/audio_convert_neon.S
index
1f88316ddec838dfe791b08cbe72533207994741..7729514701d3a02d04146f24cd9b12b11548ac64
100644
--- libswresample/arm/audio_convert_neon.S
+++ libswresample/arm/audio_convert_neon.S
@@ -22,6 +22,7 @@
#include "libavutil/arm/asm.S"
function swri_oldapi_conv_flt_to_s16_neon, export=1
+_swri_oldapi_conv_flt_to_s16_neon:
subsr2, r2, #8
vld1.32 {q0}, [r1,:128]!
vcvt.s32.f32q8, q0, #31
@@ -66,6 +67,7 @@ function swri_oldapi_conv_flt_to_s16_neon, export=1
endfunc
function swri_oldapi_conv_fltp_to_s16_2ch_neon, export=1
+_swri_oldapi_conv_fltp_to_s16_2ch_neon:
ldm r1, {r1, r3}
subsr2, r2, #8
vld1.32 {q0}, [r1,:128]!
@@ -133,8 +135,8 @@ function swri_oldapi_conv_fltp_to_s16_nch_neon, export=1
cmp r3, #2
itt lt
ldrlt r1, [r1]
-blt X(swri_oldapi_conv_flt_to_s16_neon)
-beq X(swri_oldapi_conv_fltp_to_s16_2ch_neon)
+blt _swri_oldapi_conv_flt_to_s16_neon
+beq _swri_oldapi_conv_fltp_to_s16_2ch_neon
push{r4-r8, lr}
cmp r3, #4
--
2.13.5
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.
On Tue, Apr 17, 2018 at 12:36:45PM -0700, Rahul Chaudhry wrote: > On Tue, Apr 17, 2018 at 6:17 AM, Michael Niedermayer > wrote: > > why does it go through the PLT at all ? > > You're right. The branches don't have to go through the PLT at all. > > Here's an alternate patch that modifies the branches to skip the PLT > and jump directly to the targets. > > Note that unlike the previous patch, this patch changes semantics. > When branches were going through PLT, the targets could be overridden > at runtime by another shared library or the main executable. With > this patch, they can no longer be overridden. The branches will always > go to the corresponding functions defined in this assembly file. please make sure this works on apple based arm (unless you know it works) (ive tested qemu linux based) Also please add a commit message thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.
On Tue, Apr 17, 2018 at 6:17 AM, Michael Niedermayer
wrote:
> why does it go through the PLT at all ?
You're right. The branches don't have to go through the PLT at all.
Here's an alternate patch that modifies the branches to skip the PLT
and jump directly to the targets.
Note that unlike the previous patch, this patch changes semantics.
When branches were going through PLT, the targets could be overridden
at runtime by another shared library or the main executable. With
this patch, they can no longer be overridden. The branches will always
go to the corresponding functions defined in this assembly file.
--- libswresample/arm/audio_convert_neon.S
+++ libswresample/arm/audio_convert_neon.S
@@ -22,6 +22,7 @@
#include "libavutil/arm/asm.S"
function swri_oldapi_conv_flt_to_s16_neon, export=1
+_swri_oldapi_conv_flt_to_s16_neon:
subsr2, r2, #8
vld1.32 {q0}, [r1,:128]!
vcvt.s32.f32q8, q0, #31
@@ -66,6 +67,7 @@ function swri_oldapi_conv_flt_to_s16_neon, export=1
endfunc
function swri_oldapi_conv_fltp_to_s16_2ch_neon, export=1
+_swri_oldapi_conv_fltp_to_s16_2ch_neon:
ldm r1, {r1, r3}
subsr2, r2, #8
vld1.32 {q0}, [r1,:128]!
@@ -133,8 +135,8 @@ function swri_oldapi_conv_fltp_to_s16_nch_neon, export=1
cmp r3, #2
itt lt
ldrlt r1, [r1]
-blt X(swri_oldapi_conv_flt_to_s16_neon)
-beq X(swri_oldapi_conv_fltp_to_s16_2ch_neon)
+blt _swri_oldapi_conv_flt_to_s16_neon
+beq _swri_oldapi_conv_fltp_to_s16_2ch_neon
push{r4-r8, lr}
cmp r3, #4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.
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 ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No snowflake in an avalanche ever feels responsible. -- Voltaire signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-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 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 > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] swresample/arm: avoid conditional branch to PLT in THUMB-2.
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 signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [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.
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:
https://github.com/freedesktop/gstreamer-cerbero/commit/41556c415739fbc3a72c7eaee7e70a565b719b2f
---
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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [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.
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
From 8dbb701398cf26a6a2f4686f871c5032dcbf1158 Mon Sep 17 00:00:00 2001
From: Rahul Chaudhry
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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
