[FFmpeg-devel] [PATCH] get_cabac_inline_x86: Don't inline if 32-bit Windows

[Christopher Degawa]

On Mon, Mar 27, 2023 at 7:35 AM Martin Storsjö <martin at martin.st> wrote:

> On Mon, 2 Jan 2023, Christopher Degawa wrote:
>
> > previouslly, it only was an issue with 32-bit clang from msys2's
> > mingw32 repo, however, at some point with an update to gcc 12.2.0,
> > the same issue popped up. Tested with a clean clone of ffmpeg, and even
> > tested with n5.0, but the issue persists, so I presume it's a compiler
> > issue.
> >
> > Related: https://trac.ffmpeg.org/ticket/8903
> >
> > Signed-off-by: Christopher Degawa <ccom at randomderp.com>
> > ---
> > 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..70f990db8d 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 defined(_WIN32) && !defined(_WIN64)
> > av_noinline
> > #else
> > av_always_inline
> > --
> > 2.39.0
>
> I'm ok with this change (although I'd reword the commit message a bit);
> the inline assembly here is brittle and it's easy to trigger this failure.
>
> It's very hard to say whether this is a compiler bug, or just our assembly
> crossing the line for what we can demand that the compiler accommodates,
> in combination with deep inlineing - the patch avoids inlineing the
> function which makes it much more possible to handle the inline assembly
> constraints.
>
> I and Hendrik discussed the issue on irc; I reproduced the issue with
> MSYS2's 32 bit GCC, and Hendrik also separately reproduced it with a
> separate build of GCC, by adding the options "-march=pentium4
> -mtune=generic", which are MSYS2's current defaults, which trigger the
> failure (at least in mingw builds, with GCC 12.2.0).
>
> So if there's no reasonable opposition, I'd go ahead and push this patch
> with a slightly reworded commit message.
>
> // Martin
>

This may need to be re-done since according to
https://trac.ffmpeg.org/ticket/8903,
it seems to be affecting linux 32-bit as well with the latest compilers.