| Message ID | 20190911014353.5926-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | cputlb: Adjust tlb bswap implementation | expand |
On Tue, Sep 10, 2019 at 09:43:52PM -0400, 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 *); Would 'Load' instead of 'DirectLoadHelper' have enough clarity? If so, consider also dropping the 'direct_' prefix in the functions below. > + > +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); > } > > 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); Like 'Load', would 'Store' instead of 'DirectStoreHelper' have enough clarity? > + > +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); > } > > 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); > } > -- > 2.17.1 > > Reviewed-by: Tony Nguyen <tony.nguyen@bt.com>
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); > } >
On 9/11/19 9:07 AM, Philippe Mathieu-Daudé wrote: >> { >> 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? Because the types in the function signature are different, and such a cast will cause misbehavior. r~
On 9/11/19 6:55 AM, Tony Nguyen wrote: >> typedef uint64_t FullLoadHelper(CPUArchState *env, target_ulong addr, >> TCGMemOpIdx oi, uintptr_t retaddr); >> +typedef uint64_t DirectLoadHelper(const void *); > > Would 'Load' instead of 'DirectLoadHelper' have enough clarity? I suppose so, yes. > If so, consider also dropping the 'direct_' prefix in the functions below. > >> + >> +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); >> +} I would be hesitant to call this just "lduw_be"; I think that's confusing when it's calling lduw_be_p. But perhaps "wrap_*" or "wide_*"? r~
On Wed, Sep 11, 2019 at 09:22:39AM -0400, Richard Henderson wrote: > I would be hesitant to call this just "lduw_be"; I think that's confusing when > it's calling lduw_be_p. But perhaps "wrap_*" or "wide_*"? Agree, some hamming distance is needed. "wrap_*", "wide_*", or "direct_*" all works for me.
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); } 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); } 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); }
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(-) -- 2.17.1