x86: fft: Only use the PLT on ELF

Message ID 1340709794-48765-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö June 26, 2012, 11:23 a.m.
This fixes builds on x86_64 darwin.
---
Due to the duplicated lines, the diff shows the change in a much more
convoluted way than what actually is intended.

 configure                  |    5 ++++-
 libavcodec/x86/fft_mmx.asm |    4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Mans Rullgard June 26, 2012, 11:26 a.m. | #1
Martin Storsjö <martin@martin.st> writes:

> This fixes builds on x86_64 darwin.
> ---
> Due to the duplicated lines, the diff shows the change in a much more
> convoluted way than what actually is intended.
>
>  configure                  |    5 ++++-
>  libavcodec/x86/fft_mmx.asm |    4 ++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index aa1856d..7074194 100755
> --- a/configure
> +++ b/configure
> @@ -2773,7 +2773,10 @@ EOF
>          enabled pic               && append YASMFLAGS "-DPIC"
>          test -n "$extern_prefix"  && append YASMFLAGS "-DPREFIX"
>          case "$objformat" in
> -            elf*) enabled debug && append YASMFLAGS $yasm_debug ;;
> +            elf*)
> +                enabled debug && append YASMFLAGS $yasm_debug
> +                append YASMFLAGS "-DELF"
> +            ;;
>          esac
>
>          check_yasm "pextrd [eax], xmm0, 1" && enable yasm ||
> diff --git a/libavcodec/x86/fft_mmx.asm b/libavcodec/x86/fft_mmx.asm
> index 007f5ca..a4f7d4d 100644
> --- a/libavcodec/x86/fft_mmx.asm
> +++ b/libavcodec/x86/fft_mmx.asm
> @@ -648,11 +648,15 @@ cglobal fft_permute, 2,7,1
>      RET
>  %elif ARCH_X86_64
>  %ifdef PIC
> +%ifdef ELF
>      jmp     memcpy wrt ..plt
>  %else
>      jmp     memcpy
>  %endif
>  %else
> +    jmp     memcpy
> +%endif
> +%else

Is there no way to do if PIC && ELF on one line?
Martin Storsjö June 26, 2012, 11:28 a.m. | #2
On Tue, 26 Jun 2012, Måns Rullgård wrote:

> Martin Storsjö <martin@martin.st> writes:
>
>> This fixes builds on x86_64 darwin.
>> ---
>> Due to the duplicated lines, the diff shows the change in a much more
>> convoluted way than what actually is intended.
>>
>>  configure                  |    5 ++++-
>>  libavcodec/x86/fft_mmx.asm |    4 ++++
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index aa1856d..7074194 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2773,7 +2773,10 @@ EOF
>>          enabled pic               && append YASMFLAGS "-DPIC"
>>          test -n "$extern_prefix"  && append YASMFLAGS "-DPREFIX"
>>          case "$objformat" in
>> -            elf*) enabled debug && append YASMFLAGS $yasm_debug ;;
>> +            elf*)
>> +                enabled debug && append YASMFLAGS $yasm_debug
>> +                append YASMFLAGS "-DELF"
>> +            ;;
>>          esac
>>
>>          check_yasm "pextrd [eax], xmm0, 1" && enable yasm ||
>> diff --git a/libavcodec/x86/fft_mmx.asm b/libavcodec/x86/fft_mmx.asm
>> index 007f5ca..a4f7d4d 100644
>> --- a/libavcodec/x86/fft_mmx.asm
>> +++ b/libavcodec/x86/fft_mmx.asm
>> @@ -648,11 +648,15 @@ cglobal fft_permute, 2,7,1
>>      RET
>>  %elif ARCH_X86_64
>>  %ifdef PIC
>> +%ifdef ELF
>>      jmp     memcpy wrt ..plt
>>  %else
>>      jmp     memcpy
>>  %endif
>>  %else
>> +    jmp     memcpy
>> +%endif
>> +%else
>
> Is there no way to do if PIC && ELF on one line?

