[x265] PING [PATCH] aarch64: replace ldr pseudo-instruction with adrp+add
Fangrui Song
maskray at google.com
Sun Dec 18 06:13:00 UTC 2022
On Sun, Dec 4, 2022 at 12:13 PM Fangrui Song <maskray at google.com> wrote:
>
> On Fri, Nov 18, 2022 at 3:12 PM Fangrui Song <maskray at google.com> wrote:
> >
> > On Tue, Oct 18, 2022 at 5:30 AM chen <chenm003 at 163.com> wrote:
> > >
> > > Hi Song,
> > >
> > > This is a long time history bug. the '-DPIC' just add to compile option when Nasm is enabled, the bug can simple fix with below patch (NO TEST since I haven't ARM environment).
> > > After patch, the user may acivate these compile option with ENABLE_PIC=ON
> > >
> > > Could the x265 team please help verify patch?
> > >
> > > Regards,
> > > Min Chen
> > >
> > > diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
> > > index 13e4750de..80f8e59a9 100755
> > > --- a/source/CMakeLists.txt
> > > +++ b/source/CMakeLists.txt
> > > @@ -266,6 +266,9 @@ if(GCC)
> > > add_definitions(-DHAVE_NEON)
> > > endif()
> > > endif()
> > > + if(ENABLE_PIC)
> > > + list(APPEND ARM_ARGS -DPIC)
> > > + endif()
> > > add_definitions(${ARM_ARGS})
> > > if(FPROFILE_GENERATE)
> > > if(INTEL_CXX)
> > >
> >
> > Hi Min, I think my patch as-is is correct. Whether aarch64 builds
> > wants -DPIC is a separate topic.
> > Is a maintainer able to verify and install this patch? Thanks
> >
> > > 2022-10-18 14:37:10，"Fangrui Song" <maskray at google.com>
> > >
> > > On Thu, Oct 6, 2022 at 1:39 AM chen <chenm003 at 163.com> wrote:
> > >>
> > >> Hi Song,
> > >>
> > >>
> > >> I means the current tree support these adrp+add mode with compile option -DPIC, so we need not patch the code.
> > >>
> > >>
> > >> Regards,
> > >> Min Chen
> > >
> > >
> > > Hi Min, IIUC -DPIC is only defined for x86-64 (source/cmake/CMakeASM_NASMInformation.cmake).
> > > AArch64 does not get -DPIC.
> > >>
> > >> 2022-10-06 06:42:33，"Fangrui Song" <maskray at google.com>
> > >>
> > >> Hi Min, sorry but I just saw your question. I do not understand the request.
> > >> adrp+add is just strictly superior to ldr (which uses a constant pool and does not decrease code size) and avoids text relocations (which are prohibited in many systems). `ldr \rd, =\val+\offset` should just be removed. adrp+add also works on Mach-O systems as well.
> > >>
> > >>>
> > >>> At 2022-09-24 15:21:35, "Fangrui Song" <maskray at google.com> wrote:
> > >>> >Ping. The breaks lld build and some binutils configurating defaulting
> > >>> >to disallow text relocations.
> > >>> >
> > >>> >On 2022-08-29, Fangrui Song wrote:
> > >>> >>On 2022-08-29, Fangrui Song wrote:
> > >>> >>>On 2022-08-30, chen wrote:
> > >>> >>>>Hi Song,
> > >>> >>>>
> > >>> >>>>
> > >>> >>>>Thank you for your patch.
> > >>> >>>>
> > >>> >>>>
> > >>> >>>>However, syntax of ':lo12:' depends on compiler, so more general LDR is better in here.
> > >>> >>>>
> > >>> >>>>
> > >>> >>>>Regards,
> > >>> >>>>Min Chen
> > >>> >>>
> > >>> >>>:lo12: is standard aarch64 assembly syntax.
> > >>> >>>Which aarch64 compiler supported by x265 does not support :lo12:?
> > >>> >>
> > >>> >>Note that LDR has another problem that it produced an absolute relocation R_AARCH64_ABS64. It will trigger an error
> > >>> >>when text relocations are disabled (default in ld.lld. See https://maskray.me/blog/2020-12-19-lld-and-gnu-linker-incompatibilities#:~:text=Text%20relocations)
> > >>> >>
> > >>> >>```
> > >>> >>% : && /usr/bin/c++ -fPIC -O3 -DNDEBUG -Wl,-Bsymbolic,-znoexecstack -shared ...
> > >>> >>ld: error: relocation R_AARCH64_ABS64 cannot be used against local
> > >>> >>symbol; recompile with -fPIC
> > >>> >>>>>defined in sad-a.S.o
> > >>> >>>>>>>> referenced by sad-a.S.o:(.text+0x9B00)
> > >>> >>```
> > >>> >>
> > >>> >>adrp+add work fine.
> > >>> >>
> > >>> >>(I am a maintainer of lld/ELF.)
> > >>> >>
> > >>> >>>>At 2022-08-30 02:33:37, "Fangrui Song" <maskray at google.com> wrote:
> > >>> >>>>>The ldr pseudo-instruction uses a literal pool, which is less efficient
> > >>> >>>>>and does not decrease the code size.
> > >>> >>>>>---
> > >>> >>>>>source/common/aarch64/asm.S | 4 +---
> > >>> >>>>>1 file changed, 1 insertion(+), 3 deletions(-)
> > >>> >>>>>
> > >>> >>>>>diff --git a/source/common/aarch64/asm.S b/source/common/aarch64/asm.S
> > >>> >>>>>index 399c37cf2..2506f50aa 100644
> > >>> >>>>>--- a/source/common/aarch64/asm.S
> > >>> >>>>>+++ b/source/common/aarch64/asm.S
> > >>> >>>>>@@ -130,11 +130,9 @@ ELF .size \name, . - \name
> > >>> >>>>> adrp \rd, \val+(\offset)
> > >>> >>>>> add \rd, \rd, :lo12:\val+(\offset)
> > >>> >>>>> .endif
> > >>> >>>>>-#elif defined(PIC)
> > >>> >>>>>+#else
> > >>> >>>>> adrp \rd, \val+(\offset)
> > >>> >>>>> add \rd, \rd, :lo12:\val+(\offset)
> > >>> >>>>>-#else
> > >>> >>>>>- ldr \rd, =\val+\offset
> > >>> >>>>>#endif
> > >>> >>>>>.endm
> > >>> >>>>>
> > >>> >>>>>--
> > >>> >>>>>2.37.2.672.g94769d06f0-goog
> > >>> >>>>>
> > >>> >>>>>_______________________________________________
> > >>> >>>>>x265-devel mailing list
> > >>> >>>>>x265-devel at videolan.org
> > >>> >>>>>https://mailman.videolan.org/listinfo/x265-devel
> > >>> >>>
> > >>> >>>>_______________________________________________
> > >>> >>>>x265-devel mailing list
> > >>> >>>>x265-devel at videolan.org
> > >>> >>>>https://mailman.videolan.org/listinfo/x265-devel
> > >>> >>>
> > >>> >_______________________________________________
> > >>> >x265-devel mailing list
> > >>> >x265-devel at videolan.org
> > >>> >https://mailman.videolan.org/listinfo/x265-devel
> > >>>
> > >>> _______________________________________________
> > >>> x265-devel mailing list
> > >>> x265-devel at videolan.org
> > >>> https://mailman.videolan.org/listinfo/x265-devel
> > >>
> > >>
> > >>
> > >> --
> > >> 宋方睿
> > >>
> > >> _______________________________________________
> > >> x265-devel mailing list
> > >> x265-devel at videolan.org
> > >> https://mailman.videolan.org/listinfo/x265-devel
> > >
> > >
> > >
> > > --
> > > 宋方睿
> > >
> > > _______________________________________________
> > > x265-devel mailing list
> > > x265-devel at videolan.org
> > > https://mailman.videolan.org/listinfo/x265-devel
> >
> >
> >
> > --
> > 宋方睿
>
> Ping. This fixes a copy relocation for aarch64.
Ping https://mailman.videolan.org/pipermail/x265-devel/2022-August/013487.html
More information about the x265-devel
mailing list