[FFmpeg-devel] [PATCH 1/2] lavu/bswap: remove some inline assembler

[FFmpeg-devel] [PATCH 1/2] lavu/bswap: remove some inline assembler

[Rémi Denis-Courmont]

C code or compiler built-ins are preferable over inline assembler for
byte-swaps as it allows for better optimisations (e.g. instruction
scheduling) which would otherwise be impossible.

As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
this removes the inline assembler on GCC (and Clang) since we now
require recent enough compiler versions (this indeed seems to work on
AArch64).
---
 libavutil/aarch64/bswap.h | 56 ---------------------------------------
 libavutil/avr32/bswap.h   | 44 ------------------------------
 libavutil/bswap.h         |  8 +-----
 libavutil/sh4/bswap.h     | 48 ---------------------------------
 4 files changed, 1 insertion(+), 155 deletions(-)
 delete mode 100644 libavutil/aarch64/bswap.h
 delete mode 100644 libavutil/avr32/bswap.h
 delete mode 100644 libavutil/sh4/bswap.h

diff --git a/libavutil/aarch64/bswap.h b/libavutil/aarch64/bswap.h
deleted file mode 100644
index 7abca657ba..0000000000
--- a/libavutil/aarch64/bswap.h
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#ifndef AVUTIL_AARCH64_BSWAP_H
-#define AVUTIL_AARCH64_BSWAP_H
-
-#include <stdint.h>
-#include "config.h"
-#include "libavutil/attributes.h"
-
-#if HAVE_INLINE_ASM
-
-#define av_bswap16 av_bswap16
-static av_always_inline av_const unsigned av_bswap16(unsigned x)
-{
-    unsigned y;
-
-    __asm__("rev16 %w0, %w1" : "=r"(y) : "r"(x));
-    return y;
-}
-
-#define av_bswap32 av_bswap32
-static av_always_inline av_const uint32_t av_bswap32(uint32_t x)
-{
-    uint32_t y;
-
-    __asm__("rev %w0, %w1" : "=r"(y) : "r"(x));
-    return y;
-}
-
-#define av_bswap64 av_bswap64
-static av_always_inline av_const uint64_t av_bswap64(uint64_t x)
-{
-    uint64_t y;
-
-    __asm__("rev %0, %1" : "=r"(y) : "r"(x));
-    return y;
-}
-
-#endif /* HAVE_INLINE_ASM */
-#endif /* AVUTIL_AARCH64_BSWAP_H */
diff --git a/libavutil/avr32/bswap.h b/libavutil/avr32/bswap.h
deleted file mode 100644
index e79d53f369..0000000000
--- a/libavutil/avr32/bswap.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#ifndef AVUTIL_AVR32_BSWAP_H
-#define AVUTIL_AVR32_BSWAP_H
-
-#include <stdint.h>
-#include "config.h"
-#include "libavutil/attributes.h"
-
-#if HAVE_INLINE_ASM
-
-#define av_bswap16 av_bswap16
-static av_always_inline av_const uint16_t av_bswap16(uint16_t x)
-{
-    __asm__ ("swap.bh %0" : "+r"(x));
-    return x;
-}
-
-#define av_bswap32 av_bswap32
-static av_always_inline av_const uint32_t av_bswap32(uint32_t x)
-{
-    __asm__ ("swap.b  %0" : "+r"(x));
-    return x;
-}
-
-#endif /* HAVE_INLINE_ASM */
-
-#endif /* AVUTIL_AVR32_BSWAP_H */
diff --git a/libavutil/bswap.h b/libavutil/bswap.h
index 1528906f93..b339c90b9b 100644
--- a/libavutil/bswap.h
+++ b/libavutil/bswap.h
@@ -34,16 +34,10 @@
 
 #include "config.h"
 
-#if   ARCH_AARCH64
-#   include "aarch64/bswap.h"
-#elif ARCH_ARM
+#if ARCH_ARM
 #   include "arm/bswap.h"
-#elif ARCH_AVR32
-#   include "avr32/bswap.h"
 #elif ARCH_RISCV
 #   include "riscv/bswap.h"
-#elif ARCH_SH4
-#   include "sh4/bswap.h"
 #elif ARCH_X86
 #   include "x86/bswap.h"
 #endif
