From: Max Chou <max.chou@sifive.com>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: qemu-devel@nongnu.org, frank.chang@sifive.com, qemu-riscv@nongnu.org
Subject: Re: [PATCH 11/23] target/riscv: Rewrite vext_ldff
Date: Wed, 26 Aug 2026 01:58:47 +0800 [thread overview]
Message-ID: <ao3JcPyrC7cZaCgp@sifive.com> (raw)
In-Reply-To: <20260815194549.1377505-12-richard.henderson@linaro.org>
On 2026-08-15 12:45, Richard Henderson wrote:
> Do not call probe_pages for every active element.
> We can make do with no more than 2 such calls for
> the two pages the insn might reference.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> + /*
> + * Test whether the first page is accessible.
> + * If the first element is active, it must succeed.
> + */
> + flags = probe_access_flags(env, adjust_addr(env, addr),
> + MIN(last, last_in_page) - addr + 1,
> + MMU_DATA_LOAD, mmu_index, !first_active,
> + &host, ra);
>
Hi Richard,
This probe traverses every byte from the initial active element to the
end of the page, not just the bytes of active elements. In the masked
case, the range may encompass a masked-off element, and masked-off body
elements do not perform memory accesses.
I think it may causes unexpected vl.
> + /* Get number of complete elements in the first page. */
> + elems = MIN(page_split / msize, vl - i);
> +
> + /* Load complete elements from the first page. */
> + if (likely(elems)) {
> + uint32_t page_evl = i + elems;
> +
> + if (flags == 0) {
...
> + } else {
> + /*
> + * If the first element is active, it must succeed.
> + * This will load from MMIO or fault from INVALID.
> + */
> + if (first_active) {
> + vext_ldst_nf_tlb(env, vd, addr, 0, nf, esz,
> + max_elems, ldst_tlb, ra);
> + i = 1;
> + addr += msize;
> + }
> +
> + /* Stop if invalid (unmapped) or mmio (transaction may fail). */
> + if (flags & (TLB_INVALID_MASK | TLB_MMIO)) {
> + env->vl = i;
> + goto tail;
> + }
> +
For an example, assume
- vl = 3
- vstart = 0
- the mask be [1, 0, 1]
- assume element 0 and element 2 be readable, but deny the byte at element 1
- all three elements are in the same target page.
In theory, the element 1 is masked off and doesn’t perform any memory
access, so the value of vl remains 3.
But the previous probe covers element 1 to 2 and the flags will be non
zero due to the denied masked-off element 2. Then the vl will set to 1
here.
Maybe we could switch to per element prob when vm is 0 and flags is not 0?
rnax
> + /* None of these ldst_tlb calls may fault. */
> + if (vm) {
> + vext_page_ldst_us_tlb(env, vd, addr, i, page_evl, nf,
> + log2_esz, max_elems,
> + ldst_tlb, mmu_index, ra);
> + } else {
> + do {
> + if (vext_elem_mask(v0, i)) {
> + vext_ldst_nf_tlb(env, vd, base + i * msize, i, nf,
> + esz, max_elems, ldst_tlb, ra);
> + } else if (vma) {
> + vext_set_nf_elems_1s(vd, i, nf, esz, max_elems);
> }
> - remain -= offset;
> - addr_i = adjust_addr(env, addr_i + offset);
> - }
> + } while (++i < page_evl);
> + }
> + }
> +
> + /* Usually the first page contains the entire vector. */
> + if (likely(page_evl == vl)) {
> + goto tail;
> + }
> + i = page_evl;
> + }
> +
> + /* Skip forward to the next active element. */
> + if (!vm) {
> + while (1) {
> + if (vext_elem_mask(v0, i)) {
> + break;
> + }
> + if (vma) {
> + vext_set_nf_elems_1s(vd, i, nf, esz, max_elems);
> + }
> + if (++i == vl) {
> + goto tail;
> }
> }
> }
> -ProbeSuccess:
> - /* load bytes from guest memory */
> - if (vl != 0) {
> - env->vl = vl;
> +
> + addr = base + i * msize;
> + page_split = -(addr | TARGET_PAGE_MASK);
> +
> + /* Validate the second page is accessible. */
> + if (unlikely(page_split < msize)) {
> + /*
> + * Cross page element which isn't first.
> + * We have not yet advanced addr to the next page.
> + */
> + target_ulong next_page = addr + page_split;
> + flags |= probe_access_flags(env, adjust_addr(env, next_page),
> + last - next_page + 1, MMU_DATA_LOAD,
> + mmu_index, true, &host, ra);
> +
> + /* Stop if invalid (unmapped) or mmio (transaction may fail). */
> + if (flags & (TLB_INVALID_MASK | TLB_MMIO)) {
> + env->vl = i;
> + goto tail;
> + }
> +
> + vext_ldst_nf_tlb(env, vd, addr, i, nf, esz, max_elems, ldst_tlb, ra);
> + if (++i == vl) {
> + goto tail;
> + }
> + addr += msize;
> + if (host) {
> + host += addr - next_page;
> + }
> + } else {
> + flags = probe_access_flags(env, adjust_addr(env, addr),
> + last - addr + 1, MMU_DATA_LOAD,
> + mmu_index, true, &host, ra);
> +
> + /* Stop if invalid (unmapped) or mmio (transaction may fail). */
> + if (flags & (TLB_INVALID_MASK | TLB_MMIO)) {
> + env->vl = i;
> + goto tail;
> + }
> }
>
> - if (env->vstart < env->vl) {
> + /* Load complete elements from the second page. */
> + if (flags == 0) {
> if (vm) {
> - /* Load/store elements in the first page */
> - if (likely(elems)) {
> - vext_page_ldst_us(env, vd, addr, elems, nf, max_elems,
> - log2_esz, true, mmu_index, ldst_tlb,
> - ldst_host, ra);
> - }
> -
> - /* Load/store elements in the second page */
> - if (unlikely(env->vstart < env->vl)) {
> - addr = base + env->vstart * msize;
> -
> - /* Cross page element */
> - if (unlikely(page_split % msize)) {
> - vext_ldst_nf_tlb(env, vd, addr, env->vstart, nf,
> - esz, max_elems, ldst_tlb, ra);
> - env->vstart++;
> - addr += msize;
> - }
> -
> - /* Get number of elements of second page */
> - elems = env->vl - env->vstart;
> -
> - /* Load/store elements in the second page */
> - vext_page_ldst_us(env, vd, addr, elems, nf, max_elems,
> - log2_esz, true, mmu_index, ldst_tlb,
> - ldst_host, ra);
> - }
> + vext_page_ldst_us_host(vd, host, i, vl, nf,
> + log2_esz, max_elems, ldst_host);
> } else {
> - for (i = env->vstart; i < env->vl; i++) {
> + host -= addr - base;
> + do {
> + if (vext_elem_mask(v0, i)) {
> + vext_ldst_nf_host(vd, host + i * msize, i, nf, esz,
> + max_elems, ldst_host);
> + } else if (vma) {
> + vext_set_nf_elems_1s(vd, i, nf, esz, max_elems);
> + }
> + } while (++i < vl);
> + }
> + } else {
> + /* None of these ldst_tlb calls may fault. */
> + if (vm) {
> + vext_page_ldst_us_tlb(env, vd, addr, i, vl, nf,
> + log2_esz, max_elems,
> + ldst_tlb, mmu_index, ra);
> + } else {
> + do {
> if (vext_elem_mask(v0, i)) {
> vext_ldst_nf_tlb(env, vd, base + i * msize, i, nf,
> esz, max_elems, ldst_tlb, ra);
> } else if (vma) {
> vext_set_nf_elems_1s(vd, i, nf, esz, max_elems);
> }
> - }
> + } while (++i < vl);
> }
> }
>
> --
> 2.43.0
>
>
next prev parent reply other threads:[~2026-08-25 17:59 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-15 19:45 [PATCH 00/23] target/riscv: Reorg vector load/store Richard Henderson
2026-08-15 19:45 ` [PATCH 01/23] target/riscv: Split out vext_set_nf_elems_1s Richard Henderson
2026-08-15 19:45 ` [PATCH 02/23] target/riscv: Hoist vma check out of vext_set_tail_elems_1s Richard Henderson
2026-08-16 15:25 ` Richard Henderson
2026-08-15 19:45 ` [PATCH 03/23] target/riscv: Split out vext_ldst_nf_tlb Richard Henderson
2026-08-15 19:45 ` [PATCH 04/23] target/riscv: Split out vext_ldst_nf_host Richard Henderson
2026-08-15 19:45 ` [PATCH 05/23] target/riscv: Add evl argument to vext_ldst_elem_fn_host Richard Henderson
2026-08-16 15:55 ` Richard Henderson
2026-08-25 18:30 ` Max Chou
2026-08-15 19:45 ` [PATCH 06/23] target/riscv: Drop is_load parameter from vext_continuous_ldst_tlb Richard Henderson
2026-08-15 19:45 ` [PATCH 07/23] target/riscv: Remove vext_continuous_ldst_tlb Richard Henderson
2026-08-15 19:45 ` [PATCH 08/23] target/riscv: Move misalignment check out of vext_page_ldst_us Richard Henderson
2026-08-15 19:45 ` [PATCH 09/23] target/riscv: Split out vext_page_ldst_us_{host,tlb} Richard Henderson
2026-08-15 19:45 ` [PATCH 10/23] target/riscv: Rewrite vext_ldst_us Richard Henderson
2026-08-25 19:00 ` Max Chou
2026-08-15 19:45 ` [PATCH 11/23] target/riscv: Rewrite vext_ldff Richard Henderson
2026-08-25 17:58 ` Max Chou [this message]
2026-08-25 20:15 ` Richard Henderson
2026-08-26 18:40 ` Max Chou
2026-08-26 21:33 ` Richard Henderson
2026-08-27 12:48 ` Max Chou
2026-08-15 19:45 ` [PATCH 12/23] target/riscv: Split out vext_ldst_us_desc Richard Henderson
2026-08-15 19:45 ` [PATCH 13/23] target/riscv: Use vext_ldst_us in vext_ldst_whole Richard Henderson
2026-08-15 19:45 ` [PATCH 14/23] target/riscv: Mark VSTART_CHECK_EARLY_EXIT unlikely Richard Henderson
2026-08-15 19:45 ` [PATCH 15/23] target/riscv: Remove unused VDATA,WD Richard Henderson
2026-08-15 19:45 ` [PATCH 16/23] target/riscv: Add MEM_IDX, BSWAP, ALIGN to VDATA Richard Henderson
2026-08-15 19:45 ` [PATCH 17/23] target/riscv: Pass MemOpIdx to vext_ldst_us Richard Henderson
2026-08-15 19:45 ` [PATCH 18/23] target/riscv: Build MemOpIdx to vext_ldff Richard Henderson
2026-08-15 19:45 ` [PATCH 19/23] target/riscv: Pass MemOpIdx to vext_ldst_elem_fn_tlb Richard Henderson
2026-08-15 19:45 ` [PATCH 20/23] target/riscv: Use FLATTEN rather than ALWAYS_INLINE for vector ldst Richard Henderson
2026-08-15 19:45 ` [PATCH 21/23] target/riscv: Drop v0 argument from gen_helper_ldst_us Richard Henderson
2026-08-15 19:45 ` [PATCH 22/23] target/riscv: Drop v0 argument from gen_helper_ldst_stride Richard Henderson
2026-08-15 19:45 ` [PATCH 23/23] target/riscv: Drop v0 argument from gen_helper_ldst_index Richard Henderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ao3JcPyrC7cZaCgp@sifive.com \
--to=max.chou@sifive.com \
--cc=frank.chang@sifive.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=richard.henderson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.