Re: [Qemu-devel] [PATCH 12/20] target/arm: Rewrite helper_sve_ld1*_r using pages

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> Uses tlb_vaddr_to_host for correct operation with softmmu.
> Optimize for accesses within a single page or pair of pages.
>
> Perf report comparison for cortex-strings test-strlen
> with aarch64-linux-user:
>
<snip>
> +/*
> + * Common helper for all contiguous one-register predicated loads.
> + */
> +static void sve_ld1_r(CPUARMState *env, void *vg, const target_ulong addr,
> +                      uint32_t desc, const uintptr_t retaddr,
> +                      const int esz, const int msz,
> +                      sve_ld1_host_fn *host_fn,
> +                      sve_ld1_tlb_fn *tlb_fn)
> +{
> +    void *vd = &env->vfp.zregs[simd_data(desc)];
> +    const int diffsz = esz - msz;
> +    const intptr_t reg_max = simd_oprsz(desc);
> +    const intptr_t mem_max = reg_max >> diffsz;
> +    const int mmu_idx = cpu_mmu_index(env, false);
> +    ARMVectorReg scratch;
> +    void *host, *result;
> +    intptr_t split;
> +
> +    set_helper_retaddr(retaddr);
> +
> +    host = tlb_vaddr_to_host(env, addr, MMU_DATA_LOAD, mmu_idx);
> +    if (test_host_page(host)) {
> +        split = max_for_page(addr, 0, mem_max);
> +        if (likely(split == mem_max)) {
> +            /* The load is entirely within a valid page.  For softmmu,
> +             * no faults.  For user-only, if the first byte does not
> +             * fault then none of them will fault, so Vd will never be
> +             * partially modified.
> +             */
> +            host_fn(vd, vg, host, 0, mem_max);
> +            set_helper_retaddr(0);
> +            return;
> +        }
> +    }
> +
> +    /* Perform the predicated read into a temporary, thus ensuring
> +     * if the load of the last element faults, Vd is not modified.
> +     */
> +    result = &scratch;
> +#ifdef CONFIG_USER_ONLY
> +    host_fn(vd, vg, host, 0, mem_max);
> +#else
> +    memset(result, 0, reg_max);
> +    for (intptr_t reg_off = find_next_active(vg, 0, reg_max, esz);

Hmm this blew up CI complaining about c99-isms, but QEMU is supposed to
be c99 compliant.

  https://travis-ci.org/stsquad/qemu/builds/414248994

--
Alex Bennée