Re: [PATCH v2 03/16] exec/memory_ldst: extract memory_ldst declarations from cpu-all.h

[Pierrick Bouvier <pierrick.bouvier@xxxxxxxxxx>]

On 3/10/25 21:08, Pierrick Bouvier wrote:
> They are now accessible through exec/memory.h instead, and we make sure
> all variants are available for common or target dependent code.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@xxxxxxxxxx>
> ---
>   include/exec/cpu-all.h         | 12 ------------
>   include/exec/memory_ldst.h.inc |  4 ----
>   2 files changed, 16 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index e56c064d46f..0e8205818a4 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -44,18 +44,6 @@
>   
>   #include "exec/hwaddr.h"
>   
> -#define SUFFIX
> -#define ARG1         as
> -#define ARG1_DECL    AddressSpace *as
> -#define TARGET_ENDIANNESS
> -#include "exec/memory_ldst.h.inc"
> -
> -#define SUFFIX       _cached_slow
> -#define ARG1         cache
> -#define ARG1_DECL    MemoryRegionCache *cache
> -#define TARGET_ENDIANNESS
> -#include "exec/memory_ldst.h.inc"
> -
>   static inline void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t 
> val)
>   {
>       address_space_stl_notdirty(as, addr, val,
> diff --git a/include/exec/memory_ldst.h.inc b/include/exec/memory_ldst.h.inc
> index 92ad74e9560..7270235c600 100644
> --- a/include/exec/memory_ldst.h.inc
> +++ b/include/exec/memory_ldst.h.inc
> @@ -19,7 +19,6 @@
>    * License along with this library; if not, see 
> <http://www.gnu.org/licenses/>.
>    */
>   
> -#ifdef TARGET_ENDIANNESS
>   uint16_t glue(address_space_lduw, SUFFIX)(ARG1_DECL,
>       hwaddr addr, MemTxAttrs attrs, MemTxResult *result);
>   uint32_t glue(address_space_ldl, SUFFIX)(ARG1_DECL,
> @@ -34,7 +33,6 @@ void glue(address_space_stl, SUFFIX)(ARG1_DECL,
>       hwaddr addr, uint32_t val, MemTxAttrs attrs, MemTxResult *result);
>   void glue(address_space_stq, SUFFIX)(ARG1_DECL,
>       hwaddr addr, uint64_t val, MemTxAttrs attrs, MemTxResult *result);
> -#else
>   uint8_t glue(address_space_ldub, SUFFIX)(ARG1_DECL,
>       hwaddr addr, MemTxAttrs attrs, MemTxResult *result);
>   uint16_t glue(address_space_lduw_le, SUFFIX)(ARG1_DECL,
> @@ -63,9 +61,7 @@ void glue(address_space_stq_le, SUFFIX)(ARG1_DECL,
>       hwaddr addr, uint64_t val, MemTxAttrs attrs, MemTxResult *result);
>   void glue(address_space_stq_be, SUFFIX)(ARG1_DECL,
>       hwaddr addr, uint64_t val, MemTxAttrs attrs, MemTxResult *result);
> -#endif
>   
>   #undef ARG1_DECL
>   #undef ARG1
>   #undef SUFFIX
> -#undef TARGET_ENDIANNESS
Just to track last Richard answer,
Posted on v1:

On 3/10/25 17:04, Pierrick Bouvier wrote:
>  From what I understand, non endian versions are simply passing 
DEVICE_NATIVE_ENDIAN as a
> parameter for address_space_ldl_internal, which will thus match the 
target endianness.
>
> So what is the risk for common code to call this?

You're right.  I failed to look at the current implementation
to see that it would already work.

Reviewed-by: Richard Henderson <richard.henderson@xxxxxxxxxx>

r~