diff --git a/libavutil/sh4/bswap.h b/libavutil/sh4/bswap.h
deleted file mode 100644
index 48dd27f806..0000000000
--- a/libavutil/sh4/bswap.h
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/**
- * @file
- * byte swapping routines
- */
-
-#ifndef AVUTIL_SH4_BSWAP_H
-#define AVUTIL_SH4_BSWAP_H
-
-#include <stdint.h>
-#include "config.h"
-#include "libavutil/attributes.h"
-
-#define av_bswap16 av_bswap16
-static av_always_inline av_const uint16_t av_bswap16(uint16_t x)
-{
-    __asm__("swap.b %0,%0" : "+r"(x));
-    return x;
-}
-
-#define av_bswap32 av_bswap32
-static av_always_inline av_const uint32_t av_bswap32(uint32_t x)
-{
-    __asm__("swap.b %0,%0\n"
-            "swap.w %0,%0\n"
-            "swap.b %0,%0\n"
-            : "+r"(x));
-    return x;
-}
-
-#endif /* AVUTIL_SH4_BSWAP_H */
-- 
2.45.1

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

[FFmpeg-devel] [PATCH 2/2] sh4: remove architecture

[Rémi Denis-Courmont]

Support for SuperH was dropped over a decade ago. There no longer is any
architecture-specific code to be found, so just remove the corresponding
test. Technically it is still possible to compile FFmpeg as the
"generic" (pure C) architecture.
---
 configure             | 4 ----
 libavcodec/sh4/README | 6 ------
 2 files changed, 10 deletions(-)
 delete mode 100644 libavcodec/sh4/README

diff --git a/configure b/configure
index 6c5b8aab9a..efead4075a 100755
--- a/configure
+++ b/configure
@@ -2146,7 +2146,6 @@ ARCH_LIST="
     ppc64
     riscv
     s390
-    sh4
     sparc
     sparc64
     tilegx
@@ -5249,9 +5248,6 @@ case "$arch" in
     s390|s390x)
         arch="s390"
     ;;
-    sh4|sh)
-        arch="sh4"
-    ;;
     sun4*|sparc*)
         arch="sparc"
     ;;
diff --git a/libavcodec/sh4/README b/libavcodec/sh4/README
deleted file mode 100644
index 8dd61fe875..0000000000
--- a/libavcodec/sh4/README
+++ /dev/null
@@ -1,6 +0,0 @@
-SH4 optimizations have been removed in
-commit d6096a67422534918405abb46dafbbac4608cbc3
-The last revission with the optimizations is cbfc9046e1c7e295b74f252902ae6f255eef4e78
-
-If you want to maintain these (or other) SH4 optimizations in ffmpeg, then please
-contact ffmpeg-devel@ffmpeg.org
-- 
2.45.1

_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Sean McGovern]

