Hello again. Is there anything else I can do here? On Fri, Apr 4, 2025 at 11:40 AM Pranav Kant <p...@google.com> wrote:
> Any thoughts on this? > > On Thu, Mar 20, 2025 at 5:30 PM Pranav Kant <p...@google.com> wrote: > >> Patch version v5: >> - Uses two new macros DECLARE_ASM_VAR (used for both external and inline >> asm) and DECLARE_EXTERNAL_ASM_VAR (used only for external asm) >> - I intend to remove explicit existing use of attribute_visibility_hidden >> in follow-up patch and instead use DECLARE_EXTERNAL_ASM_VAR for those >> variables >> - Other variables will be marked with these two new macros in a follow-up >> patch. I want to settle down on the infrastructure first with a handful of >> variables. >> >> Let me know if you have any questions. >> >> On Thu, Mar 20, 2025 at 5:28 PM Pranav Kant <p...@google.com> wrote: >> >>> By default, all globals in C/C++ compiled by clang are allocated >>> in non-large data sections. See [1] for background on code models. >>> For PIC (Position independent code), this is fine as long as binary is >>> small but as binary size increases, users maybe want to use medium/large >>> code models (-mcmodel=medium) which moves data in to large sections. >>> As data in these large sections cannot be accessed using PIC code >>> anymore (as it may be too far away), compiler ends up using a different >>> instruction sequence when building C/C++ code -- using GOT to access >>> these globals (which can be relaxed by linker at link time if binary >>> ends up being smaller). However, external assembly and inline assembly >>> continue to access these globals using older PC-relative addressing >>> which may not work because globals may be placed too far away. >>> >>> Introduce new macros for such variables that mark them with small code >>> model attribute. This ensures that these variables are never allocated >>> in large data sections, and continue to be validly accessed from assembly >>> code. >>> >>> This patch should not have any affect on builds that use small code >>> model, which is the default mode. >>> >>> [1] >>> https://eli.thegreenplace.net/2012/01/03/understanding-the-x64-code-models >>> >>> Signed-off-by: Pranav Kant <p...@google.com> >>> --- >>> libavcodec/ac3dsp.h | 4 +++- >>> libavcodec/cabac.h | 4 +++- >>> libavcodec/x86/constants.h | 12 +++++++----- >>> libavutil/attributes.h | 6 ++++++ >>> libavutil/attributes_internal.h | 16 ++++++++++++++++ >>> libavutil/mem_internal.h | 13 +++++++++++++ >>> 6 files changed, 48 insertions(+), 7 deletions(-) >>> >>> diff --git a/libavcodec/ac3dsp.h b/libavcodec/ac3dsp.h >>> index b1b2bced8f..a3c55a833b 100644 >>> --- a/libavcodec/ac3dsp.h >>> +++ b/libavcodec/ac3dsp.h >>> @@ -25,11 +25,13 @@ >>> #include <stddef.h> >>> #include <stdint.h> >>> >>> +#include "libavutil/mem_internal.h" >>> + >>> /** >>> * Number of mantissa bits written for each bap value. >>> * bap values with fractional bits are set to 0 and are calculated >>> separately. >>> */ >>> -extern const uint16_t ff_ac3_bap_bits[16]; >>> +extern DECLARE_EXTERNAL_ASM_VAR(16, const uint16_t, >>> ff_ac3_bap_bits)[16]; >>> >>> typedef struct AC3DSPContext { >>> /** >>> diff --git a/libavcodec/cabac.h b/libavcodec/cabac.h >>> index 38d06b2842..df352258c6 100644 >>> --- a/libavcodec/cabac.h >>> +++ b/libavcodec/cabac.h >>> @@ -29,7 +29,9 @@ >>> >>> #include <stdint.h> >>> >>> -extern const uint8_t ff_h264_cabac_tables[512 + 4*2*64 + 4*64 + 63]; >>> +#include "libavutil/mem_internal.h" >>> + >>> +extern DECLARE_ASM_VAR(1, const uint8_t, ff_h264_cabac_tables)[512 + >>> 4*2*64 + 4*64 + 63]; >>> #define H264_NORM_SHIFT_OFFSET 0 >>> #define H264_LPS_RANGE_OFFSET 512 >>> #define H264_MLPS_STATE_OFFSET 1024 >>> diff --git a/libavcodec/x86/constants.h b/libavcodec/x86/constants.h >>> index 0c6bf41fa0..2561302604 100644 >>> --- a/libavcodec/x86/constants.h >>> +++ b/libavcodec/x86/constants.h >>> @@ -23,13 +23,14 @@ >>> >>> #include <stdint.h> >>> >>> +#include "libavutil/mem_internal.h" >>> #include "libavutil/x86/asm.h" >>> >>> -extern const ymm_reg ff_pw_1; >>> +extern DECLARE_EXTERNAL_ASM_VAR(32, const ymm_reg, ff_pw_1); >>> extern const ymm_reg ff_pw_2; >>> extern const xmm_reg ff_pw_3; >>> -extern const ymm_reg ff_pw_4; >>> -extern const xmm_reg ff_pw_5; >>> +extern DECLARE_ASM_VAR(32, const ymm_reg, ff_pw_4); >>> +extern DECLARE_ASM_VAR(16, const xmm_reg, ff_pw_5); >>> extern const xmm_reg ff_pw_8; >>> extern const xmm_reg ff_pw_9; >>> extern const uint64_t ff_pw_15; >>> @@ -43,7 +44,7 @@ extern const uint64_t ff_pw_128; >>> extern const ymm_reg ff_pw_255; >>> extern const ymm_reg ff_pw_256; >>> extern const ymm_reg ff_pw_512; >>> -extern const ymm_reg ff_pw_1023; >>> +extern DECLARE_EXTERNAL_ASM_VAR(32, const ymm_reg, ff_pw_1023); >>> extern const ymm_reg ff_pw_1024; >>> extern const ymm_reg ff_pw_2048; >>> extern const ymm_reg ff_pw_4095; >>> @@ -52,9 +53,10 @@ extern const ymm_reg ff_pw_8192; >>> extern const ymm_reg ff_pw_m1; >>> >>> extern const ymm_reg ff_pb_0; >>> -extern const ymm_reg ff_pb_1; >>> +extern DECLARE_EXTERNAL_ASM_VAR(32, const ymm_reg, ff_pb_1); >>> extern const ymm_reg ff_pb_2; >>> extern const ymm_reg ff_pb_3; >>> +extern DECLARE_ASM_VAR(32, const xmm_reg, ff_pb_15); >>> extern const ymm_reg ff_pb_80; >>> extern const ymm_reg ff_pb_FE; >>> extern const uint64_t ff_pb_FC; >>> diff --git a/libavutil/attributes.h b/libavutil/attributes.h >>> index 04c615c952..dfc35fa31e 100644 >>> --- a/libavutil/attributes.h >>> +++ b/libavutil/attributes.h >>> @@ -40,6 +40,12 @@ >>> # define AV_HAS_BUILTIN(x) 0 >>> #endif >>> >>> +#ifdef __has_attribute >>> +# define AV_HAS_ATTRIBUTE(x) __has_attribute(x) >>> +#else >>> +# define AV_HAS_ATTRIBUTE(x) 0 >>> +#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/attributes_internal.h >>> b/libavutil/attributes_internal.h >>> index bc85ce77ff..c557fa0af0 100644 >>> --- a/libavutil/attributes_internal.h >>> +++ b/libavutil/attributes_internal.h >>> @@ -19,6 +19,7 @@ >>> #ifndef AVUTIL_ATTRIBUTES_INTERNAL_H >>> #define AVUTIL_ATTRIBUTES_INTERNAL_H >>> >>> +#include "config.h" >>> #include "attributes.h" >>> >>> #if (AV_GCC_VERSION_AT_LEAST(4,0) || defined(__clang__)) && >>> (defined(__ELF__) || defined(__MACH__)) >>> @@ -33,4 +34,19 @@ >>> >>> #define EXTERN extern attribute_visibility_hidden >>> >>> +/** >>> + * Some globals defined in C files are used from hardcoded asm that >>> assumes small >>> + * code model (that is, accessing these globals without GOT). This is a >>> problem >>> + * when FFMpeg is built with medium code model (-mcmodel=medium) which >>> allocates >>> + * all globals in a data section that's unreachable with PC relative >>> instructions >>> + * (small code model instruction sequence). We mark all such globals >>> with this >>> + * attribute_mcmodel_small to ensure assembly accessible globals >>> continue to be >>> + * allocated in sections reachable from PC relative instructions. >>> + */ >>> +#if ARCH_X86_64 && defined(__ELF__) && AV_HAS_ATTRIBUTE(model) >>> +# define attribute_mcmodel_small __attribute__((model("small"))) >>> +#else >>> +# define attribute_mcmodel_small >>> +#endif >>> + >>> #endif /* AVUTIL_ATTRIBUTES_INTERNAL_H */ >>> diff --git a/libavutil/mem_internal.h b/libavutil/mem_internal.h >>> index c027fa51c3..efb4c89c39 100644 >>> --- a/libavutil/mem_internal.h >>> +++ b/libavutil/mem_internal.h >>> @@ -29,6 +29,7 @@ >>> #endif >>> >>> #include "attributes.h" >>> +#include "attributes_internal.h" >>> #include "macros.h" >>> >>> /** >>> @@ -113,6 +114,18 @@ >>> #define DECLARE_ALIGNED_32(t,v) DECLARE_ALIGNED_T(ALIGN_32, t, v) >>> #define DECLARE_ALIGNED_64(t,v) DECLARE_ALIGNED_T(ALIGN_64, t, v) >>> >>> +// DECLARE_ASM_VAR used for variables accessed by inline asm >>> +// and external assembly >>> +#define DECLARE_ASM_VAR(n,t,v) \ >>> + attribute_mcmodel_small \ >>> + alignas(n) t v >>> +// DECLARE_EXTERNAL_ASM_VAR used for variables exclusively >>> +// accessed by external assembly >>> +#define DECLARE_EXTERNAL_ASM_VAR(n,t,v) \ >>> + attribute_visibility_hidden \ >>> + attribute_mcmodel_small \ >>> + alignas(n) t v >>> + >>> // Some broken preprocessors need a second expansion >>> // to be forced to tokenize __VA_ARGS__ >>> #define E1(x) x >>> -- >>> 2.49.0.395.g12beb8f557-goog >>> >>> _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
