[Qemu-devel] [PATCH 0/2] cpu_ldst.h: Minor cleanups in ld/st macros

[Qemu-devel] [PATCH 0/2] cpu_ldst.h: Minor cleanups in ld/st macros

[Peter Maydell]

These two patches make some minor cleanups to cpu_ldst.h which I
noticed while I was trying to get my head around our confusing
mess of load and store related functions.


Peter Maydell (2):
  cpu_ldst.h: Remove unused ldul_ macros
  cpu_ldst.h: Collapse laddr() and saddr() into ldst_get_host_addr()

 include/exec/cpu_ldst.h | 46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/2] cpu_ldst.h: Remove unused ldul_ macros

[Peter Maydell]

The five ldul_ macros are not used anywhere and are marked up with an XXX
comment. "ldul" is a non-standard prefix for our family of load instructions:
we don't mark 32-bit accesses for signedness because they return a 32 bit
quantity. So just delete them.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/cpu_ldst.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index e5550e7..4700831 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -151,15 +151,6 @@
 
 #else
 
-/* XXX: find something cleaner.
- * Furthermore, this is false for 64 bits targets
- */
-#define ldul_user       ldl_user
-#define ldul_kernel     ldl_kernel
-#define ldul_hypv       ldl_hypv
-#define ldul_executive  ldl_executive
-#define ldul_supervisor ldl_supervisor
-
 /* The memory helpers for tcg-generated code need tcg_target_long etc.  */
 #include "tcg.h"
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/2] cpu_ldst.h: Remove unused ldul_ macros

[Paolo Bonzini]

On 13/01/2015 19:32, Peter Maydell wrote:
> The five ldul_ macros are not used anywhere and are marked up with an XXX
> comment. "ldul" is a non-standard prefix for our family of load instructions:
> we don't mark 32-bit accesses for signedness because they return a 32 bit
> quantity. So just delete them.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/exec/cpu_ldst.h | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index e5550e7..4700831 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -151,15 +151,6 @@
>  
>  #else
>  
> -/* XXX: find something cleaner.
> - * Furthermore, this is false for 64 bits targets
> - */
> -#define ldul_user       ldl_user
> -#define ldul_kernel     ldl_kernel
> -#define ldul_hypv       ldl_hypv
> -#define ldul_executive  ldl_executive
> -#define ldul_supervisor ldl_supervisor
> -
>  /* The memory helpers for tcg-generated code need tcg_target_long etc.  */
>  #include "tcg.h"
>  
> 

Trivial, even.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

[Qemu-devel] [PATCH 2/2] cpu_ldst.h: Collapse laddr() and saddr() into ldst_get_host_addr()

[Peter Maydell]

The macros laddr() and saddr() are always defined to be identical
to each other. Replace them with a single macro, and give it a
longer name so it's easier to grep the codebase and confirm that
it's only used in this one place: ldst_get_host_addr().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/exec/cpu_ldst.h | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 4700831..0aae9e4 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -53,30 +53,31 @@
     h2g_nocheck(x); \
 })
 
-#define saddr(x) g2h(x)
-#define laddr(x) g2h(x)
+#define ldst_get_host_addr(x) g2h(x)
 
 #else /* !CONFIG_USER_ONLY */
 /* NOTE: we use double casts if pointers and target_ulong have
    different sizes */
-#define saddr(x) (uint8_t *)(intptr_t)(x)
-#define laddr(x) (uint8_t *)(intptr_t)(x)
+#define ldst_get_host_addr(x) (uint8_t *)(intptr_t)(x)
 #endif
 
-#define ldub_raw(p) ldub_p(laddr((p)))
-#define ldsb_raw(p) ldsb_p(laddr((p)))
-#define lduw_raw(p) lduw_p(laddr((p)))
-#define ldsw_raw(p) ldsw_p(laddr((p)))
-#define ldl_raw(p) ldl_p(laddr((p)))
-#define ldq_raw(p) ldq_p(laddr((p)))
-#define ldfl_raw(p) ldfl_p(laddr((p)))
-#define ldfq_raw(p) ldfq_p(laddr((p)))
-#define stb_raw(p, v) stb_p(saddr((p)), v)
-#define stw_raw(p, v) stw_p(saddr((p)), v)
-#define stl_raw(p, v) stl_p(saddr((p)), v)
-#define stq_raw(p, v) stq_p(saddr((p)), v)
-#define stfl_raw(p, v) stfl_p(saddr((p)), v)
-#define stfq_raw(p, v) stfq_p(saddr((p)), v)
+/* Note that ldst_get_host_addr() should not be used anywhere outside
+ * these macros
+ */
+#define ldub_raw(p) ldub_p(ldst_get_host_addr(p))
+#define ldsb_raw(p) ldsb_p(ldst_get_host_addr(p))
+#define lduw_raw(p) lduw_p(ldst_get_host_addr(p))
+#define ldsw_raw(p) ldsw_p(ldst_get_host_addr(p))
+#define ldl_raw(p) ldl_p(ldst_get_host_addr(p))
+#define ldq_raw(p) ldq_p(ldst_get_host_addr(p))
+#define ldfl_raw(p) ldfl_p(ldst_get_host_addr(p))
+#define ldfq_raw(p) ldfq_p(ldst_get_host_addr(p))
+#define stb_raw(p, v) stb_p(ldst_get_host_addr(p), v)
+#define stw_raw(p, v) stw_p(ldst_get_host_addr(p), v)
+#define stl_raw(p, v) stl_p(ldst_get_host_addr(p), v)
+#define stq_raw(p, v) stq_p(ldst_get_host_addr(p), v)
+#define stfl_raw(p, v) stfl_p(ldst_get_host_addr(p), v)
+#define stfq_raw(p, v) stfq_p(ldst_get_host_addr(p), v)
 
 
 #if defined(CONFIG_USER_ONLY)
-- 
1.9.1

Re: [Qemu-devel] [PATCH 2/2] cpu_ldst.h: Collapse laddr() and saddr() into ldst_get_host_addr()

[Peter Maydell]

On 13 January 2015 at 18:32, Peter Maydell <peter.maydell@linaro.org> wrote:
> The macros laddr() and saddr() are always defined to be identical
> to each other. Replace them with a single macro, and give it a
> longer name so it's easier to grep the codebase and confirm that
> it's only used in this one place: ldst_get_host_addr().

Hmm. Actually I think we should be able to drop the _raw() accessors
entirely -- they're kind of nonsensical on softmmu and four unnecessary
extra characters on linux-user. (The only place we use them in softmmu
is in monitor.c where it should be using the ld*_p functions instead.)
I'll put together a patchset...

-- PMM