On Fri, Jun 7, 2024 at 2:20 PM Rémi Denis-Courmont <remi@remlab.net> wrote:
>
> C code or compiler built-ins are preferable over inline assembler for
> byte-swaps as it allows for better optimisations (e.g. instruction
> scheduling) which would otherwise be impossible.
>
> As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
> this removes the inline assembler on GCC (and Clang) since we now
> require recent enough compiler versions (this indeed seems to work on
> AArch64).
> ---
>  libavutil/aarch64/bswap.h | 56 ---------------------------------------
>  libavutil/avr32/bswap.h   | 44 ------------------------------
>  libavutil/bswap.h         |  8 +-----
>  libavutil/sh4/bswap.h     | 48 ---------------------------------
>  4 files changed, 1 insertion(+), 155 deletions(-)
>  delete mode 100644 libavutil/aarch64/bswap.h
>  delete mode 100644 libavutil/avr32/bswap.h
>  delete mode 100644 libavutil/sh4/bswap.h
>
> diff --git a/libavutil/aarch64/bswap.h b/libavutil/aarch64/bswap.h
> deleted file mode 100644
> index 7abca657ba..0000000000
> --- a/libavutil/aarch64/bswap.h
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -/*
> - * This file is part of FFmpeg.
> - *
> - * FFmpeg is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2.1 of the License, or (at your option) any later version.
> - *
> - * FFmpeg is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with FFmpeg; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -
> -#ifndef AVUTIL_AARCH64_BSWAP_H
> -#define AVUTIL_AARCH64_BSWAP_H
> -
> -#include <stdint.h>
> -#include "config.h"
> -#include "libavutil/attributes.h"
> -
> -#if HAVE_INLINE_ASM
> -
> -#define av_bswap16 av_bswap16
> -static av_always_inline av_const unsigned av_bswap16(unsigned x)
> -{
> -    unsigned y;
> -
> -    __asm__("rev16 %w0, %w1" : "=r"(y) : "r"(x));
> -    return y;
> -}
> -
> -#define av_bswap32 av_bswap32
> -static av_always_inline av_const uint32_t av_bswap32(uint32_t x)
> -{
> -    uint32_t y;
> -
> -    __asm__("rev %w0, %w1" : "=r"(y) : "r"(x));
> -    return y;
> -}
> -
> -#define av_bswap64 av_bswap64
> -static av_always_inline av_const uint64_t av_bswap64(uint64_t x)
> -{
> -    uint64_t y;
> -
> -    __asm__("rev %0, %1" : "=r"(y) : "r"(x));
> -    return y;
> -}
> -
> -#endif /* HAVE_INLINE_ASM */
> -#endif /* AVUTIL_AARCH64_BSWAP_H */
> diff --git a/libavutil/avr32/bswap.h b/libavutil/avr32/bswap.h
> deleted file mode 100644
> index e79d53f369..0000000000
> --- a/libavutil/avr32/bswap.h
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -/*
> - * This file is part of FFmpeg.
> - *
> - * FFmpeg is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2.1 of the License, or (at your option) any later version.
> - *
> - * FFmpeg is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with FFmpeg; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -
> -#ifndef AVUTIL_AVR32_BSWAP_H
> -#define AVUTIL_AVR32_BSWAP_H
> -
> -#include <stdint.h>
> -#include "config.h"
> -#include "libavutil/attributes.h"
> -
> -#if HAVE_INLINE_ASM
> -
> -#define av_bswap16 av_bswap16
> -static av_always_inline av_const uint16_t av_bswap16(uint16_t x)
> -{
> -    __asm__ ("swap.bh %0" : "+r"(x));
> -    return x;
> -}
> -
> -#define av_bswap32 av_bswap32
> -static av_always_inline av_const uint32_t av_bswap32(uint32_t x)
> -{
> -    __asm__ ("swap.b  %0" : "+r"(x));
> -    return x;
> -}
> -
> -#endif /* HAVE_INLINE_ASM */
> -
> -#endif /* AVUTIL_AVR32_BSWAP_H */
> diff --git a/libavutil/bswap.h b/libavutil/bswap.h
> index 1528906f93..b339c90b9b 100644
> --- a/libavutil/bswap.h
> +++ b/libavutil/bswap.h
> @@ -34,16 +34,10 @@
>
>  #include "config.h"
>
> -#if   ARCH_AARCH64
> -#   include "aarch64/bswap.h"
> -#elif ARCH_ARM
> +#if ARCH_ARM
>  #   include "arm/bswap.h"
> -#elif ARCH_AVR32
> -#   include "avr32/bswap.h"
>  #elif ARCH_RISCV
>  #   include "riscv/bswap.h"
> -#elif ARCH_SH4
> -#   include "sh4/bswap.h"
>  #elif ARCH_X86
>  #   include "x86/bswap.h"
>  #endif
> diff --git a/libavutil/sh4/bswap.h b/libavutil/sh4/bswap.h
> deleted file mode 100644
> index 48dd27f806..0000000000
> --- a/libavutil/sh4/bswap.h
> +++ /dev/null
> @@ -1,48 +0,0 @@
> -/*
> - * This file is part of FFmpeg.
> - *
> - * FFmpeg is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2.1 of the License, or (at your option) any later version.
> - *
> - * FFmpeg is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with FFmpeg; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -
> -/**
> - * @file
> - * byte swapping routines
> - */
> -
> -#ifndef AVUTIL_SH4_BSWAP_H
> -#define AVUTIL_SH4_BSWAP_H
> -
> -#include <stdint.h>
> -#include "config.h"
> -#include "libavutil/attributes.h"
> -
> -#define av_bswap16 av_bswap16
> -static av_always_inline av_const uint16_t av_bswap16(uint16_t x)
> -{
> -    __asm__("swap.b %0,%0" : "+r"(x));
> -    return x;
> -}
> -
> -#define av_bswap32 av_bswap32
> -static av_always_inline av_const uint32_t av_bswap32(uint32_t x)
> -{
> -    __asm__("swap.b %0,%0\n"
> -            "swap.w %0,%0\n"
> -            "swap.b %0,%0\n"
> -            : "+r"(x));
> -    return x;
> -}
> -
> -#endif /* AVUTIL_SH4_BSWAP_H */
> --
> 2.45.1
>
> _______________________________________________
> 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".

