On Tue, 28 Jul 2026 at 10:22, Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> On 28/7/26 11:01, Peter Maydell wrote:
> > On Tue, 28 Jul 2026 at 04:01, Gavin Shan <[email protected]> wrote:
> >>
> >> On 7/27/26 10:51 PM, Peter Maydell wrote:
> >>>> + test = (uintptr_t)src | (uintptr_t)dst | n;
> >>>> + len = test & -test;
> >>>
> >>> What is this doing? I am not a fan of clever bit twiddling
> >>> that isn't commented to explain itself. Readers of the code
> >>> shouldn't have to go off and search for an explanation of what
> >>> is going on.
> >>>
> >>
> >> The following comments will be added for (v5).
> >>
> >> /*
> >> * Maximal length of aligned access that are determined by @src,
> >> * @dst and @n
> >> */
> >
> > What I mean is more that it ought to say why "x & -x"
> > achieves that goal.
>
> Hmm isn't "x & -x" restricted to power of 2? Here we have no garanty
> @test is, because we have no garanty @n is. Ah, this is catched by
> the 'len != n' check and returns after calling memmove(). Indeed not
> very clear notation. Again:
>
> ...
>
> /* Overlapping buffers, unaligned or oversized access */
> if (!QEMU_IS_ALIGNED((uintptr_t)src | (uintptr_t)dst, n)) {
> memmove(dst, src, n);
> return;
> }
>
> switch (n) {
> ...
I'm not inherently against the bit twiddling (QEMU_IS_ALIGNED
on a variable length will do a division, maybe we care?);
I just want that if we do use a bit-twiddling trick that we
explain why it does the thing we want it to do.
-- PMM
