Re: [PATCH 05/37] include/exec: Inline *_mmuidx_ra memory operations
On 3/13/25 11:26, Philippe Mathieu-Daudé wrote:
On 13/3/25 19:05, Richard Henderson wrote:
On 3/13/25 09:59, Pierrick Bouvier wrote:
+static inline int
+cpu_ldsw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+ int mmu_idx, uintptr_t ra)
+{
+ return (int16_t)cpu_lduw_be_mmuidx_ra(env, addr, mmu_idx, ra);
For my personal culture, is that strictly equivalent to doing the load with
MO_BESW?
If you're asking if it's the same as passing MO_BESW to tcg_gen_qemu_ld_i32(), yes. The
tcg code generator takes care of making the value sign-extended.
If you're asking if it's the same as passing MO_BESW to cpu_ldw_mmu(), no. The core
functions only handle unsigned values. This older api contained functions with a signed
return value, so we preserve that.
Are these 2 APIs doing the same thing? What are the uses? Can we rename
the legacy one?
There are 4 apis doing the same thing:
cpu_ld*_data()
cpu_ld*_data_ra()
cpu_ld*_mmuidx_ra()
cpu_ld*_mmu()
It would be lovely to get rid of some of them.
A mere matter of coccinelle and lots of testing, I guess.
r~
Re: [PATCH 05/37] include/exec: Inline *_mmuidx_ra memory operations
On 3/13/25 10:11, Pierrick Bouvier wrote: -uint64_t cpu_ldq_le_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); Not related to the change, but the naming _ra is very confusing, since it means the opposite of what it seems. *NO* requirement alignment, when you expect the opposite. *_ra stands for "return address". r~
Re: [PATCH 05/37] include/exec: Inline *_mmuidx_ra memory operations
On 3/13/25 11:05, Richard Henderson wrote:
On 3/13/25 09:59, Pierrick Bouvier wrote:
+static inline int
+cpu_ldsw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+ int mmu_idx, uintptr_t ra)
+{
+ return (int16_t)cpu_lduw_be_mmuidx_ra(env, addr, mmu_idx, ra);
For my personal culture, is that strictly equivalent to doing the load with
MO_BESW?
If you're asking if it's the same as passing MO_BESW to tcg_gen_qemu_ld_i32(),
yes. The
tcg code generator takes care of making the value sign-extended.
If you're asking if it's the same as passing MO_BESW to cpu_ldw_mmu(), no. The
core
functions only handle unsigned values. This older api contained functions with
a signed
return value, so we preserve that.
That was my question, thanks.
So we need to keep on doing the integral cast instead of calling
cpu_ldw_mmu with MO_BESW.
r~
Re: [PATCH 05/37] include/exec: Inline *_mmuidx_ra memory operations
On 13/3/25 19:05, Richard Henderson wrote:
On 3/13/25 09:59, Pierrick Bouvier wrote:
+static inline int
+cpu_ldsw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+ int mmu_idx, uintptr_t ra)
+{
+ return (int16_t)cpu_lduw_be_mmuidx_ra(env, addr, mmu_idx, ra);
For my personal culture, is that strictly equivalent to doing the load
with MO_BESW?
If you're asking if it's the same as passing MO_BESW to
tcg_gen_qemu_ld_i32(), yes. The tcg code generator takes care of making
the value sign-extended.
If you're asking if it's the same as passing MO_BESW to cpu_ldw_mmu(),
no. The core functions only handle unsigned values. This older api
contained functions with a signed return value, so we preserve that.
Are these 2 APIs doing the same thing? What are the uses? Can we rename
the legacy one?
Re: [PATCH 05/37] include/exec: Inline *_mmuidx_ra memory operations
On 3/13/25 09:59, Pierrick Bouvier wrote:
+static inline int
+cpu_ldsw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+ int mmu_idx, uintptr_t ra)
+{
+ return (int16_t)cpu_lduw_be_mmuidx_ra(env, addr, mmu_idx, ra);
For my personal culture, is that strictly equivalent to doing the load with
MO_BESW?
If you're asking if it's the same as passing MO_BESW to tcg_gen_qemu_ld_i32(), yes. The
tcg code generator takes care of making the value sign-extended.
If you're asking if it's the same as passing MO_BESW to cpu_ldw_mmu(), no. The core
functions only handle unsigned values. This older api contained functions with a signed
return value, so we preserve that.
r~
Re: [PATCH 05/37] include/exec: Inline *_mmuidx_ra memory operations
On 3/12/25 20:44, Richard Henderson wrote: These expand inline to the *_mmu api with trivial massaging of the arguments. Signed-off-by: Richard Henderson --- include/exec/cpu_ldst.h | 163 accel/tcg/ldst_common.c.inc | 118 -- 2 files changed, 129 insertions(+), 152 deletions(-) diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h index 1fbdbe59ae..b33755169e 100644 --- a/include/exec/cpu_ldst.h +++ b/include/exec/cpu_ldst.h @@ -118,41 +118,136 @@ void cpu_stl_le_data_ra(CPUArchState *env, abi_ptr ptr, void cpu_stq_le_data_ra(CPUArchState *env, abi_ptr ptr, uint64_t val, uintptr_t ra); -uint32_t cpu_ldub_mmuidx_ra(CPUArchState *env, abi_ptr ptr, -int mmu_idx, uintptr_t ra); -int cpu_ldsb_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); -uint32_t cpu_lduw_be_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); -int cpu_ldsw_be_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); -uint32_t cpu_ldl_be_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); -uint64_t cpu_ldq_be_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); -uint32_t cpu_lduw_le_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); -int cpu_ldsw_le_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); -uint32_t cpu_ldl_le_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); -uint64_t cpu_ldq_le_mmuidx_ra(CPUArchState *env, abi_ptr ptr, - int mmu_idx, uintptr_t ra); Not related to the change, but the naming _ra is very confusing, since it means the opposite of what it seems. *NO* requirement alignment, when you expect the opposite.
Re: [PATCH 05/37] include/exec: Inline *_mmuidx_ra memory operations
On 3/12/25 20:44, Richard Henderson wrote:
These expand inline to the *_mmu api with trivial
massaging of the arguments.
I hope they feel relaxed after that :).
Signed-off-by: Richard Henderson
---
include/exec/cpu_ldst.h | 163
accel/tcg/ldst_common.c.inc | 118 --
2 files changed, 129 insertions(+), 152 deletions(-)
+
+static inline int
+cpu_ldsw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+ int mmu_idx, uintptr_t ra)
+{
+return (int16_t)cpu_lduw_be_mmuidx_ra(env, addr, mmu_idx, ra);
For my personal culture, is that strictly equivalent to doing the load
with MO_BESW?
Reviewed-by: Pierrick Bouvier
