Re: [libav-devel] [PATCH] x86: Don't declare a non-static function as inline

2018-04-15 Thread Diego Biurrun
On Sat, Apr 14, 2018 at 10:40:26PM +0300, Martin Storsjö wrote:
> This fixes building with clang in msvc mode, which does support
> gcc style inline assembly.
> ---
>  libavcodec/x86/xvididct_sse2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

probably OK

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] x86: Don't declare a non-static function as inline

2018-04-14 Thread Martin Storsjö

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
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] x86: Don't declare a non-static function as inline

2018-04-14 Thread Diego Biurrun
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
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] x86: Don't declare a non-static function as inline

2018-04-14 Thread Luca Barbato
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
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel