* [FFmpeg-devel] [PATCH] lavu/common: use fabs(f) for FFABS/FFNABS
@ 2024-07-26 16:32 Rémi Denis-Courmont
2024-08-14 13:10 ` Anton Khirnov
0 siblings, 1 reply; 2+ messages in thread
From: Rémi Denis-Courmont @ 2024-07-26 16:32 UTC (permalink / raw)
To: ffmpeg-devel
The absolute value of a floating point value is easier to calculate than
that of an integer: it is obtained by either clearing the sign or xoring it
with itself. Accordingly, Arm, LoongArch, RISC-V have dedicated instructions
to manipulate float sign bits. x86-64 has ANDPS.
As per C11 appendix F, FFABS() is not quite the same as fabs() however,
due to differing NaN handling. GCC is able to optimise the current FFABS
definition to match fabs() anyway with just -fno-signed-zeros. But Clang is
evidently not doing so and inserts a comparison with 0 and a conditional
select or branch. Now that we have C11, this is easy to fix properly.
(Another option is to manually audit which of the ~880 FFABS/FFNABS use
sites involve floating point values.)
Note that this still preserves the old definitions for C++ and pre-C11
compilers since the 2 macros are public (even though they start with FF).
---
libavutil/common.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/libavutil/common.h b/libavutil/common.h
index 3b830daf30..48761885fb 100644
--- a/libavutil/common.h
+++ b/libavutil/common.h
@@ -71,7 +71,14 @@
* as with *abs()
* @see FFNABS()
*/
+#if !defined (__cplusplus) && __STDC_VERSION__ >= 201112L
+#define FFABS(a) _Generic(a, \
+ float: fabsf(a), \
+ double: fabs(a), \
+ default: ((a) >= 0) ? (a) : (-(a)))
+#else
#define FFABS(a) ((a) >= 0 ? (a) : (-(a)))
+#endif
#define FFSIGN(a) ((a) > 0 ? 1 : -1)
/**
@@ -80,7 +87,14 @@
* As with many macros, this evaluates its argument twice, it thus must not have
* a sideeffect, that is FFNABS(x++) has undefined behavior.
*/
+#if !defined (__cplusplus) && __STDC_VERSION__ >= 201112L
+#define FFNABS(a) _Generic(a, \
+ float: -fabsf(a), \
+ double: -fabs(a), \
+ default: ((a) <= 0) ? (a) : (-(a)))
+#else
#define FFNABS(a) ((a) <= 0 ? (a) : (-(a)))
+#endif
/**
* Unsigned Absolute value.
--
2.45.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [FFmpeg-devel] [PATCH] lavu/common: use fabs(f) for FFABS/FFNABS
2024-07-26 16:32 [FFmpeg-devel] [PATCH] lavu/common: use fabs(f) for FFABS/FFNABS Rémi Denis-Courmont
@ 2024-08-14 13:10 ` Anton Khirnov
0 siblings, 0 replies; 2+ messages in thread
From: Anton Khirnov @ 2024-08-14 13:10 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Rémi Denis-Courmont (2024-07-26 18:32:19)
> The absolute value of a floating point value is easier to calculate than
> that of an integer: it is obtained by either clearing the sign or xoring it
> with itself. Accordingly, Arm, LoongArch, RISC-V have dedicated instructions
> to manipulate float sign bits. x86-64 has ANDPS.
>
> As per C11 appendix F, FFABS() is not quite the same as fabs() however,
> due to differing NaN handling. GCC is able to optimise the current FFABS
> definition to match fabs() anyway with just -fno-signed-zeros. But Clang is
> evidently not doing so and inserts a comparison with 0 and a conditional
> select or branch. Now that we have C11, this is easy to fix properly.
> (Another option is to manually audit which of the ~880 FFABS/FFNABS use
> sites involve floating point values.)
>
> Note that this still preserves the old definitions for C++ and pre-C11
> compilers since the 2 macros are public (even though they start with FF).
> ---
> libavutil/common.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/libavutil/common.h b/libavutil/common.h
> index 3b830daf30..48761885fb 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -71,7 +71,14 @@
> * as with *abs()
> * @see FFNABS()
> */
> +#if !defined (__cplusplus) && __STDC_VERSION__ >= 201112L
Is __STDC_VERSION__ defined for C++?
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-08-14 13:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-26 16:32 [FFmpeg-devel] [PATCH] lavu/common: use fabs(f) for FFABS/FFNABS Rémi Denis-Courmont
2024-08-14 13:10 ` Anton Khirnov
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git