[FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.

[Michael Niedermayer]

On Thu, May 21, 2020 at 12:05:47PM -0700, Dale Curtis wrote:
> On Thu, May 21, 2020 at 12:37 AM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
> > used with ccache on a x86-64
> >
> 
> Huh, I guess there's no early abort for conditionals in a preprocessor
> statement with __has_builtin for some reason. I've added a AV_HAS_BUILTIN
> macro to workaround this.
> 
> - dale

>  attributes.h |    6 ++++++
>  common.h     |   10 ++++++++++
>  2 files changed, 16 insertions(+)
> 0659f15ed4807d2db1fb07a77e74d33dc5ffb90a  sat_math_builtin_v7.patch
> From 34b1e4e398d3f5af72adf6d7584008098614ea2c Mon Sep 17 00:00:00 2001
> From: Dale Curtis <dalecurtis at chromium.org>
> Date: Fri, 1 May 2020 10:20:43 -0700
> Subject: [PATCH] Use gcc/clang builtins for av_sat_(add|sub)_64_c if
>  available.
> 
> Signed-off-by: Dale Curtis <dalecurtis at chromium.org>
> ---
>  libavutil/attributes.h |  6 ++++++
>  libavutil/common.h     | 10 ++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/libavutil/attributes.h b/libavutil/attributes.h
> index ced108aa2c..ab2a1fdd0e 100644
> --- a/libavutil/attributes.h
> +++ b/libavutil/attributes.h
> @@ -34,6 +34,12 @@
>  #    define AV_GCC_VERSION_AT_MOST(x,y)  0
>  #endif
>  
> +#ifdef __has_builtin
> +#    define AV_HAS_BUILTIN(x) __has_builtin(x)
> +#else
> +#    define AV_HAS_BUILTIN(x) false
> +#endif
> +
>  #ifndef av_always_inline
>  #if AV_GCC_VERSION_AT_LEAST(3,1)
>  #    define av_always_inline __attribute__((always_inline)) inline
> diff --git a/libavutil/common.h b/libavutil/common.h
> index 7a774fc448..2777cea9f9 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -299,11 +299,16 @@ static av_always_inline int av_sat_dsub32_c(int a, int b)
>   * @return sum with signed saturation
>   */
>  static av_always_inline int64_t av_sat_add64_c(int64_t a, int64_t b) {
> +#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) || AV_HAS_BUILTIN(__builtin_add_overflow)
> +    int64_t tmp;
> +    return !__builtin_add_overflow(a, b, &tmp) ? tmp : (tmp < 0 ? INT64_MAX : INT64_MIN);
> +#else
>      if (b >= 0 && a >= INT64_MAX - b)
>          return INT64_MAX;
>      if (b <= 0 && a <= INT64_MIN - b)
>          return INT64_MIN;
>      return a + b;
> +#endif
>  }
>  
>  /**
> @@ -314,11 +319,16 @@ static av_always_inline int64_t av_sat_add64_c(int64_t a, int64_t b) {
>   * @return difference with signed saturation
>   */
>  static av_always_inline int64_t av_sat_sub64_c(int64_t a, int64_t b) {
> +#if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) || AV_HAS_BUILTIN(__builtin_sub_overflow)
> +    int64_t tmp;
> +    return !__builtin_sub_overflow(a, b, &tmp) ? tmp : (tmp < 0 ? INT64_MAX : INT64_MIN);
> +#else
>      if (b <= 0 && a >= INT64_MAX + b)
>          return INT64_MAX;
>      if (b >= 0 && a <= INT64_MIN + b)
>          return INT64_MIN;
>      return a - b;
> +#endif

does this produce faster / better code ?
that is do compilers actually fail to optimize this from plain
clean C code ?

thanks

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200522/a08dec6d/attachment.sig>