This also looks good to me.
_______________________________________________
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 2/2] sh4: remove architecture

[Sean McGovern]

On Fri, Jun 7, 2024 at 2:20 PM Rémi Denis-Courmont <remi@remlab.net> wrote:
>
> Support for SuperH was dropped over a decade ago. There no longer is any
> architecture-specific code to be found, so just remove the corresponding
> test. Technically it is still possible to compile FFmpeg as the
> "generic" (pure C) architecture.
> ---
>  configure             | 4 ----
>  libavcodec/sh4/README | 6 ------
>  2 files changed, 10 deletions(-)
>  delete mode 100644 libavcodec/sh4/README
>
> diff --git a/configure b/configure
> index 6c5b8aab9a..efead4075a 100755
> --- a/configure
> +++ b/configure
> @@ -2146,7 +2146,6 @@ ARCH_LIST="
>      ppc64
>      riscv
>      s390
> -    sh4
>      sparc
>      sparc64
>      tilegx
> @@ -5249,9 +5248,6 @@ case "$arch" in
>      s390|s390x)
>          arch="s390"
>      ;;
> -    sh4|sh)
> -        arch="sh4"
> -    ;;
>      sun4*|sparc*)
>          arch="sparc"
>      ;;
> diff --git a/libavcodec/sh4/README b/libavcodec/sh4/README
> deleted file mode 100644
> index 8dd61fe875..0000000000
> --- a/libavcodec/sh4/README
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -SH4 optimizations have been removed in
> -commit d6096a67422534918405abb46dafbbac4608cbc3
> -The last revission with the optimizations is cbfc9046e1c7e295b74f252902ae6f255eef4e78
> -
> -If you want to maintain these (or other) SH4 optimizations in ffmpeg, then please
> -contact ffmpeg-devel@ffmpeg.org
> --
> 2.45.1
>
> _______________________________________________
> 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".

Sure, OK.
_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1036 bytes --]

On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont wrote:
> C code or compiler built-ins are preferable over inline assembler for
> byte-swaps as it allows for better optimisations (e.g. instruction
> scheduling) which would otherwise be impossible.
> 
> As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
> this removes the inline assembler on GCC (and Clang) since we now
> require recent enough compiler versions (this indeed seems to work on
> AArch64).
> ---
>  libavutil/aarch64/bswap.h | 56 ---------------------------------------
>  libavutil/avr32/bswap.h   | 44 ------------------------------
>  libavutil/bswap.h         |  8 +-----
>  libavutil/sh4/bswap.h     | 48 ---------------------------------

As you are writing that this preferrable for better optimisations
Please provide benchmarks (for sh4, avr32)

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Rémi Denis-Courmont]

Le tiistaina 11. kesäkuuta 2024, 16.15.19 EEST Michael Niedermayer a écrit :
> On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont wrote:
> > C code or compiler built-ins are preferable over inline assembler for
> > byte-swaps as it allows for better optimisations (e.g. instruction
> > scheduling) which would otherwise be impossible.
> > 
> > As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
> > this removes the inline assembler on GCC (and Clang) since we now
> > require recent enough compiler versions (this indeed seems to work on
> > AArch64).
> > ---
> > 
> >  libavutil/aarch64/bswap.h | 56 ---------------------------------------
> >  libavutil/avr32/bswap.h   | 44 ------------------------------
> >  libavutil/bswap.h         |  8 +-----
> >  libavutil/sh4/bswap.h     | 48 ---------------------------------
> 
> As you are writing that this preferrable for better optimisations
> Please provide benchmarks (for sh4, avr32)

