| Message ID | 20240611185232.5384-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avutil: rename av_mod_uintp2 to av_zero_extend | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| yinshiyou/make_loongarch64 | warning | New warnings during build |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_x86 | warning | New warnings during build |
Le tiistaina 11. kesäkuuta 2024, 21.52.30 EEST James Almer a écrit : > It's more descriptive of what it does. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavutil/common.h | 16 +++++++++++++++- > libavutil/version.h | 1 + > libavutil/x86/intmath.h | 6 +++--- > 3 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/libavutil/common.h b/libavutil/common.h > index 3e4c339893..acd041fb67 100644 > --- a/libavutil/common.h > +++ b/libavutil/common.h > @@ -42,6 +42,7 @@ > #include "attributes.h" > #include "error.h" > #include "macros.h" > +#include "version.h" > > #ifdef HAVE_AV_CONFIG_H > # include "config.h" > @@ -122,9 +123,11 @@ > #ifndef av_clip_uintp2 > # define av_clip_uintp2 av_clip_uintp2_c > #endif > +#if FF_API_MOD_UINTP2 > #ifndef av_mod_uintp2 > # define av_mod_uintp2 av_mod_uintp2_c > #endif > +#endif > #ifndef av_sat_add32 > # define av_sat_add32 av_sat_add32_c > #endif > @@ -149,6 +152,9 @@ > #ifndef av_clipd > # define av_clipd av_clipd_c > #endif > +#ifndef av_zero_extend > +# define av_zero_extend av_zero_extend_c > +#endif > #ifndef av_popcount > # define av_popcount av_popcount_c > #endif What's the breakdown between common and intmath and where should new functions be defined?
On 6/11/2024 4:13 PM, Rémi Denis-Courmont wrote: > Le tiistaina 11. kesäkuuta 2024, 21.52.30 EEST James Almer a écrit : >> It's more descriptive of what it does. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavutil/common.h | 16 +++++++++++++++- >> libavutil/version.h | 1 + >> libavutil/x86/intmath.h | 6 +++--- >> 3 files changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/libavutil/common.h b/libavutil/common.h >> index 3e4c339893..acd041fb67 100644 >> --- a/libavutil/common.h >> +++ b/libavutil/common.h >> @@ -42,6 +42,7 @@ >> #include "attributes.h" >> #include "error.h" >> #include "macros.h" >> +#include "version.h" >> >> #ifdef HAVE_AV_CONFIG_H >> # include "config.h" >> @@ -122,9 +123,11 @@ >> #ifndef av_clip_uintp2 >> # define av_clip_uintp2 av_clip_uintp2_c >> #endif >> +#if FF_API_MOD_UINTP2 >> #ifndef av_mod_uintp2 >> # define av_mod_uintp2 av_mod_uintp2_c >> #endif >> +#endif >> #ifndef av_sat_add32 >> # define av_sat_add32 av_sat_add32_c >> #endif >> @@ -149,6 +152,9 @@ >> #ifndef av_clipd >> # define av_clipd av_clipd_c >> #endif >> +#ifndef av_zero_extend >> +# define av_zero_extend av_zero_extend_c >> +#endif >> #ifndef av_popcount >> # define av_popcount av_popcount_c >> #endif > > What's the breakdown between common and intmath and where should new functions > be defined? I don't know, but this being a clip function of sorts, x86/intmath.h for optimized forms is fine. There doesn't seem to be another header where this could fit.
Rémi Denis-Courmont: > Le tiistaina 11. kesäkuuta 2024, 21.52.30 EEST James Almer a écrit : >> It's more descriptive of what it does. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavutil/common.h | 16 +++++++++++++++- >> libavutil/version.h | 1 + >> libavutil/x86/intmath.h | 6 +++--- >> 3 files changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/libavutil/common.h b/libavutil/common.h >> index 3e4c339893..acd041fb67 100644 >> --- a/libavutil/common.h >> +++ b/libavutil/common.h >> @@ -42,6 +42,7 @@ >> #include "attributes.h" >> #include "error.h" >> #include "macros.h" >> +#include "version.h" >> >> #ifdef HAVE_AV_CONFIG_H >> # include "config.h" >> @@ -122,9 +123,11 @@ >> #ifndef av_clip_uintp2 >> # define av_clip_uintp2 av_clip_uintp2_c >> #endif >> +#if FF_API_MOD_UINTP2 >> #ifndef av_mod_uintp2 >> # define av_mod_uintp2 av_mod_uintp2_c >> #endif >> +#endif >> #ifndef av_sat_add32 >> # define av_sat_add32 av_sat_add32_c >> #endif >> @@ -149,6 +152,9 @@ >> #ifndef av_clipd >> # define av_clipd av_clipd_c >> #endif >> +#ifndef av_zero_extend >> +# define av_zero_extend av_zero_extend_c >> +#endif >> #ifndef av_popcount >> # define av_popcount av_popcount_c >> #endif > > What's the breakdown between common and intmath and where should new functions > be defined? > intmath.h is private, common.h is public. That's the difference. Most of these misc-math functions are integer only (IIRC with the exception of floating point clip functions). We should probably move these functions to a new header intmath.h and make it public (the old intmath.h would be renamed to intmath_internal.h or so). - Andreas
James Almer: > It's more descriptive of what it does. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavutil/common.h | 16 +++++++++++++++- > libavutil/version.h | 1 + > libavutil/x86/intmath.h | 6 +++--- > 3 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/libavutil/common.h b/libavutil/common.h > index 3e4c339893..acd041fb67 100644 > --- a/libavutil/common.h > +++ b/libavutil/common.h > @@ -42,6 +42,7 @@ > #include "attributes.h" > #include "error.h" > #include "macros.h" > +#include "version.h" > > #ifdef HAVE_AV_CONFIG_H > # include "config.h" > @@ -122,9 +123,11 @@ > #ifndef av_clip_uintp2 > # define av_clip_uintp2 av_clip_uintp2_c > #endif > +#if FF_API_MOD_UINTP2 > #ifndef av_mod_uintp2 > # define av_mod_uintp2 av_mod_uintp2_c > #endif > +#endif For the record: This patch on its own will disable the x86 optimization in this patch, only to be restored in #3. > #ifndef av_sat_add32 > # define av_sat_add32 av_sat_add32_c > #endif > @@ -149,6 +152,9 @@ > #ifndef av_clipd > # define av_clipd av_clipd_c > #endif > +#ifndef av_zero_extend > +# define av_zero_extend av_zero_extend_c > +#endif > #ifndef av_popcount > # define av_popcount av_popcount_c > #endif > @@ -288,11 +294,19 @@ static av_always_inline av_const unsigned av_clip_uintp2_c(int a, int p) > * @param p bit position to clip at > * @return clipped value > */ > -static av_always_inline av_const unsigned av_mod_uintp2_c(unsigned a, unsigned p) > +static av_always_inline av_const unsigned av_zero_extend_c(unsigned a, unsigned p) > { > return a & ((1U << p) - 1); > } > > +#if FF_API_MOD_UINTP2 > +attribute_deprecated > +static av_always_inline av_const unsigned av_mod_uintp2_c(unsigned a, unsigned p) > +{ > + return av_zero_extend_c(a, p); > +} > +#endif > + > /** > * Add two signed 32-bit values with saturation. > * > diff --git a/libavutil/version.h b/libavutil/version.h > index 9d08d56884..38456affb8 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -112,6 +112,7 @@ > #define FF_API_PALETTE_HAS_CHANGED (LIBAVUTIL_VERSION_MAJOR < 60) > #define FF_API_VULKAN_CONTIGUOUS_MEMORY (LIBAVUTIL_VERSION_MAJOR < 60) > #define FF_API_H274_FILM_GRAIN_VCS (LIBAVUTIL_VERSION_MAJOR < 60) > +#define FF_API_MOD_UINTP2 (LIBAVUTIL_VERSION_MAJOR < 60) > > /** > * @} > diff --git a/libavutil/x86/intmath.h b/libavutil/x86/intmath.h > index 8a6b5ae261..821a06ab66 100644 > --- a/libavutil/x86/intmath.h > +++ b/libavutil/x86/intmath.h > @@ -82,13 +82,13 @@ static av_always_inline av_const int ff_ctzll_x86(long long v) > #if defined(__BMI2__) > > #if AV_GCC_VERSION_AT_LEAST(5,1) > -#define av_mod_uintp2 __builtin_ia32_bzhi_si > +#define av_zero_extend __builtin_ia32_bzhi_si > #elif HAVE_INLINE_ASM > /* GCC releases before 5.1.0 have a broken bzhi builtin, so for those we > * implement it using inline assembly > */ > -#define av_mod_uintp2 av_mod_uintp2_bmi2 > -static av_always_inline av_const unsigned av_mod_uintp2_bmi2(unsigned a, unsigned p) > +#define av_zero_extend av_zero_extend_bmi2 > +static av_always_inline av_const unsigned av_zero_extend_bmi2(unsigned a, unsigned p) > { > if (av_builtin_constant_p(p)) > return a & ((1 << p) - 1); Missing APIchanges and version bump. - Andreas
On 6/11/2024 5:26 PM, Andreas Rheinhardt wrote: > James Almer: >> It's more descriptive of what it does. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavutil/common.h | 16 +++++++++++++++- >> libavutil/version.h | 1 + >> libavutil/x86/intmath.h | 6 +++--- >> 3 files changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/libavutil/common.h b/libavutil/common.h >> index 3e4c339893..acd041fb67 100644 >> --- a/libavutil/common.h >> +++ b/libavutil/common.h >> @@ -42,6 +42,7 @@ >> #include "attributes.h" >> #include "error.h" >> #include "macros.h" >> +#include "version.h" >> >> #ifdef HAVE_AV_CONFIG_H >> # include "config.h" >> @@ -122,9 +123,11 @@ >> #ifndef av_clip_uintp2 >> # define av_clip_uintp2 av_clip_uintp2_c >> #endif >> +#if FF_API_MOD_UINTP2 >> #ifndef av_mod_uintp2 >> # define av_mod_uintp2 av_mod_uintp2_c >> #endif >> +#endif > > For the record: This patch on its own will disable the x86 optimization > in this patch, only to be restored in #3. > >> #ifndef av_sat_add32 >> # define av_sat_add32 av_sat_add32_c >> #endif >> @@ -149,6 +152,9 @@ >> #ifndef av_clipd >> # define av_clipd av_clipd_c >> #endif >> +#ifndef av_zero_extend >> +# define av_zero_extend av_zero_extend_c >> +#endif >> #ifndef av_popcount >> # define av_popcount av_popcount_c >> #endif >> @@ -288,11 +294,19 @@ static av_always_inline av_const unsigned av_clip_uintp2_c(int a, int p) >> * @param p bit position to clip at >> * @return clipped value >> */ >> -static av_always_inline av_const unsigned av_mod_uintp2_c(unsigned a, unsigned p) >> +static av_always_inline av_const unsigned av_zero_extend_c(unsigned a, unsigned p) >> { >> return a & ((1U << p) - 1); >> } >> >> +#if FF_API_MOD_UINTP2 >> +attribute_deprecated >> +static av_always_inline av_const unsigned av_mod_uintp2_c(unsigned a, unsigned p) >> +{ >> + return av_zero_extend_c(a, p); >> +} >> +#endif >> + >> /** >> * Add two signed 32-bit values with saturation. >> * >> diff --git a/libavutil/version.h b/libavutil/version.h >> index 9d08d56884..38456affb8 100644 >> --- a/libavutil/version.h >> +++ b/libavutil/version.h >> @@ -112,6 +112,7 @@ >> #define FF_API_PALETTE_HAS_CHANGED (LIBAVUTIL_VERSION_MAJOR < 60) >> #define FF_API_VULKAN_CONTIGUOUS_MEMORY (LIBAVUTIL_VERSION_MAJOR < 60) >> #define FF_API_H274_FILM_GRAIN_VCS (LIBAVUTIL_VERSION_MAJOR < 60) >> +#define FF_API_MOD_UINTP2 (LIBAVUTIL_VERSION_MAJOR < 60) >> >> /** >> * @} >> diff --git a/libavutil/x86/intmath.h b/libavutil/x86/intmath.h >> index 8a6b5ae261..821a06ab66 100644 >> --- a/libavutil/x86/intmath.h >> +++ b/libavutil/x86/intmath.h >> @@ -82,13 +82,13 @@ static av_always_inline av_const int ff_ctzll_x86(long long v) >> #if defined(__BMI2__) >> >> #if AV_GCC_VERSION_AT_LEAST(5,1) >> -#define av_mod_uintp2 __builtin_ia32_bzhi_si >> +#define av_zero_extend __builtin_ia32_bzhi_si >> #elif HAVE_INLINE_ASM >> /* GCC releases before 5.1.0 have a broken bzhi builtin, so for those we >> * implement it using inline assembly >> */ >> -#define av_mod_uintp2 av_mod_uintp2_bmi2 >> -static av_always_inline av_const unsigned av_mod_uintp2_bmi2(unsigned a, unsigned p) >> +#define av_zero_extend av_zero_extend_bmi2 >> +static av_always_inline av_const unsigned av_zero_extend_bmi2(unsigned a, unsigned p) >> { >> if (av_builtin_constant_p(p)) >> return a & ((1 << p) - 1); > > Missing APIchanges and version bump. Will add both and push the set soon.
diff --git a/libavutil/common.h b/libavutil/common.h index 3e4c339893..acd041fb67 100644 --- a/libavutil/common.h +++ b/libavutil/common.h @@ -42,6 +42,7 @@ #include "attributes.h" #include "error.h" #include "macros.h" +#include "version.h" #ifdef HAVE_AV_CONFIG_H # include "config.h" @@ -122,9 +123,11 @@ #ifndef av_clip_uintp2 # define av_clip_uintp2 av_clip_uintp2_c #endif +#if FF_API_MOD_UINTP2 #ifndef av_mod_uintp2 # define av_mod_uintp2 av_mod_uintp2_c #endif +#endif #ifndef av_sat_add32 # define av_sat_add32 av_sat_add32_c #endif @@ -149,6 +152,9 @@ #ifndef av_clipd # define av_clipd av_clipd_c #endif +#ifndef av_zero_extend +# define av_zero_extend av_zero_extend_c +#endif #ifndef av_popcount # define av_popcount av_popcount_c #endif @@ -288,11 +294,19 @@ static av_always_inline av_const unsigned av_clip_uintp2_c(int a, int p) * @param p bit position to clip at * @return clipped value */ -static av_always_inline av_const unsigned av_mod_uintp2_c(unsigned a, unsigned p) +static av_always_inline av_const unsigned av_zero_extend_c(unsigned a, unsigned p) { return a & ((1U << p) - 1); } +#if FF_API_MOD_UINTP2 +attribute_deprecated +static av_always_inline av_const unsigned av_mod_uintp2_c(unsigned a, unsigned p) +{ + return av_zero_extend_c(a, p); +} +#endif + /** * Add two signed 32-bit values with saturation. * diff --git a/libavutil/version.h b/libavutil/version.h index 9d08d56884..38456affb8 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -112,6 +112,7 @@ #define FF_API_PALETTE_HAS_CHANGED (LIBAVUTIL_VERSION_MAJOR < 60) #define FF_API_VULKAN_CONTIGUOUS_MEMORY (LIBAVUTIL_VERSION_MAJOR < 60) #define FF_API_H274_FILM_GRAIN_VCS (LIBAVUTIL_VERSION_MAJOR < 60) +#define FF_API_MOD_UINTP2 (LIBAVUTIL_VERSION_MAJOR < 60) /** * @} diff --git a/libavutil/x86/intmath.h b/libavutil/x86/intmath.h index 8a6b5ae261..821a06ab66 100644 --- a/libavutil/x86/intmath.h +++ b/libavutil/x86/intmath.h @@ -82,13 +82,13 @@ static av_always_inline av_const int ff_ctzll_x86(long long v) #if defined(__BMI2__) #if AV_GCC_VERSION_AT_LEAST(5,1) -#define av_mod_uintp2 __builtin_ia32_bzhi_si +#define av_zero_extend __builtin_ia32_bzhi_si #elif HAVE_INLINE_ASM /* GCC releases before 5.1.0 have a broken bzhi builtin, so for those we * implement it using inline assembly */ -#define av_mod_uintp2 av_mod_uintp2_bmi2 -static av_always_inline av_const unsigned av_mod_uintp2_bmi2(unsigned a, unsigned p) +#define av_zero_extend av_zero_extend_bmi2 +static av_always_inline av_const unsigned av_zero_extend_bmi2(unsigned a, unsigned p) { if (av_builtin_constant_p(p)) return a & ((1 << p) - 1);
It's more descriptive of what it does. Signed-off-by: James Almer <jamrial@gmail.com> --- libavutil/common.h | 16 +++++++++++++++- libavutil/version.h | 1 + libavutil/x86/intmath.h | 6 +++--- 3 files changed, 19 insertions(+), 4 deletions(-)