Re: [FFmpeg-devel] [PATCH v2 1/2] lavu/hashtable: create generic robin hood hash table

[Connor Worley <connorbworley@gmail.com>]

On Wed, Feb 7, 2024 at 2:16 AM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Connor Worley:
> > +#define ENTRY_PSL(entry) (entry)
> > +#define ENTRY_OCC(entry) (ENTRY_PSL(entry) + sizeof(size_t))
> > +#define ENTRY_KEY(entry) (ENTRY_OCC(entry) + 4)
> > +#define ENTRY_VAL(entry) (ENTRY_KEY(entry) + FFALIGN(ctx->key_size, 4))
> > +
>
> It seems you misunderstood what I said about alignment in my last
> review. You should align entry_size and not align the size of key and
> val fields. The way you are doing it now does not prevent unaligned
> accesses: Imagine the common case of size_t having an alignment
> requirement of 8 and key_size and val_size being four. Then entry_size
> is not a multiple of 8 and therefore of two consecutive entries, only
> one can be properly aligned (e.g. the second entry
> ctx->table+ctx->entry_size is misaligned).
> (For the same reason, the alignment of a struct is a multiple of the
> alignment of every member of said struct.)
> The way you are doing it also adds FFALIGN to every access to val.
>

Apologies as I don't usually write C. Is it not necessary to ensure val is
aligned?
Would aligning every field to sizeof(size_t) be sufficient for ensuring
consecutive entries are aligned?

I believe I would need to store a possibly rounded-up key_size to avoid the
FFALIGN in val access.

-- 
Connor Worley
_______________________________________________
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".