Re: [FFmpeg-devel] [PATCH 1/2] avutil/intmath: use AV_HAS_BUILTIN to detect builtin availability

From: Kacper Michajlow <kasper93-at-gmail.com@ffmpeg.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/2] avutil/intmath: use AV_HAS_BUILTIN to detect builtin availability
Date: Wed, 11 Jun 2025 20:57:26 +0200
Message-ID: <CABPLASS7uQc=AaCRhheFV-8rw_YXZtxMvGVuoQxd+Fea-v8=yw@mail.gmail.com> (raw)
In-Reply-To: <CABPLASS-7iayhpGW63+5Mm6a=wbZ_nzeq1DxfdScxAJ33nAC5g@mail.gmail.com>

On Wed, 11 Jun 2025 at 20:51, Kacper Michajlow <kasper93@gmail.com> wrote:
>
> On Wed, 11 Jun 2025 at 20:45, Martin Storsjö <martin@martin.st> wrote:
> >
> > On Wed, 11 Jun 2025, Kacper Michajlow wrote:
> >
> > > On Mon, 31 Mar 2025 at 14:03, Kacper Michajłow <kasper93@gmail.com> wrote:
> > >>
> > >> Fixes use of bultins on clang x86_64-pc-windows-msvc which does not
> > >> define any __GNUC__. Also on other targets __GNUC__ is defined to 4 by
> > >> default, so any feature testing based on version is not really valid.
> > >>
> > >> Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
> > >> ---
> > >>  libavutil/intmath.h     | 12 +++++-------
> > >>  libavutil/x86/intmath.h |  4 ++--
> > >>  2 files changed, 7 insertions(+), 9 deletions(-)
> > >>
> > >> diff --git a/libavutil/intmath.h b/libavutil/intmath.h
> > >> index 52e11a8d5f..b177a848c6 100644
> > >> --- a/libavutil/intmath.h
> > >> +++ b/libavutil/intmath.h
> > >> @@ -35,7 +35,7 @@
> > >>  #endif
> > >>
> > >>  #if HAVE_FAST_CLZ
> > >> -#if AV_GCC_VERSION_AT_LEAST(3,4)
> > >> +#if AV_GCC_VERSION_AT_LEAST(3,4) || AV_HAS_BUILTIN(__builtin_clz)
> > >>  #ifndef ff_log2
> > >>  #   define ff_log2(x) (31 - __builtin_clz((x)|1))
> > >>  #   ifndef ff_log2_16bit
> > >> @@ -90,18 +90,16 @@ static av_always_inline av_const int ff_log2_16bit_c(unsigned int v)
> > >>   */
> > >>
> > >>  #if HAVE_FAST_CLZ
> > >> -#if AV_GCC_VERSION_AT_LEAST(3,4)
> > >> -#ifndef ff_ctz
> > >> +#if !defined(ff_ctz) && (AV_GCC_VERSION_AT_LEAST(3,4) || AV_HAS_BUILTIN(__builtin_ctz))
> > >>  #define ff_ctz(v) __builtin_ctz(v)
> > >>  #endif
> > >> -#ifndef ff_ctzll
> > >> +#if !defined(ff_ctzll) && (AV_GCC_VERSION_AT_LEAST(3,4) || AV_HAS_BUILTIN(__builtin_ctzll))
> > >>  #define ff_ctzll(v) __builtin_ctzll(v)
> > >>  #endif
> > >> -#ifndef ff_clz
> > >> +#if !defined(ff_clz) && (AV_GCC_VERSION_AT_LEAST(3,4) || AV_HAS_BUILTIN(__builtin_clz))
> > >>  #define ff_clz(v) __builtin_clz(v)
> > >>  #endif
> > >>  #endif
> > >> -#endif
> > >>
> > >>  #ifndef ff_ctz
> > >>  #define ff_ctz ff_ctz_c
> > >> @@ -154,7 +152,7 @@ static av_always_inline av_const unsigned ff_clz_c(unsigned x)
> > >>  }
> > >>  #endif
> > >>
> > >> -#if AV_GCC_VERSION_AT_LEAST(3,4)
> > >> +#if AV_GCC_VERSION_AT_LEAST(3,4) || AV_HAS_BUILTIN(__builtin_parity)
> > >>  #ifndef av_parity
> > >>  #define av_parity __builtin_parity
> > >>  #endif
> > >> diff --git a/libavutil/x86/intmath.h b/libavutil/x86/intmath.h
> > >> index 4893a1f1b4..030ded6b6f 100644
> > >> --- a/libavutil/x86/intmath.h
> > >> +++ b/libavutil/x86/intmath.h
> > >> @@ -66,7 +66,7 @@ static av_always_inline av_const int ff_ctzll_x86(long long v)
> > >>
> > >>  #endif /* HAVE_FAST_CLZ */
> > >>
> > >> -#if defined(__GNUC__)
> > >> +#if defined(__GNUC__) || defined(__clang__)
> > >>
> > >>  /* Our generic version of av_popcount is faster than GCC's built-in on
> > >>   * CPUs that don't support the popcnt instruction.
> > >> @@ -81,7 +81,7 @@ static av_always_inline av_const int ff_ctzll_x86(long long v)
> > >>
> > >>  #if defined(__BMI2__)
> > >>
> > >> -#if AV_GCC_VERSION_AT_LEAST(5,1)
> > >> +#if AV_GCC_VERSION_AT_LEAST(5,1) || AV_HAS_BUILTIN(__builtin_ia32_bzhi_si)
> > >>  #if defined(ASSERT_LEVEL) && ASSERT_LEVEL >= 2
> > >>  #define av_zero_extend av_zero_extend_bmi2
> > >>  static av_always_inline av_const unsigned av_zero_extend_bmi2(unsigned a, unsigned p)
> > >> --
> > >> 2.45.1
> > >>
> > >
> > > bump
> >
> > Can you specify the symptoms that are fixed? Is there a specific build
> > configuration that fails, or is it a case of using more optimal builtins
> > where suitable? (Same goes for 2/2 probably.)
>
> As the commit message says it "Fixes use of bultins on clang
> x86_64-pc-windows-msvc". It now uses builtins where previously it
> would fallback to C implementation.
>
> - Kacper

Additionally for __builtin_ia32_bzhi_si check, currently it uses
AV_GCC_VERSION_AT_LEAST(5,1), which wouldn't work on any platform,
because clang always defines __GNUC__ to 4 for compatibility reasons
only.

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