How would someone benchmark an architecture like AVR32 that is not just dead 
but barely even commercially existed at all, and for which there exist no 
known C11 compiler and thus cannot even compile FFmpeg?

That toxic attitude of yours is very demotivating, and not just to me.

-- 
Rémi Denis-Courmont
http://www.remlab.net/



_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[James Almer]

On 6/11/2024 10:15 AM, Michael Niedermayer wrote:
> On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont wrote:
>> C code or compiler built-ins are preferable over inline assembler for
>> byte-swaps as it allows for better optimisations (e.g. instruction
>> scheduling) which would otherwise be impossible.
>>
>> As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
>> this removes the inline assembler on GCC (and Clang) since we now
>> require recent enough compiler versions (this indeed seems to work on
>> AArch64).
>> ---
>>   libavutil/aarch64/bswap.h | 56 ---------------------------------------
>>   libavutil/avr32/bswap.h   | 44 ------------------------------
>>   libavutil/bswap.h         |  8 +-----
>>   libavutil/sh4/bswap.h     | 48 ---------------------------------
> 
> As you are writing that this preferrable for better optimisations
> Please provide benchmarks (for sh4, avr32)

This is a ridiculous request, considering nobody has such hardware at all.

> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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".
_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Tomas Härdin]

tis 2024-06-11 klockan 12:38 -0300 skrev James Almer:
> On 6/11/2024 10:15 AM, Michael Niedermayer wrote:
> > On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont
> > wrote:
> > > C code or compiler built-ins are preferable over inline assembler
> > > for
> > > byte-swaps as it allows for better optimisations (e.g.
> > > instruction
> > > scheduling) which would otherwise be impossible.
> > > 
> > > As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
> > > this removes the inline assembler on GCC (and Clang) since we now
> > > require recent enough compiler versions (this indeed seems to
> > > work on
> > > AArch64).
> > > ---
> > >   libavutil/aarch64/bswap.h | 56 --------------------------------
> > > -------
> > >   libavutil/avr32/bswap.h   | 44 ------------------------------
> > >   libavutil/bswap.h         |  8 +-----
> > >   libavutil/sh4/bswap.h     | 48 --------------------------------
> > > -
> > 
> > As you are writing that this preferrable for better optimisations
> > Please provide benchmarks (for sh4, avr32)
> 
> This is a ridiculous request, considering nobody has such hardware at
> all.

Maybe Måns has? He's the one who added the AVR32 code. The SH4 code was
added all the way back in 2003 in 0c6bd2ea by someone who goes by BERO.

Perhaps we should demand platforms for which we have asm also have FATE
instances?

/Tomas
_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1793 bytes --]

On Tue, Jun 11, 2024 at 12:38:37PM -0300, James Almer wrote:
> On 6/11/2024 10:15 AM, Michael Niedermayer wrote:
> > On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont wrote:
> > > C code or compiler built-ins are preferable over inline assembler for
> > > byte-swaps as it allows for better optimisations (e.g. instruction
> > > scheduling) which would otherwise be impossible.
> > > 
> > > As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
> > > this removes the inline assembler on GCC (and Clang) since we now
> > > require recent enough compiler versions (this indeed seems to work on
> > > AArch64).
> > > ---
> > >   libavutil/aarch64/bswap.h | 56 ---------------------------------------
> > >   libavutil/avr32/bswap.h   | 44 ------------------------------
> > >   libavutil/bswap.h         |  8 +-----
> > >   libavutil/sh4/bswap.h     | 48 ---------------------------------
> > 
> > As you are writing that this preferrable for better optimisations
> > Please provide benchmarks (for sh4, avr32)
> 
> This is a ridiculous request, considering nobody has such hardware at all.

Then I think its a ridiculous claim that this optimizes the code

I mean, at some point there was hardware and these optimisations did improve
speed.

This patch is not removing the code because its a rare (or dead) platform, it removes
it with the claim that this would "allows for better optimisations"
Iam sorry but i do not see why asking for the claim in the commit message
to be backed up with facts being ridiculous
The claim in the commit message may be ridiculous

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Paul B Mahol]

