From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, mark.cave-ayland@ilande.co.uk,
tony.nguyen@bt.com
Subject: Re: [Qemu-devel] [PATCH 2/3] cputlb: Replace switches in load/store_helper with callback
Date: Wed, 11 Sep 2019 15:07:20 +0200 [thread overview]
Message-ID: <500ed763-5c50-0be5-993d-9b8b1b68b932@redhat.com> (raw)
In-Reply-To: <20190911014353.5926-3-richard.henderson@linaro.org>
On 9/11/19 3:43 AM, Richard Henderson wrote:
> Add a function parameter to perform the actual load/store to ram.
> With optimization, this results in identical code.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> accel/tcg/cputlb.c | 157 +++++++++++++++++++++++----------------------
> 1 file changed, 81 insertions(+), 76 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 909f01ebcc..e6229d100a 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1292,11 +1292,37 @@ static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
>
> typedef uint64_t FullLoadHelper(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr);
> +typedef uint64_t DirectLoadHelper(const void *);
> +
> +static inline uint64_t direct_ldub(const void *haddr)
> +{
> + return *(uint8_t *)haddr;
> +}
> +
> +static inline uint64_t direct_lduw_be(const void *haddr)
> +{
> + return lduw_be_p(haddr);
> +}
> +
> +static inline uint64_t direct_lduw_le(const void *haddr)
> +{
> + return lduw_le_p(haddr);
> +}
> +
> +static inline uint64_t direct_ldul_be(const void *haddr)
> +{
> + return (uint32_t)ldl_be_p(haddr);
> +}
> +
> +static inline uint64_t direct_ldul_le(const void *haddr)
> +{
> + return (uint32_t)ldl_le_p(haddr);
> +}
>
> static inline uint64_t ALWAYS_INLINE
> load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
> uintptr_t retaddr, MemOp op, bool code_read,
> - FullLoadHelper *full_load)
> + FullLoadHelper *full_load, DirectLoadHelper *direct)
> {
> uintptr_t mmu_idx = get_mmuidx(oi);
> uintptr_t index = tlb_index(env, mmu_idx, addr);
> @@ -1385,33 +1411,7 @@ load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
>
> do_aligned_access:
> haddr = (void *)((uintptr_t)addr + entry->addend);
> - switch (op) {
> - case MO_UB:
> - res = ldub_p(haddr);
> - break;
> - case MO_BEUW:
> - res = lduw_be_p(haddr);
> - break;
> - case MO_LEUW:
> - res = lduw_le_p(haddr);
> - break;
> - case MO_BEUL:
> - res = (uint32_t)ldl_be_p(haddr);
> - break;
> - case MO_LEUL:
> - res = (uint32_t)ldl_le_p(haddr);
> - break;
> - case MO_BEQ:
> - res = ldq_be_p(haddr);
> - break;
> - case MO_LEQ:
> - res = ldq_le_p(haddr);
> - break;
> - default:
> - g_assert_not_reached();
> - }
> -
> - return res;
> + return direct(haddr);
> }
>
> /*
> @@ -1427,7 +1427,8 @@ load_helper(CPUArchState *env, target_ulong addr, TCGMemOpIdx oi,
> static uint64_t full_ldub_mmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - return load_helper(env, addr, oi, retaddr, MO_UB, false, full_ldub_mmu);
> + return load_helper(env, addr, oi, retaddr, MO_UB, false,
> + full_ldub_mmu, direct_ldub);
> }
>
> tcg_target_ulong helper_ret_ldub_mmu(CPUArchState *env, target_ulong addr,
> @@ -1440,7 +1441,7 @@ static uint64_t full_le_lduw_mmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_LEUW, false,
> - full_le_lduw_mmu);
> + full_le_lduw_mmu, direct_lduw_le);
Why not cast lduw_be_p? (except for direct_ldub).
return load_helper(env, addr, oi, retaddr, MO_LEUW, false,
full_le_lduw_mmu, (DirectLoadHelper)lduw_be_p);
Useful to set breakpoint while debugging?
> }
>
> tcg_target_ulong helper_le_lduw_mmu(CPUArchState *env, target_ulong addr,
> @@ -1453,7 +1454,7 @@ static uint64_t full_be_lduw_mmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_BEUW, false,
> - full_be_lduw_mmu);
> + full_be_lduw_mmu, direct_lduw_be);
> }
>
> tcg_target_ulong helper_be_lduw_mmu(CPUArchState *env, target_ulong addr,
> @@ -1466,7 +1467,7 @@ static uint64_t full_le_ldul_mmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_LEUL, false,
> - full_le_ldul_mmu);
> + full_le_ldul_mmu, direct_ldul_le);
> }
>
> tcg_target_ulong helper_le_ldul_mmu(CPUArchState *env, target_ulong addr,
> @@ -1479,7 +1480,7 @@ static uint64_t full_be_ldul_mmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_BEUL, false,
> - full_be_ldul_mmu);
> + full_be_ldul_mmu, direct_ldul_be);
> }
>
> tcg_target_ulong helper_be_ldul_mmu(CPUArchState *env, target_ulong addr,
> @@ -1492,14 +1493,14 @@ uint64_t helper_le_ldq_mmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_LEQ, false,
> - helper_le_ldq_mmu);
> + helper_le_ldq_mmu, ldq_le_p);
> }
>
> uint64_t helper_be_ldq_mmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_BEQ, false,
> - helper_be_ldq_mmu);
> + helper_be_ldq_mmu, ldq_be_p);
> }
>
> /*
> @@ -1542,9 +1543,37 @@ tcg_target_ulong helper_be_ldsl_mmu(CPUArchState *env, target_ulong addr,
> * Store Helpers
> */
>
> +typedef void DirectStoreHelper(void *, uint64_t);
> +
> +static inline void direct_stb(void *haddr, uint64_t val)
> +{
> + *(uint8_t *)haddr = val;
> +}
> +
> +static inline void direct_stw_be(void *haddr, uint64_t val)
> +{
> + stw_be_p(haddr, val);
> +}
> +
> +static inline void direct_stw_le(void *haddr, uint64_t val)
> +{
> + stw_le_p(haddr, val);
> +}
> +
> +static inline void direct_stl_be(void *haddr, uint64_t val)
> +{
> + stl_be_p(haddr, val);
> +}
> +
> +static inline void direct_stl_le(void *haddr, uint64_t val)
> +{
> + stl_le_p(haddr, val);
> +}
> +
> static inline void ALWAYS_INLINE
> store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
> - TCGMemOpIdx oi, uintptr_t retaddr, MemOp op)
> + TCGMemOpIdx oi, uintptr_t retaddr, MemOp op,
> + DirectStoreHelper *direct)
> {
> uintptr_t mmu_idx = get_mmuidx(oi);
> uintptr_t index = tlb_index(env, mmu_idx, addr);
> @@ -1669,74 +1698,49 @@ store_helper(CPUArchState *env, target_ulong addr, uint64_t val,
>
> do_aligned_access:
> haddr = (void *)((uintptr_t)addr + entry->addend);
> - switch (op) {
> - case MO_UB:
> - stb_p(haddr, val);
> - break;
> - case MO_BEUW:
> - stw_be_p(haddr, val);
> - break;
> - case MO_LEUW:
> - stw_le_p(haddr, val);
> - break;
> - case MO_BEUL:
> - stl_be_p(haddr, val);
> - break;
> - case MO_LEUL:
> - stl_le_p(haddr, val);
> - break;
> - case MO_BEQ:
> - stq_be_p(haddr, val);
> - break;
> - case MO_LEQ:
> - stq_le_p(haddr, val);
> - break;
> - default:
> - g_assert_not_reached();
> - break;
> - }
> + direct(haddr, val);
> }
>
> void helper_ret_stb_mmu(CPUArchState *env, target_ulong addr, uint8_t val,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - store_helper(env, addr, val, oi, retaddr, MO_UB);
> + store_helper(env, addr, val, oi, retaddr, MO_UB, direct_stb);
> }
>
> void helper_le_stw_mmu(CPUArchState *env, target_ulong addr, uint16_t val,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - store_helper(env, addr, val, oi, retaddr, MO_LEUW);
> + store_helper(env, addr, val, oi, retaddr, MO_LEUW, direct_stw_le);
Ditto, why not cast with DirectStoreHelper? (except for direct_stb).
> }
>
> void helper_be_stw_mmu(CPUArchState *env, target_ulong addr, uint16_t val,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - store_helper(env, addr, val, oi, retaddr, MO_BEUW);
> + store_helper(env, addr, val, oi, retaddr, MO_BEUW, direct_stw_be);
> }
>
> void helper_le_stl_mmu(CPUArchState *env, target_ulong addr, uint32_t val,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - store_helper(env, addr, val, oi, retaddr, MO_LEUL);
> + store_helper(env, addr, val, oi, retaddr, MO_LEUL, direct_stl_le);
> }
>
> void helper_be_stl_mmu(CPUArchState *env, target_ulong addr, uint32_t val,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - store_helper(env, addr, val, oi, retaddr, MO_BEUL);
> + store_helper(env, addr, val, oi, retaddr, MO_BEUL, direct_stl_be);
> }
>
> void helper_le_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - store_helper(env, addr, val, oi, retaddr, MO_LEQ);
> + store_helper(env, addr, val, oi, retaddr, MO_LEQ, stq_le_p);
> }
>
> void helper_be_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - store_helper(env, addr, val, oi, retaddr, MO_BEQ);
> + store_helper(env, addr, val, oi, retaddr, MO_BEQ, stq_be_p);
> }
>
> /* First set of helpers allows passing in of OI and RETADDR. This makes
> @@ -1801,7 +1805,8 @@ void helper_be_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
> static uint64_t full_ldub_cmmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> - return load_helper(env, addr, oi, retaddr, MO_8, true, full_ldub_cmmu);
> + return load_helper(env, addr, oi, retaddr, MO_8, true,
> + full_ldub_cmmu, direct_ldub);
> }
>
> uint8_t helper_ret_ldb_cmmu(CPUArchState *env, target_ulong addr,
> @@ -1814,7 +1819,7 @@ static uint64_t full_le_lduw_cmmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_LEUW, true,
> - full_le_lduw_cmmu);
> + full_le_lduw_cmmu, direct_lduw_le);
> }
>
> uint16_t helper_le_ldw_cmmu(CPUArchState *env, target_ulong addr,
> @@ -1827,7 +1832,7 @@ static uint64_t full_be_lduw_cmmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_BEUW, true,
> - full_be_lduw_cmmu);
> + full_be_lduw_cmmu, direct_lduw_be);
> }
>
> uint16_t helper_be_ldw_cmmu(CPUArchState *env, target_ulong addr,
> @@ -1840,7 +1845,7 @@ static uint64_t full_le_ldul_cmmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_LEUL, true,
> - full_le_ldul_cmmu);
> + full_le_ldul_cmmu, direct_ldul_le);
> }
>
> uint32_t helper_le_ldl_cmmu(CPUArchState *env, target_ulong addr,
> @@ -1853,7 +1858,7 @@ static uint64_t full_be_ldul_cmmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_BEUL, true,
> - full_be_ldul_cmmu);
> + full_be_ldul_cmmu, direct_ldul_be);
> }
>
> uint32_t helper_be_ldl_cmmu(CPUArchState *env, target_ulong addr,
> @@ -1866,12 +1871,12 @@ uint64_t helper_le_ldq_cmmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_LEQ, true,
> - helper_le_ldq_cmmu);
> + helper_le_ldq_cmmu, ldq_le_p);
> }
>
> uint64_t helper_be_ldq_cmmu(CPUArchState *env, target_ulong addr,
> TCGMemOpIdx oi, uintptr_t retaddr)
> {
> return load_helper(env, addr, oi, retaddr, MO_BEQ, true,
> - helper_be_ldq_cmmu);
> + helper_be_ldq_cmmu, ldq_be_p);
> }
>
next prev parent reply other threads:[~2019-09-11 13:09 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-11 1:43 [Qemu-devel] [PATCH 0/3] cputlb: Adjust tlb bswap implementation Richard Henderson
2019-09-11 1:43 ` [Qemu-devel] [PATCH 1/3] cputlb: Disable __always_inline__ without optimization Richard Henderson
2019-09-11 8:34 ` Peter Maydell
2019-09-11 1:43 ` [Qemu-devel] [PATCH 2/3] cputlb: Replace switches in load/store_helper with callback Richard Henderson
2019-09-11 10:55 ` Tony Nguyen
2019-09-11 13:22 ` Richard Henderson
2019-09-11 15:17 ` Tony Nguyen
2019-09-11 13:07 ` Philippe Mathieu-Daudé [this message]
2019-09-11 13:20 ` Richard Henderson
2019-09-11 1:43 ` [Qemu-devel] [PATCH 3/3] cputlb: Introduce TLB_BSWAP Richard Henderson
2019-09-11 14:56 ` Tony Nguyen
2019-09-12 19:07 ` Richard Henderson
2019-09-11 3:46 ` [Qemu-devel] [PATCH 0/3] cputlb: Adjust tlb bswap implementation no-reply
2019-09-11 9:55 ` no-reply
2019-09-11 13:07 ` no-reply
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=500ed763-5c50-0be5-993d-9b8b1b68b932@redhat.com \
--to=philmd@redhat.com \
--cc=mark.cave-ayland@ilande.co.uk \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=tony.nguyen@bt.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).