diff mbox series

[05/37] include/exec: Inline *_mmuidx_ra memory operations

Message ID 20250313034524.3069690-6-richard.henderson@linaro.org
State New
Headers show
Series accel/tcg, codebase: Build once patches | expand

Commit Message

Richard Henderson March 13, 2025, 3:44 a.m. UTC 
These expand inline to the *_mmu api with trivial
massaging of the arguments.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu_ldst.h     | 163 ++++++++++++++++++++++++++++--------
 accel/tcg/ldst_common.c.inc | 118 --------------------------
 2 files changed, 129 insertions(+), 152 deletions(-)

Comments

Pierrick Bouvier March 13, 2025, 4:59 p.m. UTC | #1 
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 <richard.henderson@linaro.org>
> ---
>   include/exec/cpu_ldst.h     | 163 ++++++++++++++++++++++++++++--------
>   accel/tcg/ldst_common.c.inc | 118 --------------------------
>   2 files changed, 129 insertions(+), 152 deletions(-)
> 

<snip>

> +
> +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 <pierrick.bouvier@linaro.org>
Pierrick Bouvier March 13, 2025, 5:11 p.m. UTC | #2 
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 <richard.henderson@linaro.org>
> ---
>   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.
Richard Henderson March 13, 2025, 5:55 p.m. UTC | #3 
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~
Richard Henderson March 13, 2025, 6:05 p.m. UTC | #4 
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~
Philippe Mathieu-Daudé March 13, 2025, 6:26 p.m. UTC | #5 
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?
Pierrick Bouvier March 13, 2025, 8:14 p.m. UTC | #6 
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~
diff mbox series

Patch

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);
+static inline uint32_t
+cpu_ldub_mmuidx_ra(CPUArchState *env, abi_ptr addr, int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
+    return cpu_ldb_mmu(env, addr, oi, ra);
+}
 
-void cpu_stb_mmuidx_ra(CPUArchState *env, abi_ptr ptr, uint32_t val,
-                       int mmu_idx, uintptr_t ra);
-void cpu_stw_be_mmuidx_ra(CPUArchState *env, abi_ptr ptr, uint32_t val,
-                          int mmu_idx, uintptr_t ra);
-void cpu_stl_be_mmuidx_ra(CPUArchState *env, abi_ptr ptr, uint32_t val,
-                          int mmu_idx, uintptr_t ra);
-void cpu_stq_be_mmuidx_ra(CPUArchState *env, abi_ptr ptr, uint64_t val,
-                          int mmu_idx, uintptr_t ra);
-void cpu_stw_le_mmuidx_ra(CPUArchState *env, abi_ptr ptr, uint32_t val,
-                          int mmu_idx, uintptr_t ra);
-void cpu_stl_le_mmuidx_ra(CPUArchState *env, abi_ptr ptr, uint32_t val,
-                          int mmu_idx, uintptr_t ra);
-void cpu_stq_le_mmuidx_ra(CPUArchState *env, abi_ptr ptr, uint64_t val,
-                          int mmu_idx, uintptr_t ra);
+static inline int
+cpu_ldsb_mmuidx_ra(CPUArchState *env, abi_ptr addr, int mmu_idx, uintptr_t ra)
+{
+    return (int8_t)cpu_ldub_mmuidx_ra(env, addr, mmu_idx, ra);
+}
+
+static inline uint32_t
+cpu_lduw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+                      int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_BEUW | MO_UNALN, mmu_idx);
+    return cpu_ldw_mmu(env, addr, oi, ra);
+}
+
+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);
+}
+
+static inline uint32_t
+cpu_ldl_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_BEUL | MO_UNALN, mmu_idx);
+    return cpu_ldl_mmu(env, addr, oi, ra);
+}
+
+static inline uint64_t
+cpu_ldq_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_BEUQ | MO_UNALN, mmu_idx);
+    return cpu_ldq_mmu(env, addr, oi, ra);
+}
+
+static inline uint32_t
+cpu_lduw_le_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+                      int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_LEUW | MO_UNALN, mmu_idx);
+    return cpu_ldw_mmu(env, addr, oi, ra);
+}
+
+static inline int
+cpu_ldsw_le_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+                      int mmu_idx, uintptr_t ra)
+{
+    return (int16_t)cpu_lduw_le_mmuidx_ra(env, addr, mmu_idx, ra);
+}
+
+static inline uint32_t
+cpu_ldl_le_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_LEUL | MO_UNALN, mmu_idx);
+    return cpu_ldl_mmu(env, addr, oi, ra);
+}
+
+static inline uint64_t
+cpu_ldq_le_mmuidx_ra(CPUArchState *env, abi_ptr addr,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_LEUQ | MO_UNALN, mmu_idx);
+    return cpu_ldq_mmu(env, addr, oi, ra);
+}
+
+static inline void
+cpu_stb_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
+                  int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
+    cpu_stb_mmu(env, addr, val, oi, ra);
+}
+
+static inline void
+cpu_stw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_BEUW | MO_UNALN, mmu_idx);
+    cpu_stw_mmu(env, addr, val, oi, ra);
+}
+
+static inline void
+cpu_stl_be_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_BEUL | MO_UNALN, mmu_idx);
+    cpu_stl_mmu(env, addr, val, oi, ra);
+}
+
+static inline void
+cpu_stq_be_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint64_t val,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_BEUQ | MO_UNALN, mmu_idx);
+    cpu_stq_mmu(env, addr, val, oi, ra);
+}
+
+static inline void
+cpu_stw_le_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_LEUW | MO_UNALN, mmu_idx);
+    cpu_stw_mmu(env, addr, val, oi, ra);
+}
+
+static inline void
+cpu_stl_le_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_LEUL | MO_UNALN, mmu_idx);
+    cpu_stl_mmu(env, addr, val, oi, ra);
+}
+
+static inline void
+cpu_stq_le_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint64_t val,
+                     int mmu_idx, uintptr_t ra)
+{
+    MemOpIdx oi = make_memop_idx(MO_LEUQ | MO_UNALN, mmu_idx);
+    cpu_stq_mmu(env, addr, val, oi, ra);
+}
 
 #if TARGET_BIG_ENDIAN
 # define cpu_lduw_data        cpu_lduw_be_data
