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

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

[Christopher Degawa]

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@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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Hendrik Leppkes]

On Tue, Jan 3, 2023 at 12:01 AM Christopher Degawa <ccom@randomderp.com> 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
>

I regularly build with 12.2 on win32 and its fine.

In fact, there is a fate station for that:
https://fate.ffmpeg.org/report.cgi?slot=x86_32-mingw-w64-dll-windows-native&time=20230102232810

So if you are seeing this issue, more details that trigger it will be
required, and maybe a more targeted fix.

- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Christopher Degawa]

On Mon, Jan 2, 2023 at 5:36 PM Hendrik Leppkes <h.leppkes@gmail.com> wrote:

> On Tue, Jan 3, 2023 at 12:01 AM Christopher Degawa <ccom@randomderp.com>
> wrote:
> >
>
> I regularly build with 12.2 on win32 and its fine.
>
> In fact, there is a fate station for that:
>
> https://fate.ffmpeg.org/report.cgi?slot=x86_32-mingw-w64-dll-windows-native&time=20230102232810
>
> So if you are seeing this issue, more details that trigger it will be
> required, and maybe a more targeted fix.
>
> - Hendrik

I can try to see if I can narrow down the configuration more, and try the
one used in fate, but for now, I was reproducing it by using ../configure
&& make -j 12

As additional information for now, I'm using

Target: i686-w64-mingw32
gcc version 12.2.0 (Rev7, Built by MSYS2 project)

with no notable environment variables like CFLAGS etc.
I did make sure to update my packages from msys2.

Interestingly, when I ran "../configure  --enable-gpl
--enable-memory-poisoning --arch=x86 --cpu=i686 --enable-shared" there were
no errors, but did confirm that with just "../configure
 && make libavcodec/h264_cabac.o" the error reappeared.

CC      libavcodec/h264_cabac.o
In file included from C:/Users/cddeg/FFmpeg/libavcodec/cabac_functions.h:49,
                 from C:/Users/cddeg/FFmpeg/libavcodec/h264_cabac.c:36:
In function 'get_cabac_inline_x86',
    inlined from 'get_cabac' at
C:/Users/cddeg/FFmpeg/libavcodec/cabac_functions.h:145:12,
    inlined from 'decode_cabac_mb_intra4x4_pred_mode' at
C:/Users/cddeg/FFmpeg/libavcodec/h264_cabac.c:1377:9,
    inlined from 'ff_h264_decode_mb_cabac' at
C:/Users/cddeg/FFmpeg/libavcodec/h264_cabac.c:2081:32:
C:/Users/cddeg/FFmpeg/libavcodec/x86/cabac.h:199:5: error: 'asm' operand
has impossible constraints
  199 |     __asm__ volatile(
      |     ^~~~~~~
C:/Users/cddeg/FFmpeg/libavcodec/x86/cabac.h:199:5: error: 'asm' operand
has impossible constraints
C:/Users/cddeg/FFmpeg/libavcodec/x86/cabac.h:199:5: error: 'asm' operand
has impossible constraints
C:/Users/cddeg/FFmpeg/libavcodec/x86/cabac.h:199:5: error: 'asm' operand
has impossible constraints
make: *** [/c/Users/cddeg/FFmpeg/ffbuild/common.mak:81:
libavcodec/h264_cabac.o] Error 1


I will try to see which flag in that configure line causes the issues to
disappear
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Christopher Degawa]

On Tue, Jan 3, 2023 at 12:32 PM Christopher Degawa <ccom@randomderp.com>
wrote:

>
>
> On Mon, Jan 2, 2023 at 5:36 PM Hendrik Leppkes <h.leppkes@gmail.com>
> wrote:
>
>> On Tue, Jan 3, 2023 at 12:01 AM Christopher Degawa <ccom@randomderp.com>
>> wrote:
>> >
>>
>> I regularly build with 12.2 on win32 and its fine.
>>
>> In fact, there is a fate station for that:
>>
>> https://fate.ffmpeg.org/report.cgi?slot=x86_32-mingw-w64-dll-windows-native&time=20230102232810
>>
>> So if you are seeing this issue, more details that trigger it will be
>> required, and maybe a more targeted fix.
>>
>> - Hendrik
>
> I will try to see which flag in that configure line causes the issues to
> disappear
>

Seems I got it to disappear with just "--cpu=i686", so "ARCH
       x86 (generic)" fails but "ARCH                      x86 (i686)"
passes
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Andreas Rheinhardt]

Christopher Degawa:
> 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@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

Have these presumed compiler bugs ever been reported upstream?

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Christopher Degawa]

On Tue, Jan 3, 2023 at 4:16 AM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Christopher Degawa:
> > 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.
>
> Have these presumed compiler bugs ever been reported upstream?
>
> - Andreas
>

 I'm not sure if it's necessarily a compiler bug, rather I'm suspecting
something was changed in msys2's i686 gcc's. Based on my small findings, it
might be related to what “generic” means for -mtune for the 32-bit
compiler. I was asking around msys2's channels to see if anyone else could
reproduce it, but nobody has responded yet.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Christopher Degawa]

On Tue, Jan 3, 2023 at 12:51 PM Christopher Degawa <ccom@randomderp.com>
wrote:

>
>
> On Tue, Jan 3, 2023 at 4:16 AM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
>
>> Christopher Degawa:
>> > 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.
>>
>> Have these presumed compiler bugs ever been reported upstream?
>>
>> - Andreas
>>
>
>  I'm not sure if it's necessarily a compiler bug, rather I'm suspecting
> something was changed in msys2's i686 gcc's. Based on my small findings, it
> might be related to what “generic” means for -mtune for the 32-bit
> compiler. I was asking around msys2's channels to see if anyone else could
> reproduce it, but nobody has responded yet.
>

Seems some of them have encountered it
https://github.com/msys2/MINGW-packages/pull/11683
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Martin Storsjö]

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@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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [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@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@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Martin Storsjö]

On Mon, 27 Mar 2023, Christopher Degawa wrote:

> 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.

Oh, interesting - both me and Hendrik tried to reproduce it with 32 bit 
compilers for Linux today, but the combination that triggered it on 
Windows (-march=pentium4 -mtune=generic) didn't trigger it there.

With that in mind, I think it's rather clear that we should make this 
function noinline for all i386 combinations; yes it used to work back in 
the day, but compilers these days do way more things than they used to do, 
and inlineing this everywhere might not be feasible on i386.

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".