[PATCH v2 09/42] include/exec: Inline *_data memory operations

Richard Henderson posted 42 patches 2 weeks, 1 day ago
[PATCH v2 09/42] include/exec: Inline *_data memory operations
Posted by Richard Henderson 2 weeks, 1 day ago
These need to be per-target for 'abi_ptr'.  Expand inline to
the *_data_ra api with ra == 0.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu_ldst.h     | 123 ++++++++++++++++++++++++++++++------
 accel/tcg/ldst_common.c.inc |  89 --------------------------
 2 files changed, 104 insertions(+), 108 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index a2a90c7554..d084da0b5f 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -74,25 +74,6 @@
 #include "user/guest-host.h"
 #endif /* CONFIG_USER_ONLY */
 
-uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr ptr);
-int cpu_ldsb_data(CPUArchState *env, abi_ptr ptr);
-uint32_t cpu_lduw_be_data(CPUArchState *env, abi_ptr ptr);
-int cpu_ldsw_be_data(CPUArchState *env, abi_ptr ptr);
-uint32_t cpu_ldl_be_data(CPUArchState *env, abi_ptr ptr);
-uint64_t cpu_ldq_be_data(CPUArchState *env, abi_ptr ptr);
-uint32_t cpu_lduw_le_data(CPUArchState *env, abi_ptr ptr);
-int cpu_ldsw_le_data(CPUArchState *env, abi_ptr ptr);
-uint32_t cpu_ldl_le_data(CPUArchState *env, abi_ptr ptr);
-uint64_t cpu_ldq_le_data(CPUArchState *env, abi_ptr ptr);
-
-void cpu_stb_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
-void cpu_stw_be_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
-void cpu_stl_be_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
-void cpu_stq_be_data(CPUArchState *env, abi_ptr ptr, uint64_t val);
-void cpu_stw_le_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
-void cpu_stl_le_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
-void cpu_stq_le_data(CPUArchState *env, abi_ptr ptr, uint64_t val);
-
 static inline uint32_t
 cpu_ldub_mmuidx_ra(CPUArchState *env, abi_ptr addr, int mmu_idx, uintptr_t ra)
 {
@@ -342,6 +323,110 @@ cpu_stq_le_data_ra(CPUArchState *env, abi_ptr addr, uint64_t val, uintptr_t ra)
     cpu_stq_le_mmuidx_ra(env, addr, val, mmu_index, ra);
 }
 
+/*--------------------------*/
+
+static inline uint32_t
+cpu_ldub_data(CPUArchState *env, abi_ptr addr)
+{
+    return cpu_ldub_data_ra(env, addr, 0);
+}
+
+static inline int
+cpu_ldsb_data(CPUArchState *env, abi_ptr addr)
+{
+    return (int8_t)cpu_ldub_data(env, addr);
+}
+
+static inline uint32_t
+cpu_lduw_be_data(CPUArchState *env, abi_ptr addr)
+{
+    return cpu_lduw_be_data_ra(env, addr, 0);
+}
+
+static inline int
+cpu_ldsw_be_data(CPUArchState *env, abi_ptr addr)
+{
+    return (int16_t)cpu_lduw_be_data(env, addr);
+}
+
+static inline uint32_t
+cpu_ldl_be_data(CPUArchState *env, abi_ptr addr)
+{
+    return cpu_ldl_be_data_ra(env, addr, 0);
+}
+
+static inline uint64_t
+cpu_ldq_be_data(CPUArchState *env, abi_ptr addr)
+{
+    return cpu_ldq_be_data_ra(env, addr, 0);
+}
+
+static inline uint32_t
+cpu_lduw_le_data(CPUArchState *env, abi_ptr addr)
+{
+    return cpu_lduw_le_data_ra(env, addr, 0);
+}
+
+static inline int
+cpu_ldsw_le_data(CPUArchState *env, abi_ptr addr)
+{
+    return (int16_t)cpu_lduw_le_data(env, addr);
+}
+
+static inline uint32_t
+cpu_ldl_le_data(CPUArchState *env, abi_ptr addr)
+{
+    return cpu_ldl_le_data_ra(env, addr, 0);
+}
+
+static inline uint64_t
+cpu_ldq_le_data(CPUArchState *env, abi_ptr addr)
+{
+    return cpu_ldq_le_data_ra(env, addr, 0);
+}
+
+static inline void
+cpu_stb_data(CPUArchState *env, abi_ptr addr, uint32_t val)
+{
+    cpu_stb_data_ra(env, addr, val, 0);
+}
+
+static inline void
+cpu_stw_be_data(CPUArchState *env, abi_ptr addr, uint32_t val)
+{
+    cpu_stw_be_data_ra(env, addr, val, 0);
+}
+
+static inline void
+cpu_stl_be_data(CPUArchState *env, abi_ptr addr, uint32_t val)
+{
+    cpu_stl_be_data_ra(env, addr, val, 0);
+}
+
+static inline void
+cpu_stq_be_data(CPUArchState *env, abi_ptr addr, uint64_t val)
+{
+    cpu_stq_be_data_ra(env, addr, val, 0);
+}
+
+static inline void
+cpu_stw_le_data(CPUArchState *env, abi_ptr addr, uint32_t val)
+{
+    cpu_stw_le_data_ra(env, addr, val, 0);
+}
+
+static inline void
+cpu_stl_le_data(CPUArchState *env, abi_ptr addr, uint32_t val)
+{
+    cpu_stl_le_data_ra(env, addr, val, 0);
+}
+
+static inline void
+cpu_stq_le_data(CPUArchState *env, abi_ptr addr, uint64_t val)
+{
+    cpu_stq_le_data_ra(env, addr, val, 0);
+}
+
 #if TARGET_BIG_ENDIAN
 # define cpu_lduw_data        cpu_lduw_be_data
 # define cpu_ldsw_data        cpu_ldsw_be_data