On Tue, Jun 11, 2024 at 5:57 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Tue, Jun 11, 2024 at 12:38:37PM -0300, James Almer wrote:
> > On 6/11/2024 10:15 AM, Michael Niedermayer wrote:
> > > On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont wrote:
> > > > C code or compiler built-ins are preferable over inline assembler for
> > > > byte-swaps as it allows for better optimisations (e.g. instruction
> > > > scheduling) which would otherwise be impossible.
> > > >
> > > > As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
> > > > this removes the inline assembler on GCC (and Clang) since we now
> > > > require recent enough compiler versions (this indeed seems to work on
> > > > AArch64).
> > > > ---
> > > >   libavutil/aarch64/bswap.h | 56
> ---------------------------------------
> > > >   libavutil/avr32/bswap.h   | 44 ------------------------------
> > > >   libavutil/bswap.h         |  8 +-----
> > > >   libavutil/sh4/bswap.h     | 48 ---------------------------------
> > >
> > > As you are writing that this preferrable for better optimisations
> > > Please provide benchmarks (for sh4, avr32)
> >
> > This is a ridiculous request, considering nobody has such hardware at
> all.
>
> Then I think its a ridiculous claim that this optimizes the code
>
> I mean, at some point there was hardware and these optimisations did
> improve
> speed.
>
> This patch is not removing the code because its a rare (or dead) platform,
> it removes
> it with the claim that this would "allows for better optimisations"
> Iam sorry but i do not see why asking for the claim in the commit message
> to be backed up with facts being ridiculous
> The claim in the commit message may be ridiculous
>

But at same time keeping sonic State of Art of audio codec compression and
maintaining it and responding to high user demand for its new features is
virtue of this project.


>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No great genius has ever existed without some touch of madness. --
> Aristotle
> _______________________________________________
> 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".
>
_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1714 bytes --]

On Tue, Jun 11, 2024 at 06:28:30PM +0300, Rémi Denis-Courmont wrote:
> Le tiistaina 11. kesäkuuta 2024, 16.15.19 EEST Michael Niedermayer a écrit :
> > On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont wrote:
> > > C code or compiler built-ins are preferable over inline assembler for
> > > byte-swaps as it allows for better optimisations (e.g. instruction
> > > scheduling) which would otherwise be impossible.
> > > 
> > > As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
> > > this removes the inline assembler on GCC (and Clang) since we now
> > > require recent enough compiler versions (this indeed seems to work on
> > > AArch64).
> > > ---
> > > 
> > >  libavutil/aarch64/bswap.h | 56 ---------------------------------------
> > >  libavutil/avr32/bswap.h   | 44 ------------------------------
> > >  libavutil/bswap.h         |  8 +-----
> > >  libavutil/sh4/bswap.h     | 48 ---------------------------------
> > 
> > As you are writing that this preferrable for better optimisations
> > Please provide benchmarks (for sh4, avr32)
> 
> How would someone benchmark an architecture like AVR32 that is not just dead 
> but barely even commercially existed at all, and for which there exist no 
> known C11 compiler and thus cannot even compile FFmpeg?

then simply remove avr32 with that explanation (no C11 compiler, and any other
reason)

but if a commit message says the code is removed because that "allows for better optimisations"
then yes i ask for benchmarks

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[James Almer]

On 6/11/2024 12:57 PM, Michael Niedermayer wrote:
> On Tue, Jun 11, 2024 at 12:38:37PM -0300, James Almer wrote:
>> On 6/11/2024 10:15 AM, Michael Niedermayer wrote:
>>> On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont wrote:
>>>> C code or compiler built-ins are preferable over inline assembler for
>>>> byte-swaps as it allows for better optimisations (e.g. instruction
>>>> scheduling) which would otherwise be impossible.
>>>>
>>>> As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
>>>> this removes the inline assembler on GCC (and Clang) since we now
>>>> require recent enough compiler versions (this indeed seems to work on
>>>> AArch64).
>>>> ---
>>>>    libavutil/aarch64/bswap.h | 56 ---------------------------------------
>>>>    libavutil/avr32/bswap.h   | 44 ------------------------------
>>>>    libavutil/bswap.h         |  8 +-----
>>>>    libavutil/sh4/bswap.h     | 48 ---------------------------------
>>>
>>> As you are writing that this preferrable for better optimisations
>>> Please provide benchmarks (for sh4, avr32)
>>
>> This is a ridiculous request, considering nobody has such hardware at all.
> 
> Then I think its a ridiculous claim that this optimizes the code
> 
> I mean, at some point there was hardware and these optimisations did improve
> speed.
> 
> This patch is not removing the code because its a rare (or dead) platform, it removes
> it with the claim that this would "allows for better optimisations"
> Iam sorry but i do not see why asking for the claim in the commit message
> to be backed up with facts being ridiculous
> The claim in the commit message may be ridiculous