I tried the obvious
%if defined(PIC) && defined(ELF)
yielding
fft_mmx.asm:650: error: cannot reference symbol `defined' in preprocessor

And from their manual, I gather that the only way to check defines is via 
the special %ifdef, %ifndef, %elifdef and so on.

// Martin
Martin Storsjö June 26, 2012, 11:31 a.m. | #3
On Tue, 26 Jun 2012, Martin Storsjö wrote:

> On Tue, 26 Jun 2012, Måns Rullgård wrote:
>
>> Martin Storsjö <martin@martin.st> writes:
>> 
>>> This fixes builds on x86_64 darwin.
>>> ---
>>> Due to the duplicated lines, the diff shows the change in a much more
>>> convoluted way than what actually is intended.
>>>
>>>  configure                  |    5 ++++-
>>>  libavcodec/x86/fft_mmx.asm |    4 ++++
>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/configure b/configure
>>> index aa1856d..7074194 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2773,7 +2773,10 @@ EOF
>>>          enabled pic               && append YASMFLAGS "-DPIC"
>>>          test -n "$extern_prefix"  && append YASMFLAGS "-DPREFIX"
>>>          case "$objformat" in
>>> -            elf*) enabled debug && append YASMFLAGS $yasm_debug ;;
>>> +            elf*)
>>> +                enabled debug && append YASMFLAGS $yasm_debug
>>> +                append YASMFLAGS "-DELF"
>>> +            ;;
>>>          esac
>>>
>>>          check_yasm "pextrd [eax], xmm0, 1" && enable yasm ||
>>> diff --git a/libavcodec/x86/fft_mmx.asm b/libavcodec/x86/fft_mmx.asm
>>> index 007f5ca..a4f7d4d 100644
>>> --- a/libavcodec/x86/fft_mmx.asm
>>> +++ b/libavcodec/x86/fft_mmx.asm
>>> @@ -648,11 +648,15 @@ cglobal fft_permute, 2,7,1
>>>      RET
>>>  %elif ARCH_X86_64
>>>  %ifdef PIC
>>> +%ifdef ELF
>>>      jmp     memcpy wrt ..plt
>>>  %else
>>>      jmp     memcpy
>>>  %endif
>>>  %else
>>> +    jmp     memcpy
>>> +%endif
>>> +%else
>> 
>> Is there no way to do if PIC && ELF on one line?
>
> I tried the obvious
> %if defined(PIC) && defined(ELF)
> yielding
> fft_mmx.asm:650: error: cannot reference symbol `defined' in preprocessor
>
> And from their manual, I gather that the only way to check defines is via the 
> special %ifdef, %ifndef, %elifdef and so on.

... unless we define PIC/ELF as 0 or 1, but that's a bit bigger change, 
especially with x86inc.asm which I guess we should keep as close to x264 
as possible.

// Martin
Mans Rullgard June 26, 2012, 11:37 a.m. | #4
Martin Storsjö <martin@martin.st> writes:

> On Tue, 26 Jun 2012, Martin Storsjö wrote:
>
>> On Tue, 26 Jun 2012, Måns Rullgård wrote:
>>
>>> Martin Storsjö <martin@martin.st> writes:
>>>
>>>> This fixes builds on x86_64 darwin.
>>>> ---
>>>> Due to the duplicated lines, the diff shows the change in a much more
>>>> convoluted way than what actually is intended.
>>>>
>>>>  configure                  |    5 ++++-
>>>>  libavcodec/x86/fft_mmx.asm |    4 ++++
>>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index aa1856d..7074194 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -2773,7 +2773,10 @@ EOF
>>>>          enabled pic               && append YASMFLAGS "-DPIC"
>>>>          test -n "$extern_prefix"  && append YASMFLAGS "-DPREFIX"
>>>>          case "$objformat" in
>>>> -            elf*) enabled debug && append YASMFLAGS $yasm_debug ;;
>>>> +            elf*)
>>>> +                enabled debug && append YASMFLAGS $yasm_debug
>>>> +                append YASMFLAGS "-DELF"
>>>> +            ;;
>>>>          esac
>>>>
>>>>          check_yasm "pextrd [eax], xmm0, 1" && enable yasm ||
>>>> diff --git a/libavcodec/x86/fft_mmx.asm b/libavcodec/x86/fft_mmx.asm
>>>> index 007f5ca..a4f7d4d 100644
>>>> --- a/libavcodec/x86/fft_mmx.asm
>>>> +++ b/libavcodec/x86/fft_mmx.asm
>>>> @@ -648,11 +648,15 @@ cglobal fft_permute, 2,7,1
>>>>      RET
>>>>  %elif ARCH_X86_64
>>>>  %ifdef PIC
>>>> +%ifdef ELF
>>>>      jmp     memcpy wrt ..plt
>>>>  %else
>>>>      jmp     memcpy
>>>>  %endif
>>>>  %else
>>>> +    jmp     memcpy
>>>> +%endif
>>>> +%else
>>>
>>> Is there no way to do if PIC && ELF on one line?
>>
>> I tried the obvious
>> %if defined(PIC) && defined(ELF)
>> yielding
>> fft_mmx.asm:650: error: cannot reference symbol `defined' in preprocessor
>>
>> And from their manual, I gather that the only way to check defines
>> is via the special %ifdef, %ifndef, %elifdef and so on.
>
> ... unless we define PIC/ELF as 0 or 1, but that's a bit bigger
> change, especially with x86inc.asm which I guess we should keep as
> close to x264 as possible.

The cleaner, and long-term better, solution would be to define some
macros to add the wrt ..plt automatically for ELF.  This could be done
either as CALL/JMP macros or by redefining the symbol when declaring it
extern.
Martin Storsjö June 26, 2012, 11:45 a.m. | #5
On Tue, 26 Jun 2012, Måns Rullgård wrote:

> Martin Storsjö <martin@martin.st> writes:
>
>> On Tue, 26 Jun 2012, Martin Storsjö wrote:
>>
>>> On Tue, 26 Jun 2012, Måns Rullgård wrote:
>>>
>>>> Martin Storsjö <martin@martin.st> writes:
>>>>
>>>>> This fixes builds on x86_64 darwin.
>>>>> ---
>>>>> Due to the duplicated lines, the diff shows the change in a much more
>>>>> convoluted way than what actually is intended.
>>>>>
>>>>>  configure                  |    5 ++++-
>>>>>  libavcodec/x86/fft_mmx.asm |    4 ++++
>>>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/configure b/configure
>>>>> index aa1856d..7074194 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -2773,7 +2773,10 @@ EOF
>>>>>          enabled pic               && append YASMFLAGS "-DPIC"
>>>>>          test -n "$extern_prefix"  && append YASMFLAGS "-DPREFIX"
>>>>>          case "$objformat" in
>>>>> -            elf*) enabled debug && append YASMFLAGS $yasm_debug ;;
>>>>> +            elf*)
>>>>> +                enabled debug && append YASMFLAGS $yasm_debug
>>>>> +                append YASMFLAGS "-DELF"
>>>>> +            ;;
>>>>>          esac
>>>>>
>>>>>          check_yasm "pextrd [eax], xmm0, 1" && enable yasm ||
>>>>> diff --git a/libavcodec/x86/fft_mmx.asm b/libavcodec/x86/fft_mmx.asm
>>>>> index 007f5ca..a4f7d4d 100644
>>>>> --- a/libavcodec/x86/fft_mmx.asm
>>>>> +++ b/libavcodec/x86/fft_mmx.asm
>>>>> @@ -648,11 +648,15 @@ cglobal fft_permute, 2,7,1
>>>>>      RET
>>>>>  %elif ARCH_X86_64
>>>>>  %ifdef PIC
>>>>> +%ifdef ELF
>>>>>      jmp     memcpy wrt ..plt
>>>>>  %else
>>>>>      jmp     memcpy
>>>>>  %endif
>>>>>  %else
>>>>> +    jmp     memcpy
>>>>> +%endif
>>>>> +%else
>>>>
>>>> Is there no way to do if PIC && ELF on one line?
>>>
>>> I tried the obvious
>>> %if defined(PIC) && defined(ELF)
>>> yielding
>>> fft_mmx.asm:650: error: cannot reference symbol `defined' in preprocessor
>>>
>>> And from their manual, I gather that the only way to check defines
>>> is via the special %ifdef, %ifndef, %elifdef and so on.
>>
>> ... unless we define PIC/ELF as 0 or 1, but that's a bit bigger
>> change, especially with x86inc.asm which I guess we should keep as
>> close to x264 as possible.
>
> The cleaner, and long-term better, solution would be to define some
> macros to add the wrt ..plt automatically for ELF.  This could be done
> either as CALL/JMP macros or by redefining the symbol when declaring it
> extern.

Yes, that's probably a better and cleaner solution. Unfortunately, it's a 
bit out of my reach with my current yasm knowledge...

// Martin

Patch

diff --git a/configure b/configure
index aa1856d..7074194 100755
--- a/configure
+++ b/configure
@@ -2773,7 +2773,10 @@  EOF
         enabled pic               && append YASMFLAGS "-DPIC"
         test -n "$extern_prefix"  && append YASMFLAGS "-DPREFIX"
         case "$objformat" in
-            elf*) enabled debug && append YASMFLAGS $yasm_debug ;;
+            elf*)
+                enabled debug && append YASMFLAGS $yasm_debug
+                append YASMFLAGS "-DELF"
+            ;;
         esac
 
         check_yasm "pextrd [eax], xmm0, 1" && enable yasm ||
diff --git a/libavcodec/x86/fft_mmx.asm b/libavcodec/x86/fft_mmx.asm
index 007f5ca..a4f7d4d 100644
--- a/libavcodec/x86/fft_mmx.asm
+++ b/libavcodec/x86/fft_mmx.asm
@@ -648,11 +648,15 @@  cglobal fft_permute, 2,7,1
     RET
 %elif ARCH_X86_64
 %ifdef PIC
+%ifdef ELF
     jmp     memcpy wrt ..plt
 %else
     jmp     memcpy
 %endif
 %else
+    jmp     memcpy
+%endif
+%else
     push    r2
     push    r5
     push    r1