diff --git a/accel/tcg/ldst_common.c.inc b/accel/tcg/ldst_common.c.inc
index 2f203290db..9791a4e9ef 100644
--- a/accel/tcg/ldst_common.c.inc
+++ b/accel/tcg/ldst_common.c.inc
@@ -243,92 +243,3 @@ void cpu_st16_mmu(CPUArchState *env, vaddr addr, Int128 val,
     do_st16_mmu(env_cpu(env), addr, val, oi, retaddr);
     plugin_store_cb(env, addr, int128_getlo(val), int128_gethi(val), oi);
 }
-
-/*
- * Wrappers of the above
- */
-
-uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr addr)
-{
-    return cpu_ldub_data_ra(env, addr, 0);
-}
-
-int cpu_ldsb_data(CPUArchState *env, abi_ptr addr)
-{
-    return (int8_t)cpu_ldub_data(env, addr);
-}
-
-uint32_t cpu_lduw_be_data(CPUArchState *env, abi_ptr addr)
-{
-    return cpu_lduw_be_data_ra(env, addr, 0);
-}
-
-int cpu_ldsw_be_data(CPUArchState *env, abi_ptr addr)
-{
-    return (int16_t)cpu_lduw_be_data(env, addr);
-}
-
-uint32_t cpu_ldl_be_data(CPUArchState *env, abi_ptr addr)
-{
-    return cpu_ldl_be_data_ra(env, addr, 0);
-}
-
-uint64_t cpu_ldq_be_data(CPUArchState *env, abi_ptr addr)
-{
-    return cpu_ldq_be_data_ra(env, addr, 0);
-}
-
-uint32_t cpu_lduw_le_data(CPUArchState *env, abi_ptr addr)
-{
-    return cpu_lduw_le_data_ra(env, addr, 0);
-}
-
-int cpu_ldsw_le_data(CPUArchState *env, abi_ptr addr)
-{
-    return (int16_t)cpu_lduw_le_data(env, addr);
-}
-
-uint32_t cpu_ldl_le_data(CPUArchState *env, abi_ptr addr)
-{
-    return cpu_ldl_le_data_ra(env, addr, 0);
-}
-
-uint64_t cpu_ldq_le_data(CPUArchState *env, abi_ptr addr)
-{
-    return cpu_ldq_le_data_ra(env, addr, 0);
-}
-
-void cpu_stb_data(CPUArchState *env, abi_ptr addr, uint32_t val)
-{
-    cpu_stb_data_ra(env, addr, val, 0);
-}
-
-void cpu_stw_be_data(CPUArchState *env, abi_ptr addr, uint32_t val)
-{
-    cpu_stw_be_data_ra(env, addr, val, 0);
-}
-
-void cpu_stl_be_data(CPUArchState *env, abi_ptr addr, uint32_t val)
-{
-    cpu_stl_be_data_ra(env, addr, val, 0);
-}
-
-void cpu_stq_be_data(CPUArchState *env, abi_ptr addr, uint64_t val)
-{
-    cpu_stq_be_data_ra(env, addr, val, 0);
-}
-
-void cpu_stw_le_data(CPUArchState *env, abi_ptr addr, uint32_t val)
-{
-    cpu_stw_le_data_ra(env, addr, val, 0);
-}
-
-void cpu_stl_le_data(CPUArchState *env, abi_ptr addr, uint32_t val)
-{
-    cpu_stl_le_data_ra(env, addr, val, 0);
-}
-
-void cpu_stq_le_data(CPUArchState *env, abi_ptr addr, uint64_t val)
-{
-    cpu_stq_le_data_ra(env, addr, val, 0);
-}
-- 
2.43.0
Re: [PATCH v2 09/42] include/exec: Inline *_data memory operations
Posted by Philippe Mathieu-Daudé 2 days, 3 hours ago
On 18/3/25 22:31, Richard Henderson wrote:
> These need to be per-target for 'abi_ptr'.  Expand inline to
> the *_data_ra api with ra == 0.
> 
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/cpu_ldst.h     | 123 ++++++++++++++++++++++++++++++------
>   accel/tcg/ldst_common.c.inc |  89 --------------------------
>   2 files changed, 104 insertions(+), 108 deletions(-)
> 
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index a2a90c7554..d084da0b5f 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -74,25 +74,6 @@
>   #include "user/guest-host.h"
>   #endif /* CONFIG_USER_ONLY */
>   
> -uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr ptr);
> -int cpu_ldsb_data(CPUArchState *env, abi_ptr ptr);
> -uint32_t cpu_lduw_be_data(CPUArchState *env, abi_ptr ptr);
> -int cpu_ldsw_be_data(CPUArchState *env, abi_ptr ptr);
> -uint32_t cpu_ldl_be_data(CPUArchState *env, abi_ptr ptr);
> -uint64_t cpu_ldq_be_data(CPUArchState *env, abi_ptr ptr);
> -uint32_t cpu_lduw_le_data(CPUArchState *env, abi_ptr ptr);
> -int cpu_ldsw_le_data(CPUArchState *env, abi_ptr ptr);
> -uint32_t cpu_ldl_le_data(CPUArchState *env, abi_ptr ptr);
> -uint64_t cpu_ldq_le_data(CPUArchState *env, abi_ptr ptr);
> -
> -void cpu_stb_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
> -void cpu_stw_be_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
> -void cpu_stl_be_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
> -void cpu_stq_be_data(CPUArchState *env, abi_ptr ptr, uint64_t val);
> -void cpu_stw_le_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
> -void cpu_stl_le_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
> -void cpu_stq_le_data(CPUArchState *env, abi_ptr ptr, uint64_t val);
> -
>   static inline uint32_t
>   cpu_ldub_mmuidx_ra(CPUArchState *env, abi_ptr addr, int mmu_idx, uintptr_t ra)
>   {
> @@ -342,6 +323,110 @@ cpu_stq_le_data_ra(CPUArchState *env, abi_ptr addr, uint64_t val, uintptr_t ra)
>       cpu_stq_le_mmuidx_ra(env, addr, val, mmu_index, ra);
>   }
>   
> +/*--------------------------*/
> +
> +static inline uint32_t

