diff mbox series

[FFmpeg-devel] get_cabac_inline_x86: Don't inline the assembly function on 32 bit

Message ID 20230328105347.93335-1-martin@martin.st
State Accepted
Commit 182663a58a7a099e02e76da3b0f96d63e5c26a6d
Headers show
Series [FFmpeg-devel] get_cabac_inline_x86: Don't inline the assembly function on 32 bit | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Martin Storsjö March 28, 2023, 10:53 a.m. UTC 
From: Christopher Degawa <ccom@randomderp.com>

While the inline cabac assembly has worked correctly in i386 builds
historically, modern compiler updates has started showing issues
with it, when the function gets inlined into larger contexts that
fail to provide the amount of free registers as this function
requires.

This was an issue with Clang on Windows on i386, which was fixed
in c6d284b945324a7bc70ea8b9056040c8148aa835. However, recently
the same issues also have started showing up with GCC (both for
Windows and Linux). Whether the issue appears seems dependent on
a lot of optimizer tuning (e.g. the issue appears or goes away
depenent on the combinaton of -march= and -mtune= options),
potentially due to the compiler making different decisions on
how much to inline.

Fixes: https://trac.ffmpeg.org/ticket/8903

Signed-off-by: Martin Storsjö <martin@martin.st>
---
 libavcodec/x86/cabac.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Martin Storsjö March 31, 2023, 6:52 a.m. UTC | #1 
On Tue, 28 Mar 2023, Martin Storsjö wrote:

> From: Christopher Degawa <ccom@randomderp.com>
>
> While the inline cabac assembly has worked correctly in i386 builds
> historically, modern compiler updates has started showing issues
> with it, when the function gets inlined into larger contexts that
> fail to provide the amount of free registers as this function
> requires.
>
> This was an issue with Clang on Windows on i386, which was fixed
> in c6d284b945324a7bc70ea8b9056040c8148aa835. However, recently
> the same issues also have started showing up with GCC (both for
> Windows and Linux). Whether the issue appears seems dependent on
> a lot of optimizer tuning (e.g. the issue appears or goes away
> depenent on the combinaton of -march= and -mtune= options),
> potentially due to the compiler making different decisions on
> how much to inline.
>
> Fixes: https://trac.ffmpeg.org/ticket/8903
>
> Signed-off-by: Martin Storsjö <martin@martin.st>
> ---
> libavcodec/x86/cabac.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/x86/cabac.h b/libavcodec/x86/cabac.h
> index b046a56a6b..ce2aefcbac 100644
> --- a/libavcodec/x86/cabac.h
> +++ b/libavcodec/x86/cabac.h
> @@ -178,7 +178,7 @@
> #if HAVE_7REGS && !BROKEN_COMPILER
> #define get_cabac_inline get_cabac_inline_x86
> static
> -#if defined(_WIN32) && !defined(_WIN64) && defined(__clang__)
> +#if ARCH_X86_32
> av_noinline
> #else
> av_always_inline
> -- 
> 2.37.1 (Apple Git-137.1)

Will push this today if there's no objections.

// Martin
diff mbox series

Patch

diff --git a/libavcodec/x86/cabac.h b/libavcodec/x86/cabac.h
index b046a56a6b..ce2aefcbac 100644
--- a/libavcodec/x86/cabac.h
+++ b/libavcodec/x86/cabac.h
@@ -178,7 +178,7 @@ 
 #if HAVE_7REGS && !BROKEN_COMPILER
 #define get_cabac_inline get_cabac_inline_x86
 static
-#if defined(_WIN32) && !defined(_WIN64) && defined(__clang__)
+#if ARCH_X86_32
 av_noinline
 #else
 av_always_inline