mathops: fix MULL() when the compiler does not inline the function.

Message ID 1300236870-3881-1-git-send-email-justin.ruggles@gmail.com
State Committed
Commit 79414257e23a0dee82a9978b5444ae8953376221
Headers show

Commit Message

Justin Ruggles March 16, 2011, 12:54 a.m.
If the function is not inlined, an immmediate cannot be used for the
shift parameter, so the %cl register must be used instead in that case.

This fixes compilation for x86-32 using gcc with --disable-optimizations.
---
 libavcodec/x86/mathops.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Mans Rullgard March 16, 2011, 12:59 a.m. | #1
Justin Ruggles <justin.ruggles@gmail.com> writes:

> If the function is not inlined, an immmediate cannot be used for the
> shift parameter, so the %cl register must be used instead in that case.
>
> This fixes compilation for x86-32 using gcc with --disable-optimizations.

Workarounds for that silly flag should be implemented with extreme caution.
I seriously question the utility of --disable-optimizations.  When would
anyone want to use it?

> ---
>  libavcodec/x86/mathops.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
>
> diff --git a/libavcodec/x86/mathops.h b/libavcodec/x86/mathops.h
> index b183027..33d9a6c 100644
> --- a/libavcodec/x86/mathops.h
> +++ b/libavcodec/x86/mathops.h
> @@ -35,7 +35,7 @@ static av_always_inline av_const int MULL(int a, int b, unsigned shift)
>          "imull %3               \n\t"
>          "shrdl %4, %%edx, %%eax \n\t"
>          :"=a"(rt), "=d"(dummy)
> -        :"a"(a), "rm"(b), "i"(shift)
> +        :"a"(a), "rm"(b), "ci"((uint8_t)shift)

I would only do this after double and triple-checking that gcc doesn't
decide to use the register even where it doesn't need to.
Justin Ruggles March 16, 2011, 1:26 a.m. | #2
On 03/15/2011 08:59 PM, Måns Rullgård wrote:

> Justin Ruggles <justin.ruggles@gmail.com> writes:
> 
>> If the function is not inlined, an immmediate cannot be used for the
>> shift parameter, so the %cl register must be used instead in that case.
>>
>> This fixes compilation for x86-32 using gcc with --disable-optimizations.
> 
> Workarounds for that silly flag should be implemented with extreme caution.
> I seriously question the utility of --disable-optimizations.  When would
> anyone want to use it?
> 
>> ---
>>  libavcodec/x86/mathops.h |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>>
>> diff --git a/libavcodec/x86/mathops.h b/libavcodec/x86/mathops.h
>> index b183027..33d9a6c 100644
>> --- a/libavcodec/x86/mathops.h
>> +++ b/libavcodec/x86/mathops.h
>> @@ -35,7 +35,7 @@ static av_always_inline av_const int MULL(int a, int b, unsigned shift)
>>          "imull %3               \n\t"
>>          "shrdl %4, %%edx, %%eax \n\t"
>>          :"=a"(rt), "=d"(dummy)
>> -        :"a"(a), "rm"(b), "i"(shift)
>> +        :"a"(a), "rm"(b), "ci"((uint8_t)shift)
> 
> I would only do this after double and triple-checking that gcc doesn't
> decide to use the register even where it doesn't need to.


ok, I double and triple-checked.

MULL() is used in mpegaudiodec.c and lsp.c.
gcc always uses an immediate in shrd when --disable-optimizations is not
used.

-Justin
Mans Rullgard March 16, 2011, 1:42 a.m. | #3
Justin Ruggles <justin.ruggles@gmail.com> writes:

> On 03/15/2011 08:59 PM, Måns Rullgård wrote:
>
>> Justin Ruggles <justin.ruggles@gmail.com> writes:
>> 
>>> If the function is not inlined, an immmediate cannot be used for the
>>> shift parameter, so the %cl register must be used instead in that case.
>>>
>>> This fixes compilation for x86-32 using gcc with --disable-optimizations.
>> 
>> Workarounds for that silly flag should be implemented with extreme caution.
>> I seriously question the utility of --disable-optimizations.  When would
>> anyone want to use it?
>> 
>>> ---
>>>  libavcodec/x86/mathops.h |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>>
>>> diff --git a/libavcodec/x86/mathops.h b/libavcodec/x86/mathops.h
>>> index b183027..33d9a6c 100644
>>> --- a/libavcodec/x86/mathops.h
>>> +++ b/libavcodec/x86/mathops.h
>>> @@ -35,7 +35,7 @@ static av_always_inline av_const int MULL(int a, int b, unsigned shift)
>>>          "imull %3               \n\t"
>>>          "shrdl %4, %%edx, %%eax \n\t"
>>>          :"=a"(rt), "=d"(dummy)
>>> -        :"a"(a), "rm"(b), "i"(shift)
>>> +        :"a"(a), "rm"(b), "ci"((uint8_t)shift)
>> 
>> I would only do this after double and triple-checking that gcc doesn't
>> decide to use the register even where it doesn't need to.
>
> ok, I double and triple-checked.
>
> MULL() is used in mpegaudiodec.c and lsp.c.
> gcc always uses an immediate in shrd when --disable-optimizations is not
> used.

OK then.

Patch

diff --git a/libavcodec/x86/mathops.h b/libavcodec/x86/mathops.h
index b183027..33d9a6c 100644
--- a/libavcodec/x86/mathops.h
+++ b/libavcodec/x86/mathops.h
@@ -35,7 +35,7 @@  static av_always_inline av_const int MULL(int a, int b, unsigned shift)
         "imull %3               \n\t"
         "shrdl %4, %%edx, %%eax \n\t"
         :"=a"(rt), "=d"(dummy)
-        :"a"(a), "rm"(b), "i"(shift)
+        :"a"(a), "rm"(b), "ci"((uint8_t)shift)
     );
     return rt;
 }