[x265] [PATCH] aarch64: replace ldr pseudo-instruction with adrp+add

[chen]

Hi Song,


In your patch, it looks you just change default mode from Absolute to Relative addres.
I understand newer OS may required binary execute code without text relocation, such as Android 6 and above, IOS, etc.
But we may also generate these relative address by cmake option ENABLE_PIC. I suggest keep current code, so the user may configuration by themselves.


btw: the ENABLE_PIC looks just work with GCC, so I think we need take a look these option on Apple platform.


Regards,
Min Chen
At 2022-08-30 13:42:39, "Fangrui Song" <maskray at google.com> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20220830/bbd8fd3a/attachment-0001.html>