x86: Don't declare a non-static function as inline
diff mbox series

Message ID 1523702310-32042-1-git-send-email-martin@martin.st
State New
Headers show
Series
  • x86: Don't declare a non-static function as inline
Related show

Commit Message

Martin Storsjö April 14, 2018, 10:38 a.m. 
Make the actual implementation static inline, but add a non-static
non-inline frontend for it.

This fixes building with clang in msvc mode, which does support
gcc style inline assembly.
---
 libavcodec/x86/xvididct_sse2.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Luca Barbato April 14, 2018, 11:47 a.m. | #1 
On 14/04/2018 19:38, Martin Storsjö wrote:
> Make the actual implementation static inline, but add a non-static
> non-inline frontend for it.
> 
> This fixes building with clang in msvc mode, which does support
> gcc style inline assembly.
> ---
>  libavcodec/x86/xvididct_sse2.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/x86/xvididct_sse2.c b/libavcodec/x86/xvididct_sse2.c
> index f318e95..00ed803 100644
> --- a/libavcodec/x86/xvididct_sse2.c
> +++ b/libavcodec/x86/xvididct_sse2.c
> @@ -342,7 +342,7 @@ DECLARE_ASM_CONST(16, int32_t, walkenIdctRounders)[] = {
>      "movdqa   %%xmm6, 4*16("dct")     \n\t" \
>      "movdqa   "SREG2", 7*16("dct")    \n\t"
>  
> -inline void ff_xvid_idct_sse2(short *block)
> +static inline void xvid_idct_sse2(short *block)
>  {
>      __asm__ volatile (
>          "movq     "MANGLE (m127) ", %%mm0                              \n\t"
> @@ -390,15 +390,20 @@ inline void ff_xvid_idct_sse2(short *block)
>            "%eax", "%ecx", "%edx", "%esi", "memory");
>  }
>  
> +void ff_xvid_idct_sse2(short *block)
> +{
> +    xvid_idct_sse2(block);
> +}
> +
>  void ff_xvid_idct_sse2_put(uint8_t *dest, ptrdiff_t line_size, short *block)
>  {
> -    ff_xvid_idct_sse2(block);
> +    xvid_idct_sse2(block);
>      ff_put_pixels_clamped_mmx(block, dest, line_size);
>  }
>  
>  void ff_xvid_idct_sse2_add(uint8_t *dest, ptrdiff_t line_size, short *block)
>  {
> -    ff_xvid_idct_sse2(block);
> +    xvid_idct_sse2(block);
>      ff_add_pixels_clamped_mmx(block, dest, line_size);
>  }
>  
> 

Sure
Diego Biurrun April 14, 2018, 3:01 p.m. | #2 
On Sat, Apr 14, 2018 at 01:38:30PM +0300, Martin Storsjö wrote:
> Make the actual implementation static inline, but add a non-static
> non-inline frontend for it.
> 
> This fixes building with clang in msvc mode, which does support
> gcc style inline assembly.
> --- a/libavcodec/x86/xvididct_sse2.c
> +++ b/libavcodec/x86/xvididct_sse2.c
> @@ -342,7 +342,7 @@ DECLARE_ASM_CONST(16, int32_t, walkenIdctRounders)[] = {
>  
> -inline void ff_xvid_idct_sse2(short *block)
> +static inline void xvid_idct_sse2(short *block)
>  {
>      __asm__ volatile (
>          "movq     "MANGLE (m127) ", %%mm0                              \n\t"
> @@ -390,15 +390,20 @@ inline void ff_xvid_idct_sse2(short *block)
>  
> +void ff_xvid_idct_sse2(short *block)
> +{
> +    xvid_idct_sse2(block);
> +}

Why not simply drop the inline and be done with it? I notice that the MMX
version of this does not have the inline keyword.

Diego
Martin Storsjö April 14, 2018, 6:18 p.m. | #3 
On Sat, 14 Apr 2018, Diego Biurrun wrote:

> On Sat, Apr 14, 2018 at 01:38:30PM +0300, Martin Storsjö wrote:
>> Make the actual implementation static inline, but add a non-static
>> non-inline frontend for it.
>> 
>> This fixes building with clang in msvc mode, which does support
>> gcc style inline assembly.
>> --- a/libavcodec/x86/xvididct_sse2.c
>> +++ b/libavcodec/x86/xvididct_sse2.c
>> @@ -342,7 +342,7 @@ DECLARE_ASM_CONST(16, int32_t, walkenIdctRounders)[] = {
>> 
>> -inline void ff_xvid_idct_sse2(short *block)
>> +static inline void xvid_idct_sse2(short *block)
>>  {
>>      __asm__ volatile (
>>          "movq     "MANGLE (m127) ", %%mm0                              \n\t"
>> @@ -390,15 +390,20 @@ inline void ff_xvid_idct_sse2(short *block)
>> 
>> +void ff_xvid_idct_sse2(short *block)
>> +{
>> +    xvid_idct_sse2(block);
>> +}
>
> Why not simply drop the inline and be done with it? I notice that the MMX
> version of this does not have the inline keyword.

That's probably just as good.

// Martin

Patch
diff mbox series 

diff --git a/libavcodec/x86/xvididct_sse2.c b/libavcodec/x86/xvididct_sse2.c
index f318e95..00ed803 100644
--- a/libavcodec/x86/xvididct_sse2.c
+++ b/libavcodec/x86/xvididct_sse2.c
@@ -342,7 +342,7 @@  DECLARE_ASM_CONST(16, int32_t, walkenIdctRounders)[] = {
     "movdqa   %%xmm6, 4*16("dct")     \n\t" \
     "movdqa   "SREG2", 7*16("dct")    \n\t"
 
-inline void ff_xvid_idct_sse2(short *block)
+static inline void xvid_idct_sse2(short *block)
 {
     __asm__ volatile (
         "movq     "MANGLE (m127) ", %%mm0                              \n\t"
@@ -390,15 +390,20 @@  inline void ff_xvid_idct_sse2(short *block)
           "%eax", "%ecx", "%edx", "%esi", "memory");
 }
 
+void ff_xvid_idct_sse2(short *block)
+{
+    xvid_idct_sse2(block);
+}
+
 void ff_xvid_idct_sse2_put(uint8_t *dest, ptrdiff_t line_size, short *block)
 {
-    ff_xvid_idct_sse2(block);
+    xvid_idct_sse2(block);
     ff_put_pixels_clamped_mmx(block, dest, line_size);
 }
 
 void ff_xvid_idct_sse2_add(uint8_t *dest, ptrdiff_t line_size, short *block)
 {
-    ff_xvid_idct_sse2(block);
+    xvid_idct_sse2(block);
     ff_add_pixels_clamped_mmx(block, dest, line_size);
 }