diff --git a/accel/tcg/ldst_common.c.inc b/accel/tcg/ldst_common.c.inc
index 0447c0bb92..99a56df3fb 100644
--- a/accel/tcg/ldst_common.c.inc
+++ b/accel/tcg/ldst_common.c.inc
@@ -248,124 +248,6 @@  void cpu_st16_mmu(CPUArchState *env, vaddr addr, Int128 val,
  * Wrappers of the above
  */
 
-uint32_t cpu_ldub_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                            int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
-    return cpu_ldb_mmu(env, addr, oi, ra);
-}
-
-int cpu_ldsb_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                       int mmu_idx, uintptr_t ra)
-{
-    return (int8_t)cpu_ldub_mmuidx_ra(env, addr, mmu_idx, ra);
-}
-
-uint32_t cpu_lduw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                               int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_BEUW | MO_UNALN, mmu_idx);
-    return cpu_ldw_mmu(env, addr, oi, ra);
-}
-
-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);
-}
-
-uint32_t cpu_ldl_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                              int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_BEUL | MO_UNALN, mmu_idx);
-    return cpu_ldl_mmu(env, addr, oi, ra);
-}
-
-uint64_t cpu_ldq_be_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                              int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_BEUQ | MO_UNALN, mmu_idx);
-    return cpu_ldq_mmu(env, addr, oi, ra);
-}
-
-uint32_t cpu_lduw_le_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                               int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_LEUW | MO_UNALN, mmu_idx);
-    return cpu_ldw_mmu(env, addr, oi, ra);
-}
-
-int cpu_ldsw_le_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                          int mmu_idx, uintptr_t ra)
-{
-    return (int16_t)cpu_lduw_le_mmuidx_ra(env, addr, mmu_idx, ra);
-}
-
-uint32_t cpu_ldl_le_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                              int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_LEUL | MO_UNALN, mmu_idx);
-    return cpu_ldl_mmu(env, addr, oi, ra);
-}
-
-uint64_t cpu_ldq_le_mmuidx_ra(CPUArchState *env, abi_ptr addr,
-                              int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_LEUQ | MO_UNALN, mmu_idx);
-    return cpu_ldq_mmu(env, addr, oi, ra);
-}
-
-void cpu_stb_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
-                       int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
-    cpu_stb_mmu(env, addr, val, oi, ra);
-}
-
-void cpu_stw_be_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
-                          int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_BEUW | MO_UNALN, mmu_idx);
-    cpu_stw_mmu(env, addr, val, oi, ra);
-}
-
-void cpu_stl_be_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
-                          int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_BEUL | MO_UNALN, mmu_idx);
-    cpu_stl_mmu(env, addr, val, oi, ra);
-}
-
-void cpu_stq_be_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint64_t val,
-                          int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_BEUQ | MO_UNALN, mmu_idx);
-    cpu_stq_mmu(env, addr, val, oi, ra);
-}
-
-void cpu_stw_le_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
-                          int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_LEUW | MO_UNALN, mmu_idx);
-    cpu_stw_mmu(env, addr, val, oi, ra);
-}
-
-void cpu_stl_le_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint32_t val,
-                          int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_LEUL | MO_UNALN, mmu_idx);
-    cpu_stl_mmu(env, addr, val, oi, ra);
-}
-
-void cpu_stq_le_mmuidx_ra(CPUArchState *env, abi_ptr addr, uint64_t val,
-                          int mmu_idx, uintptr_t ra)
-{
-    MemOpIdx oi = make_memop_idx(MO_LEUQ | MO_UNALN, mmu_idx);
-    cpu_stq_mmu(env, addr, val, oi, ra);
-}
-
-/*--------------------------*/
-
 uint32_t cpu_ldub_data_ra(CPUArchState *env, abi_ptr addr, uintptr_t ra)
 {
     int mmu_index = cpu_mmu_index(env_cpu(env), false);