Re: [Qemu-devel] [RFC PATCH v1 2/2] target-arm: Use Neon for zero checking

[Paolo Bonzini <pbonzini@redhat.com>]

On 05/04/2016 16:36, Peter Maydell wrote:
>> > +
>> > +#define BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR_NEON 16
>> > +
>> > +/*
>> > + * Zero page/buffer checking using SIMD(Neon)
>> > + */
>> > +
>> > +static bool
>> > +can_use_buffer_find_nonzero_offset_neon(const void *buf, size_t len)
>> > +{
>> > +    return (len % (BUFFER_FIND_NONZERO_OFFSET_UNROLL_FACTOR_NEON
>> > +                   * sizeof(NEON_VECTYPE)) == 0
>> > +            && ((uintptr_t) buf) % sizeof(NEON_VECTYPE) == 0);
>> > +}
>> > +
>> > +static size_t buffer_find_nonzero_offset_neon(const void *buf, size_t len)
>> > +{
>> > +    size_t i;
>> > +    NEON_VECTYPE d0, d1, d2, d3, d4, d5, d6;
>> > +    NEON_VECTYPE d7, d8, d9, d10, d11, d12, d13, d14;
>> > +    uint64_t const *data = buf;
>> > +
>> > +    assert(can_use_buffer_find_nonzero_offset_neon(buf, len));
>> > +    len /= sizeof(unsigned long);
>> > +
>> > +    for (i = 0; i < len; i += 32) {
>> > +        d0 = NEON_LOAD_N_ORR(data[i], data[i + 2]);
>> > +        d1 = NEON_LOAD_N_ORR(data[i + 4], data[i + 6]);
>> > +        d2 = NEON_LOAD_N_ORR(data[i + 8], data[i + 10]);
>> > +        d3 = NEON_LOAD_N_ORR(data[i + 12], data[i + 14]);
>> > +        d4 = NEON_ORR(d0, d1);
>> > +        d5 = NEON_ORR(d2, d3);
>> > +        d6 = NEON_ORR(d4, d5);
>> > +
>> > +        d7 = NEON_LOAD_N_ORR(data[i + 16], data[i + 18]);
>> > +        d8 = NEON_LOAD_N_ORR(data[i + 20], data[i + 22]);
>> > +        d9 = NEON_LOAD_N_ORR(data[i + 24], data[i + 26]);
>> > +        d10 = NEON_LOAD_N_ORR(data[i + 28], data[i + 30]);
>> > +        d11 = NEON_ORR(d7, d8);
>> > +        d12 = NEON_ORR(d9, d10);
>> > +        d13 = NEON_ORR(d11, d12);
>> > +
>> > +        d14 = NEON_ORR(d6, d13);
>> > +        if (NEON_EQ_ZERO(d14)) {
>> > +            break;
>> > +        }
>> > +    }
> Both the other optimised find_nonzero implementations in this
> file have two loops, not just one. Is it OK that this
> implementation has only a single loop?
> 
> Paolo: do you know why we have two loops in the other
> implementations?

Because usually the first one or two iterations are enough to exit the
function if the page is nonzero.  It's measurably slower to go through
the unrolled loop in that case.  On the other hand, once the first few
iterations found only zero bytes, the buffer is very likely entirely
zero and the unrolled loop helps.

But in theory it should be enough to add a new #elif branch like this:

#include "arm_neon.h"
#define VECTYPE   uint64x2_t
#define VEC_OR(a, b) ((a) | (b))
#define ALL_EQ(a, b) /* ??? :) */

around the

/* vector definitions */

comment in util/cutils.c.  GCC should do everything else.

Paolo