Compilers have come a long way since 20 years ago when this code was added.
See https://godbolt.org/z/jPose4rj3, where new GCC generates the same 
code for sh4. And no inline assembly means instruction scheduling will 
take these functions into account.

> 
> thx
> 
> [...]
> 
> 
> _______________________________________________
> 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".
_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 465 bytes --]

On Tue, Jun 11, 2024 at 05:50:35PM +0200, Tomas Härdin wrote:
[...]
> Perhaps we should demand platforms for which we have asm also have FATE
> instances?

qemu based fate we have for sh-4:
https://fate.ffmpeg.org/?query=subarch:sh4%2F%2F

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2644 bytes --]

On Tue, Jun 11, 2024 at 01:08:04PM -0300, James Almer wrote:
> On 6/11/2024 12:57 PM, Michael Niedermayer wrote:
> > On Tue, Jun 11, 2024 at 12:38:37PM -0300, James Almer wrote:
> > > On 6/11/2024 10:15 AM, Michael Niedermayer wrote:
> > > > On Fri, Jun 07, 2024 at 09:19:46PM +0300, Rémi Denis-Courmont wrote:
> > > > > C code or compiler built-ins are preferable over inline assembler for
> > > > > byte-swaps as it allows for better optimisations (e.g. instruction
> > > > > scheduling) which would otherwise be impossible.
> > > > > 
> > > > > As with f64c2e710fa1a7b59753224e717f57c48462076f for x86 and Arm,
> > > > > this removes the inline assembler on GCC (and Clang) since we now
> > > > > require recent enough compiler versions (this indeed seems to work on
> > > > > AArch64).
> > > > > ---
> > > > >    libavutil/aarch64/bswap.h | 56 ---------------------------------------
> > > > >    libavutil/avr32/bswap.h   | 44 ------------------------------
> > > > >    libavutil/bswap.h         |  8 +-----
> > > > >    libavutil/sh4/bswap.h     | 48 ---------------------------------
> > > > 
> > > > As you are writing that this preferrable for better optimisations
> > > > Please provide benchmarks (for sh4, avr32)
> > > 
> > > This is a ridiculous request, considering nobody has such hardware at all.
> > 
> > Then I think its a ridiculous claim that this optimizes the code
> > 
> > I mean, at some point there was hardware and these optimisations did improve
> > speed.
> > 
> > This patch is not removing the code because its a rare (or dead) platform, it removes
> > it with the claim that this would "allows for better optimisations"
> > Iam sorry but i do not see why asking for the claim in the commit message
> > to be backed up with facts being ridiculous
> > The claim in the commit message may be ridiculous
> 
> Compilers have come a long way since 20 years ago when this code was added.
> See https://godbolt.org/z/jPose4rj3, where new GCC generates the same code
> for sh4. And no inline assembly means instruction scheduling will take these
> functions into account.

thanks for checking
please add a note to the commit message that this was checked for sh-4
that resolves my concern about sh-4

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Rémi Denis-Courmont]

Le tiistaina 11. kesäkuuta 2024, 19.10.04 EEST Michael Niedermayer a écrit :
> On Tue, Jun 11, 2024 at 05:50:35PM +0200, Tomas Härdin wrote:
> [...]
> 
> > Perhaps we should demand platforms for which we have asm also have FATE
> > instances?
> 
> qemu based fate we have for sh-4:
> https://fate.ffmpeg.org/?query=subarch:sh4%2F%2F