Pre-existing, but why don't we use 'unsigned int' like for the signed
variants, or the plain int8_t type?

> +cpu_ldub_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return cpu_ldub_data_ra(env, addr, 0);
> +}
> +
> +static inline int
> +cpu_ldsb_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return (int8_t)cpu_ldub_data(env, addr);
> +}
> +
> +static inline uint32_t
> +cpu_lduw_be_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return cpu_lduw_be_data_ra(env, addr, 0);
> +}
> +
> +static inline int
> +cpu_ldsw_be_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return (int16_t)cpu_lduw_be_data(env, addr);
> +}
> +
> +static inline uint32_t
> +cpu_ldl_be_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return cpu_ldl_be_data_ra(env, addr, 0);
> +}
> +
> +static inline uint64_t
> +cpu_ldq_be_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return cpu_ldq_be_data_ra(env, addr, 0);
> +}
> +
> +static inline uint32_t
> +cpu_lduw_le_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return cpu_lduw_le_data_ra(env, addr, 0);
> +}
> +
> +static inline int
> +cpu_ldsw_le_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return (int16_t)cpu_lduw_le_data(env, addr);
> +}
> +
> +static inline uint32_t
> +cpu_ldl_le_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return cpu_ldl_le_data_ra(env, addr, 0);
> +}
> +
> +static inline uint64_t
> +cpu_ldq_le_data(CPUArchState *env, abi_ptr addr)
> +{
> +    return cpu_ldq_le_data_ra(env, addr, 0);
> +}
> +
> +static inline void
> +cpu_stb_data(CPUArchState *env, abi_ptr addr, uint32_t val)
> +{
> +    cpu_stb_data_ra(env, addr, val, 0);
> +}
> +
> +static inline void
> +cpu_stw_be_data(CPUArchState *env, abi_ptr addr, uint32_t val)
> +{
> +    cpu_stw_be_data_ra(env, addr, val, 0);
> +}
> +
> +static inline void
> +cpu_stl_be_data(CPUArchState *env, abi_ptr addr, uint32_t val)
> +{
> +    cpu_stl_be_data_ra(env, addr, val, 0);
> +}
> +
> +static inline void
> +cpu_stq_be_data(CPUArchState *env, abi_ptr addr, uint64_t val)
> +{
> +    cpu_stq_be_data_ra(env, addr, val, 0);
> +}
> +
> +static inline void
> +cpu_stw_le_data(CPUArchState *env, abi_ptr addr, uint32_t val)
> +{
> +    cpu_stw_le_data_ra(env, addr, val, 0);
> +}
> +
> +static inline void
> +cpu_stl_le_data(CPUArchState *env, abi_ptr addr, uint32_t val)
> +{
> +    cpu_stl_le_data_ra(env, addr, val, 0);
> +}
> +
> +static inline void
> +cpu_stq_le_data(CPUArchState *env, abi_ptr addr, uint64_t val)
> +{
> +    cpu_stq_le_data_ra(env, addr, val, 0);
> +}
Re: [PATCH v2 09/42] include/exec: Inline *_data memory operations
Posted by Richard Henderson 1 day, 16 hours ago
On 4/1/25 01:24, Philippe Mathieu-Daudé wrote:
> On 18/3/25 22:31, Richard Henderson wrote:
>> These need to be per-target for 'abi_ptr'.  Expand inline to
>> the *_data_ra api with ra == 0.
>>
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/exec/cpu_ldst.h     | 123 ++++++++++++++++++++++++++++++------
>>   accel/tcg/ldst_common.c.inc |  89 --------------------------
>>   2 files changed, 104 insertions(+), 108 deletions(-)
>>
>> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
>> index a2a90c7554..d084da0b5f 100644
>> --- a/include/exec/cpu_ldst.h
>> +++ b/include/exec/cpu_ldst.h
>> @@ -74,25 +74,6 @@
>>   #include "user/guest-host.h"
>>   #endif /* CONFIG_USER_ONLY */
>> -uint32_t cpu_ldub_data(CPUArchState *env, abi_ptr ptr);
>> -int cpu_ldsb_data(CPUArchState *env, abi_ptr ptr);
>> -uint32_t cpu_lduw_be_data(CPUArchState *env, abi_ptr ptr);
>> -int cpu_ldsw_be_data(CPUArchState *env, abi_ptr ptr);
>> -uint32_t cpu_ldl_be_data(CPUArchState *env, abi_ptr ptr);
>> -uint64_t cpu_ldq_be_data(CPUArchState *env, abi_ptr ptr);
>> -uint32_t cpu_lduw_le_data(CPUArchState *env, abi_ptr ptr);
>> -int cpu_ldsw_le_data(CPUArchState *env, abi_ptr ptr);
>> -uint32_t cpu_ldl_le_data(CPUArchState *env, abi_ptr ptr);
>> -uint64_t cpu_ldq_le_data(CPUArchState *env, abi_ptr ptr);
>> -
>> -void cpu_stb_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
>> -void cpu_stw_be_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
>> -void cpu_stl_be_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
>> -void cpu_stq_be_data(CPUArchState *env, abi_ptr ptr, uint64_t val);
>> -void cpu_stw_le_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
>> -void cpu_stl_le_data(CPUArchState *env, abi_ptr ptr, uint32_t val);
>> -void cpu_stq_le_data(CPUArchState *env, abi_ptr ptr, uint64_t val);
>> -
>>   static inline uint32_t
>>   cpu_ldub_mmuidx_ra(CPUArchState *env, abi_ptr addr, int mmu_idx, uintptr_t ra)
>>   {
>> @@ -342,6 +323,110 @@ cpu_stq_le_data_ra(CPUArchState *env, abi_ptr addr, uint64_t val, 
>> uintptr_t ra)
>>       cpu_stq_le_mmuidx_ra(env, addr, val, mmu_index, ra);
>>   }
>> +/*--------------------------*/
>> +
>> +static inline uint32_t
> 
> Pre-existing, but why don't we use 'unsigned int' like for the signed
> variants, or the plain int8_t type?

It's the original type used by the api.

You'd never change this.  If you want to audit all of the uses vs the api change, you'd 
update to one of the newer function groups.


r~