Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Pranav Kant via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: Pranav Kant <prka@google.com>
Subject: Re: [FFmpeg-devel] [PATCH] Mark C globals with small code model
Date: Thu, 6 Mar 2025 18:14:05 -0800
Message-ID: <CAPvNhsJQGyCAeCDXSvYQCNcyVwR2czFuc2O__AbdAViOppfMYg@mail.gmail.com> (raw)
In-Reply-To: <29f03aab-1c56-46da-93ff-c96d66ef7b9a@lynne.ee>

I think you were looking at an older version of the patch. Newer version
didn't have this. Anyhow, there's a new version I uploaded (v3).

On Thu, Feb 27, 2025 at 6:31 PM Lynne <dev@lynne.ee> wrote:

> On 25/02/2025 22:37, Pranav Kant via ffmpeg-devel 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, assembly files continue to access these
> > globals defined in C/C++ files using older (and invalid instruction
> > sequence). So, we mark all such globals with an attribute that forces
> > them to be allocated in small sections allowing them to validly be
> > accessed from the 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
> > ---
> >   libavcodec/ac3dsp.c        | 2 +-
> >   libavcodec/cabac.c         | 3 ++-
> >   libavcodec/x86/constants.c | 8 ++++++++
> >   libavutil/attributes.h     | 6 ++++++
> >   4 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/ac3dsp.c b/libavcodec/ac3dsp.c
> > index 730fa70fff..43b4fcbda9 100644
> > --- a/libavcodec/ac3dsp.c
> > +++ b/libavcodec/ac3dsp.c
> > @@ -104,7 +104,7 @@ static void ac3_update_bap_counts_c(uint16_t
> mant_cnt[16], uint8_t *bap,
> >           mant_cnt[bap[len]]++;
> >   }
> >
> > -DECLARE_ALIGNED(16, const uint16_t, ff_ac3_bap_bits)[16] = {
> > +av_mcmodel_small DECLARE_ALIGNED(16, const uint16_t,
> ff_ac3_bap_bits)[16] = {
> >       0,  0,  0,  3,  0,  4,  5,  6,  7,  8,  9, 10, 11, 12, 14, 16
> >   };
> >
> > diff --git a/libavcodec/cabac.c b/libavcodec/cabac.c
> > index 7d41cd2ae6..dfc3ba135a 100644
> > --- a/libavcodec/cabac.c
> > +++ b/libavcodec/cabac.c
> > @@ -24,12 +24,13 @@
> >    * Context Adaptive Binary Arithmetic Coder.
> >    */
> >
> > +#include "libavutil/attributes.h"
> >   #include "libavutil/error.h"
> >   #include "libavutil/mem_internal.h"
> >
> >   #include "cabac.h"
> >
> > -DECLARE_ASM_ALIGNED(1, const uint8_t, ff_h264_cabac_tables)[512 +
> 4*2*64 + 4*64 + 63] = {
> > +av_mcmodel_small DECLARE_ASM_ALIGNED(1, const uint8_t,
> ff_h264_cabac_tables)[512 + 4*2*64 + 4*64 + 63] = {
> >       9,8,7,7,6,6,6,6,5,5,5,5,5,5,5,5,
> >       4,4,4,4,4,4,4,4,4,4,4,4,4,4,4,4,
> >       3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,
> > diff --git a/libavcodec/x86/constants.c b/libavcodec/x86/constants.c
> > index bc7f2b17b8..9a5af2871c 100644
> > --- a/libavcodec/x86/constants.c
> > +++ b/libavcodec/x86/constants.c
> > @@ -18,17 +18,21 @@
> >    * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> >    */
> >
> > +#include "libavutil/attributes.h"
> >   #include "libavutil/mem_internal.h"
> >   #include "libavutil/x86/asm.h" // for xmm_reg
> >   #include "constants.h"
> >
> > +av_mcmodel_small
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_1)    = {
> 0x0001000100010001ULL, 0x0001000100010001ULL,
> >
>  0x0001000100010001ULL, 0x0001000100010001ULL };
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_2)    = {
> 0x0002000200020002ULL, 0x0002000200020002ULL,
> >
>  0x0002000200020002ULL, 0x0002000200020002ULL };
> >   DECLARE_ASM_ALIGNED(16, const xmm_reg,  ff_pw_3)    = {
> 0x0003000300030003ULL, 0x0003000300030003ULL };
> > +av_mcmodel_small
> >   DECLARE_ASM_ALIGNED(32, const ymm_reg,  ff_pw_4)    = {
> 0x0004000400040004ULL, 0x0004000400040004ULL,
> >
>  0x0004000400040004ULL, 0x0004000400040004ULL };
> > +av_mcmodel_small
> >   DECLARE_ASM_ALIGNED(16, const xmm_reg,  ff_pw_5)    = {
> 0x0005000500050005ULL, 0x0005000500050005ULL };
> >   DECLARE_ALIGNED(16, const xmm_reg,  ff_pw_8)    = {
> 0x0008000800080008ULL, 0x0008000800080008ULL };
> >   DECLARE_ASM_ALIGNED(16, const xmm_reg,  ff_pw_9)    = {
> 0x0009000900090009ULL, 0x0009000900090009ULL };
> > @@ -49,6 +53,7 @@ DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_256)  = {
> 0x0100010001000100ULL, 0x010
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_512)  = {
> 0x0200020002000200ULL, 0x0200020002000200ULL,
> >
>  0x0200020002000200ULL, 0x0200020002000200ULL };
> >   DECLARE_ALIGNED(16, const xmm_reg,  ff_pw_1019) = {
> 0x03FB03FB03FB03FBULL, 0x03FB03FB03FB03FBULL };
> > +av_mcmodel_small
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_1023) = {
> 0x03ff03ff03ff03ffULL, 0x03ff03ff03ff03ffULL,
> >
>  0x03ff03ff03ff03ffULL, 0x03ff03ff03ff03ffULL};
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_1024) = {
> 0x0400040004000400ULL, 0x0400040004000400ULL,
> > @@ -66,13 +71,16 @@ DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_m1)   = {
> 0xFFFFFFFFFFFFFFFFULL, 0xFFF
> >
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pb_0)    = {
> 0x0000000000000000ULL, 0x0000000000000000ULL,
> >
>  0x0000000000000000ULL, 0x0000000000000000ULL };
> > +av_mcmodel_small
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pb_1)    = {
> 0x0101010101010101ULL, 0x0101010101010101ULL,
> >
>  0x0101010101010101ULL, 0x0101010101010101ULL };
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pb_2)    = {
> 0x0202020202020202ULL, 0x0202020202020202ULL,
> >
>  0x0202020202020202ULL, 0x0202020202020202ULL };
> > +av_mcmodel_small
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pb_3)    = {
> 0x0303030303030303ULL, 0x0303030303030303ULL,
> >
>  0x0303030303030303ULL, 0x0303030303030303ULL };
> >   DECLARE_ALIGNED(32, const xmm_reg,  ff_pb_15)   = {
> 0x0F0F0F0F0F0F0F0FULL, 0x0F0F0F0F0F0F0F0FULL };
> > +av_mcmodel_small
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pb_80)   = {
> 0x8080808080808080ULL, 0x8080808080808080ULL,
> >
>  0x8080808080808080ULL, 0x8080808080808080ULL };
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pb_FE)   = {
> 0xFEFEFEFEFEFEFEFEULL, 0xFEFEFEFEFEFEFEFEULL,
> > diff --git a/libavutil/attributes.h b/libavutil/attributes.h
> > index 04c615c952..704a7070db 100644
> > --- a/libavutil/attributes.h
> > +++ b/libavutil/attributes.h
> > @@ -104,6 +104,12 @@
> >   #    define attribute_deprecated
> >   #endif
> >
> > +#if defined(__clang__) && __has_attribute(model)
> > +#    define av_mcmodel_small __attribute__((model("small")))
> > +#else
> > +#    define av_mcmodel_small
> > +#endif
>
> That's a bad macro name, moreover being a compiler-specific name.
> Could you consider calling it simply av_global?
> _______________________________________________
> 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".
>
_______________________________________________
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".

      reply	other threads:[~2025-03-07  2:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-25 21:37 Pranav Kant via ffmpeg-devel
2025-02-25 23:03 ` Andreas Rheinhardt
2025-02-26 19:44 ` Pranav Kant via ffmpeg-devel
2025-02-26 19:45   ` Pranav Kant via ffmpeg-devel
2025-02-27  3:36   ` James Zern via ffmpeg-devel
2025-02-28  1:14   ` Michael Niedermayer
2025-03-07  2:13     ` Pranav Kant via ffmpeg-devel
2025-03-03 20:56   ` [FFmpeg-devel] [PATCH v3] " Pranav Kant via ffmpeg-devel
2025-02-28  2:31 ` [FFmpeg-devel] [PATCH] " Lynne
2025-03-07  2:14   ` Pranav Kant via ffmpeg-devel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPvNhsJQGyCAeCDXSvYQCNcyVwR2czFuc2O__AbdAViOppfMYg@mail.gmail.com \
    --to=ffmpeg-devel@ffmpeg.org \
    --cc=prka@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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