Are you seriously suggesting to use QEMU TCG for benchmarking? As someone who 
has contributed to QEMU a little, I daresay that this is insane.

-- 
レミ・デニ-クールモン
http://www.remlab.net/



_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Rémi Denis-Courmont]

Le tiistaina 11. kesäkuuta 2024, 19.04.17 EEST Michael Niedermayer a écrit :
> then simply remove avr32 with that explanation (no C11 compiler, and any
> other reason)

No. Måns and my optimisation arguments stand, even if it is purely 
hypothetical in the case of AVR32 (for which there is no working compiler). It 
is a *general* argument.

Removing the AVR32 support is not the point of *this* patch, so you are asking 
me to misrepresent what the patch does and why. As for SH4, James already 
addressed that.

> but if a commit message says the code is removed because that "allows for
> better optimisations" then yes i ask for benchmarks

"Allows for better optimisations" means exactly that: enable compilers to 
*potentially* optimise better. I never claimed that it actually improved 
performance in any given particular case.

Nevertheless it will make performance worse in one and only one case: a 
defective/half-baked compiler: missing the byte-swap instruction (if it 
exists) and/or a proper scheduling model, for the target. In other words, you 
are essentially arguing that FFmpeg should be optimised for bad C compilers 
instead of good ones.

-- 
雷米‧德尼-库尔蒙
http://www.remlab.net/



_______________________________________________
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 1/2] lavu/bswap: remove some inline assembler

[Tomas Härdin]

tis 2024-06-11 klockan 18:10 +0200 skrev Michael Niedermayer:
> On Tue, Jun 11, 2024 at 05:50:35PM +0200, Tomas Härdin wrote:
> [...]
> > Perhaps we should demand platforms for which we have asm also have
> > FATE
> > instances?
> 
> qemu based fate we have for sh-4:
> https://fate.ffmpeg.org/?query=subarch:sh4%2F%2F

I think we need actual machines, and actual users that want to run on
those machines, else we're just doing mental self-gratification

/Tomas
_______________________________________________
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 2/2] sh4: remove architecture

[Sean McGovern]

On Fri, Jun 7, 2024, 17:18 Sean McGovern <gseanmcg@gmail.com> wrote:

> On Fri, Jun 7, 2024 at 2:20 PM Rémi Denis-Courmont <remi@remlab.net>
> wrote:
> >
> > Support for SuperH was dropped over a decade ago. There no longer is any
> > architecture-specific code to be found, so just remove the corresponding
> > test. Technically it is still possible to compile FFmpeg as the
> > "generic" (pure C) architecture.
> > ---
> >  configure             | 4 ----
> >  libavcodec/sh4/README | 6 ------
> >  2 files changed, 10 deletions(-)
> >  delete mode 100644 libavcodec/sh4/README
> >
> > diff --git a/configure b/configure
> > index 6c5b8aab9a..efead4075a 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2146,7 +2146,6 @@ ARCH_LIST="
> >      ppc64
> >      riscv
> >      s390
> > -    sh4
> >      sparc
> >      sparc64
> >      tilegx
> > @@ -5249,9 +5248,6 @@ case "$arch" in
> >      s390|s390x)
> >          arch="s390"
> >      ;;
> > -    sh4|sh)
> > -        arch="sh4"
> > -    ;;
> >      sun4*|sparc*)
> >          arch="sparc"
> >      ;;
> > diff --git a/libavcodec/sh4/README b/libavcodec/sh4/README
> > deleted file mode 100644
> > index 8dd61fe875..0000000000
> > --- a/libavcodec/sh4/README
> > +++ /dev/null
> > @@ -1,6 +0,0 @@
> > -SH4 optimizations have been removed in
> > -commit d6096a67422534918405abb46dafbbac4608cbc3
> > -The last revission with the optimizations is
> cbfc9046e1c7e295b74f252902ae6f255eef4e78
> > -
> > -If you want to maintain these (or other) SH4 optimizations in ffmpeg,
> then please
> > -contact ffmpeg-devel@ffmpeg.org
> > --
> > 2.45.1
> >
> > _______________________________________________
> > 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".
>
> Sure, OK.
>

Also ping.

-- Sean McGovern

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