Re: [PATCH 10/23] target/riscv: Rewrite vext_ldst_us

On 2026-08-15 12:45, Richard Henderson wrote:
> We already computed the page split here, which means we
> were quite ready to call probe_access directly.  Do so,
> then use vext_ldst_nf_{host,tlb} to load the elements.
> 
> Signed-off-by: Richard Henderson <[email protected]>
> ---
>  target/riscv/tcg/vector_helper.c | 92 ++++++++++++++++++++------------
>  1 file changed, 57 insertions(+), 35 deletions(-)
> 
> diff --git a/target/riscv/tcg/vector_helper.c b/target/riscv/tcg/vector_helper.c
> index 7f721dc24d..5b03e23fc3 100644
> --- a/target/riscv/tcg/vector_helper.c
> +++ b/target/riscv/tcg/vector_helper.c
> @@ -483,7 +483,7 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
>               vext_ldst_elem_fn_host *ldst_host, uint32_t log2_esz,
>               uint32_t evl, uintptr_t ra, bool is_load)
>  {
> -    target_ulong page_split, elems, addr;
> +    target_ulong elems, addr, last, last_in_page, page_split;
>      uint32_t nf = vext_nf(desc);
>      uint32_t vma = vext_vma(desc);
>      uint32_t max_elems = vext_max_elems(desc, log2_esz);
> @@ -491,10 +491,12 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
>      uint32_t msize = nf * esz;
>      int mmu_index = riscv_env_mmu_index(env, false);
>      MMUAccessType access_type = is_load ? MMU_DATA_LOAD : MMU_DATA_STORE;
> +    uint32_t i = env->vstart;
> +    void *host;
>  
>      VSTART_CHECK_EARLY_EXIT(env, evl);
>  
> -    addr = base + env->vstart * msize;
> +    addr = base + i * msize;
>  
>      /* Recognize alignment fault before memory protection fault. */
>      vext_test_alignment(env, addr, esz, access_type, mmu_index, ra);
> @@ -505,52 +507,72 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
>       * by simply calling ldst_tlb.
>       */
>      if (nf == 1 && (evl << log2_esz) <= 6) {
> -        for (uint32_t i = env->vstart; i < evl;
> -             env->vstart = ++i, addr += esz) {
> +        for (; i < evl; env->vstart = ++i, addr += esz) {
>              ldst_tlb(env, adjust_addr(env, addr), i, vd, ra);
>          }
> -        env->vstart = 0;
> -        if (vma) {
> -            vext_set_tail_elems_1s(evl, vd, nf, esz, max_elems);
> -        }
> -        return;
> +        goto tail;
>      }
>  #endif
>  
> -    /* Calculate the page range of first page */
> -    page_split = -(addr | TARGET_PAGE_MASK);
> -    /* Get number of elements */
> -    elems = page_split / msize;
> -    if (unlikely(env->vstart + elems >= evl)) {
> -        elems = evl - env->vstart;
> -    }
> +    /* Calculate the page range of first page. */
> +    last = base + evl * msize - 1;
> +    last_in_page = addr | ~TARGET_PAGE_MASK;
> +    page_split = last_in_page - addr;
> +
> +    /* Validate the first page is accessible. */
> +    host = probe_access(env, adjust_addr(env, addr),
> +                        MIN(last, last_in_page) - addr + 1,
> +                        access_type, mmu_index, ra);
> +
> +    /* Get number of complete elements in the first page. */
> +    elems = MIN(page_split / msize, evl - i);
>  

The page_split may excludes the byte at last_in_page, which the probe
size includes it. That elems will undercount a complete element ending
at the page boundary.
Then the following cross page element and second page probe will
be affected.

Maybe we could fix it by replacing the page_split with something like
probe_size below:

+    target_unlong probe_bytes = MIN(last, last_in_page) - addr + 1;
 
     /* Validate the first page is accessible. */
-    host = probe_access(env, adjust_addr(env, addr),
-                        MIN(last, last_in_page) - addr + 1,
+    host = probe_access(env, adjust_addr(env, addr), probe_bytes,
                         access_type, mmu_index, ra);
 
     /* Get number of complete elements in the first page. */
-    elems = MIN(page_split / msize, evl - i);
+    elems = MIN(probe_bytes / msize, evl - i);
 

> +    /* Cross page element */
> +    if (unlikely(page_split % msize)) {

+    if (unlikely(probe_bytes % msize)) {

> +        vext_ldst_nf_tlb(env, vd, addr, i++, nf, esz, max_elems, ldst_tlb, ra);
> +        if (i == evl) {
> +            goto tail;
> +        }
> +        env->vstart = i;
> +        addr += msize;
> +    }
> +
> +    /* Validate the second page is accessible. */
> +    assert(i < evl);
> +    elems = evl - i;
> +    host = probe_access(env, adjust_addr(env, addr), elems * msize,

+    probe_bytes = last - addr + 1;
+    host = probe_access(env, adjust_addr(env, addr), probe_bytes,

> +                        access_type, mmu_index, ra);

rnax