* [PATCH 1/7] exec: Use TARGET_PAGE_BITS_MIN for TLB flags
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
@ 2019-09-19 23:29 ` Richard Henderson
2019-09-19 23:29 ` [PATCH 2/7] exec: Split out variable page size support to exec-vary.c Richard Henderson
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2019-09-19 23:29 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.maydell
These bits do not need to vary with the actual page size
used by the guest.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index d2d443c4f9..e0c8dc540c 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -317,20 +317,24 @@ CPUArchState *cpu_copy(CPUArchState *env);
#if !defined(CONFIG_USER_ONLY)
-/* Flags stored in the low bits of the TLB virtual address. These are
- * defined so that fast path ram access is all zeros.
+/*
+ * Flags stored in the low bits of the TLB virtual address.
+ * These are defined so that fast path ram access is all zeros.
* The flags all must be between TARGET_PAGE_BITS and
* maximum address alignment bit.
+ *
+ * Use TARGET_PAGE_BITS_MIN so that these bits are constant
+ * when TARGET_PAGE_BITS_VARY is in effect.
*/
/* Zero if TLB entry is valid. */
-#define TLB_INVALID_MASK (1 << (TARGET_PAGE_BITS - 1))
+#define TLB_INVALID_MASK (1 << (TARGET_PAGE_BITS_MIN - 1))
/* Set if TLB entry references a clean RAM page. The iotlb entry will
contain the page physical address. */
-#define TLB_NOTDIRTY (1 << (TARGET_PAGE_BITS - 2))
+#define TLB_NOTDIRTY (1 << (TARGET_PAGE_BITS_MIN - 2))
/* Set if TLB entry is an IO callback. */
-#define TLB_MMIO (1 << (TARGET_PAGE_BITS - 3))
+#define TLB_MMIO (1 << (TARGET_PAGE_BITS_MIN - 3))
/* Set if TLB entry contains a watchpoint. */
-#define TLB_WATCHPOINT (1 << (TARGET_PAGE_BITS - 4))
+#define TLB_WATCHPOINT (1 << (TARGET_PAGE_BITS_MIN - 4))
/* Use this mask to check interception with an alignment mask
* in a TCG backend.
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/7] exec: Split out variable page size support to exec-vary.c
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
2019-09-19 23:29 ` [PATCH 1/7] exec: Use TARGET_PAGE_BITS_MIN for TLB flags Richard Henderson
@ 2019-09-19 23:29 ` Richard Henderson
2019-09-21 8:59 ` Philippe Mathieu-Daudé
2019-09-19 23:29 ` [PATCH 3/7] exec: Use const alias for TARGET_PAGE_BITS_VARY Richard Henderson
` (6 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Richard Henderson @ 2019-09-19 23:29 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.maydell
The next patch will play a trick with "const" that will
confuse the compiler about the uses of target_page_bits
within exec.c. Moving everything to a new file prevents
this confusion.
No functional change so far.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
Makefile.target | 2 +-
include/qemu-common.h | 6 +++++
exec-vary.c | 57 +++++++++++++++++++++++++++++++++++++++++++
exec.c | 34 --------------------------
4 files changed, 64 insertions(+), 35 deletions(-)
create mode 100644 exec-vary.c
diff --git a/Makefile.target b/Makefile.target
index 5e916230c4..ca3d14efe1 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -107,7 +107,7 @@ obj-y += trace/
#########################################################
# cpu emulator library
-obj-y += exec.o
+obj-y += exec.o exec-vary.o
obj-y += accel/
obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o tcg/tcg-op-gvec.o
obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/optimize.o
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 0235cd3b91..3e800c2224 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -74,6 +74,12 @@ void cpu_exec_step_atomic(CPUState *cpu);
*/
bool set_preferred_target_page_bits(int bits);
+/**
+ * finalize_target_page_bits:
+ * Commit the final value set by set_preferred_target_page_bits.
+ */
+void finalize_target_page_bits(void);
+
/**
* Sends a (part of) iovec down a socket, yielding when the socket is full, or
* Receives data into a (part of) iovec from a socket,
diff --git a/exec-vary.c b/exec-vary.c
new file mode 100644
index 0000000000..48c0ab306c
--- /dev/null
+++ b/exec-vary.c
@@ -0,0 +1,57 @@
+/*
+ * Variable page size handling
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "exec/exec-all.h"
+
+#ifdef TARGET_PAGE_BITS_VARY
+int target_page_bits;
+bool target_page_bits_decided;
+#endif
+
+bool set_preferred_target_page_bits(int bits)
+{
+ /*
+ * The target page size is the lowest common denominator for all
+ * the CPUs in the system, so we can only make it smaller, never
+ * larger. And we can't make it smaller once we've committed to
+ * a particular size.
+ */
+#ifdef TARGET_PAGE_BITS_VARY
+ assert(bits >= TARGET_PAGE_BITS_MIN);
+ if (target_page_bits == 0 || target_page_bits > bits) {
+ if (target_page_bits_decided) {
+ return false;
+ }
+ target_page_bits = bits;
+ }
+#endif
+ return true;
+}
+
+void finalize_target_page_bits(void)
+{
+#ifdef TARGET_PAGE_BITS_VARY
+ if (target_page_bits == 0) {
+ target_page_bits = TARGET_PAGE_BITS_MIN;
+ }
+ target_page_bits_decided = true;
+#endif
+}
diff --git a/exec.c b/exec.c
index 8b998974f8..33bd0e36c1 100644
--- a/exec.c
+++ b/exec.c
@@ -92,11 +92,6 @@ MemoryRegion io_mem_rom, io_mem_notdirty;
static MemoryRegion io_mem_unassigned;
#endif
-#ifdef TARGET_PAGE_BITS_VARY
-int target_page_bits;
-bool target_page_bits_decided;
-#endif
-
CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
/* current CPU in the current thread. It is only valid inside
@@ -110,37 +105,8 @@ int use_icount;
uintptr_t qemu_host_page_size;
intptr_t qemu_host_page_mask;
-bool set_preferred_target_page_bits(int bits)
-{
- /* The target page size is the lowest common denominator for all
- * the CPUs in the system, so we can only make it smaller, never
- * larger. And we can't make it smaller once we've committed to
- * a particular size.
- */
-#ifdef TARGET_PAGE_BITS_VARY
- assert(bits >= TARGET_PAGE_BITS_MIN);
- if (target_page_bits == 0 || target_page_bits > bits) {
- if (target_page_bits_decided) {
- return false;
- }
- target_page_bits = bits;
- }
-#endif
- return true;
-}
-
#if !defined(CONFIG_USER_ONLY)
-static void finalize_target_page_bits(void)
-{
-#ifdef TARGET_PAGE_BITS_VARY
- if (target_page_bits == 0) {
- target_page_bits = TARGET_PAGE_BITS_MIN;
- }
- target_page_bits_decided = true;
-#endif
-}
-
typedef struct PhysPageEntry PhysPageEntry;
struct PhysPageEntry {
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/7] exec: Split out variable page size support to exec-vary.c
2019-09-19 23:29 ` [PATCH 2/7] exec: Split out variable page size support to exec-vary.c Richard Henderson
@ 2019-09-21 8:59 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-21 8:59 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: pbonzini, peter.maydell
On 9/20/19 1:29 AM, Richard Henderson wrote:
> The next patch will play a trick with "const" that will
> confuse the compiler about the uses of target_page_bits
> within exec.c. Moving everything to a new file prevents
> this confusion.
>
> No functional change so far.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> Makefile.target | 2 +-
> include/qemu-common.h | 6 +++++
> exec-vary.c | 57 +++++++++++++++++++++++++++++++++++++++++++
> exec.c | 34 --------------------------
> 4 files changed, 64 insertions(+), 35 deletions(-)
> create mode 100644 exec-vary.c
>
> diff --git a/Makefile.target b/Makefile.target
> index 5e916230c4..ca3d14efe1 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -107,7 +107,7 @@ obj-y += trace/
>
> #########################################################
> # cpu emulator library
> -obj-y += exec.o
> +obj-y += exec.o exec-vary.o
> obj-y += accel/
> obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/tcg-op-vec.o tcg/tcg-op-gvec.o
> obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/optimize.o
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 0235cd3b91..3e800c2224 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -74,6 +74,12 @@ void cpu_exec_step_atomic(CPUState *cpu);
> */
> bool set_preferred_target_page_bits(int bits);
>
> +/**
> + * finalize_target_page_bits:
> + * Commit the final value set by set_preferred_target_page_bits.
> + */
> +void finalize_target_page_bits(void);
> +
> /**
> * Sends a (part of) iovec down a socket, yielding when the socket is full, or
> * Receives data into a (part of) iovec from a socket,
> diff --git a/exec-vary.c b/exec-vary.c
> new file mode 100644
> index 0000000000..48c0ab306c
> --- /dev/null
> +++ b/exec-vary.c
> @@ -0,0 +1,57 @@
> +/*
> + * Variable page size handling
> + *
> + * Copyright (c) 2003 Fabrice Bellard
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "exec/exec-all.h"
> +
> +#ifdef TARGET_PAGE_BITS_VARY
> +int target_page_bits;
> +bool target_page_bits_decided;
> +#endif
> +
> +bool set_preferred_target_page_bits(int bits)
> +{
> + /*
> + * The target page size is the lowest common denominator for all
> + * the CPUs in the system, so we can only make it smaller, never
> + * larger. And we can't make it smaller once we've committed to
> + * a particular size.
> + */
> +#ifdef TARGET_PAGE_BITS_VARY
> + assert(bits >= TARGET_PAGE_BITS_MIN);
> + if (target_page_bits == 0 || target_page_bits > bits) {
> + if (target_page_bits_decided) {
> + return false;
> + }
> + target_page_bits = bits;
> + }
> +#endif
> + return true;
> +}
> +
> +void finalize_target_page_bits(void)
> +{
> +#ifdef TARGET_PAGE_BITS_VARY
> + if (target_page_bits == 0) {
> + target_page_bits = TARGET_PAGE_BITS_MIN;
> + }
> + target_page_bits_decided = true;
> +#endif
> +}
> diff --git a/exec.c b/exec.c
> index 8b998974f8..33bd0e36c1 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -92,11 +92,6 @@ MemoryRegion io_mem_rom, io_mem_notdirty;
> static MemoryRegion io_mem_unassigned;
> #endif
>
> -#ifdef TARGET_PAGE_BITS_VARY
> -int target_page_bits;
> -bool target_page_bits_decided;
> -#endif
> -
> CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
>
> /* current CPU in the current thread. It is only valid inside
> @@ -110,37 +105,8 @@ int use_icount;
> uintptr_t qemu_host_page_size;
> intptr_t qemu_host_page_mask;
>
> -bool set_preferred_target_page_bits(int bits)
> -{
> - /* The target page size is the lowest common denominator for all
> - * the CPUs in the system, so we can only make it smaller, never
> - * larger. And we can't make it smaller once we've committed to
> - * a particular size.
> - */
> -#ifdef TARGET_PAGE_BITS_VARY
> - assert(bits >= TARGET_PAGE_BITS_MIN);
> - if (target_page_bits == 0 || target_page_bits > bits) {
> - if (target_page_bits_decided) {
> - return false;
> - }
> - target_page_bits = bits;
> - }
> -#endif
> - return true;
> -}
> -
> #if !defined(CONFIG_USER_ONLY)
>
> -static void finalize_target_page_bits(void)
> -{
> -#ifdef TARGET_PAGE_BITS_VARY
> - if (target_page_bits == 0) {
> - target_page_bits = TARGET_PAGE_BITS_MIN;
> - }
> - target_page_bits_decided = true;
> -#endif
> -}
> -
> typedef struct PhysPageEntry PhysPageEntry;
>
> struct PhysPageEntry {
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/7] exec: Use const alias for TARGET_PAGE_BITS_VARY
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
2019-09-19 23:29 ` [PATCH 1/7] exec: Use TARGET_PAGE_BITS_MIN for TLB flags Richard Henderson
2019-09-19 23:29 ` [PATCH 2/7] exec: Split out variable page size support to exec-vary.c Richard Henderson
@ 2019-09-19 23:29 ` Richard Henderson
2019-09-19 23:29 ` [PATCH 4/7] exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG Richard Henderson
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2019-09-19 23:29 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.maydell
Using a variable that is declared "const" for this tells the
compiler that it may read the value once and assume that it
does not change across function calls.
For target_page_size, this means we have only one assert per
function, and one read of the variable.
This reduces the size of qemu-system-aarch64 by 8k.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 10 +++++----
exec-vary.c | 46 ++++++++++++++++++++++++++++++++++--------
2 files changed, 44 insertions(+), 12 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index e0c8dc540c..a53b761b48 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -210,10 +210,12 @@ static inline void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val
/* page related stuff */
#ifdef TARGET_PAGE_BITS_VARY
-extern bool target_page_bits_decided;
-extern int target_page_bits;
-#define TARGET_PAGE_BITS ({ assert(target_page_bits_decided); \
- target_page_bits; })
+typedef struct {
+ bool decided;
+ int bits;
+} TargetPageBits;
+extern const TargetPageBits target_page;
+#define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
#else
#define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
#endif
diff --git a/exec-vary.c b/exec-vary.c
index 48c0ab306c..67cdf57a9c 100644
--- a/exec-vary.c
+++ b/exec-vary.c
@@ -22,8 +22,38 @@
#include "exec/exec-all.h"
#ifdef TARGET_PAGE_BITS_VARY
-int target_page_bits;
-bool target_page_bits_decided;
+/*
+ * We want to declare the "target_page" variable as const, which tells
+ * the compiler that it can cache any value that it reads across calls.
+ * This avoids multiple assertions and multiple reads within any one user.
+ *
+ * This works because we initialize the target_page data very early, in a
+ * location far removed from the functions that require the final results.
+ *
+ * This also requires that we have a non-constant symbol by which we can
+ * perform the actual initialization, and which forces the data to be
+ * allocated within writable memory. Thus "init_target_page", and we use
+ * that symbol exclusively in the two functions that initialize this value.
+ *
+ * The "target_page" symbol is created as an alias of "init_target_page".
+ */
+static TargetPageBits init_target_page;
+
+/*
+ * Note that this is *not* a redundant decl, this is the definition of
+ * the "target_page" symbol. The syntax for this definition requires
+ * the use of the extern keyword. This seems to be a GCC bug in
+ * either the syntax for the alias attribute or in -Wredundant-decls.
+ *
+ * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91765
+ */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wredundant-decls"
+
+extern const TargetPageBits target_page
+ __attribute__((alias("init_target_page")));
+
+#pragma GCC diagnostic pop
#endif
bool set_preferred_target_page_bits(int bits)
@@ -36,11 +66,11 @@ bool set_preferred_target_page_bits(int bits)
*/
#ifdef TARGET_PAGE_BITS_VARY
assert(bits >= TARGET_PAGE_BITS_MIN);
- if (target_page_bits == 0 || target_page_bits > bits) {
- if (target_page_bits_decided) {
+ if (init_target_page.bits == 0 || init_target_page.bits > bits) {
+ if (init_target_page.decided) {
return false;
}
- target_page_bits = bits;
+ init_target_page.bits = bits;
}
#endif
return true;
@@ -49,9 +79,9 @@ bool set_preferred_target_page_bits(int bits)
void finalize_target_page_bits(void)
{
#ifdef TARGET_PAGE_BITS_VARY
- if (target_page_bits == 0) {
- target_page_bits = TARGET_PAGE_BITS_MIN;
+ if (init_target_page.bits == 0) {
+ init_target_page.bits = TARGET_PAGE_BITS_MIN;
}
- target_page_bits_decided = true;
+ init_target_page.decided = true;
#endif
}
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/7] exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
` (2 preceding siblings ...)
2019-09-19 23:29 ` [PATCH 3/7] exec: Use const alias for TARGET_PAGE_BITS_VARY Richard Henderson
@ 2019-09-19 23:29 ` Richard Henderson
2019-09-21 9:01 ` Philippe Mathieu-Daudé
2019-09-19 23:29 ` [PATCH 5/7] exec: Promote TARGET_PAGE_MASK to target_long Richard Henderson
` (4 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Richard Henderson @ 2019-09-19 23:29 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.maydell
This reduces the size of a release build by about 10k.
Noticably, within the tlb miss helpers.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index a53b761b48..b11ee1f711 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -215,7 +215,11 @@ typedef struct {
int bits;
} TargetPageBits;
extern const TargetPageBits target_page;
-#define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
+# ifdef CONFIG_DEBUG_TCG
+# define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
+# else
+# define TARGET_PAGE_BITS target_page.bits
+# endif
#else
#define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/7] exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG
2019-09-19 23:29 ` [PATCH 4/7] exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG Richard Henderson
@ 2019-09-21 9:01 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-21 9:01 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: pbonzini, peter.maydell
On 9/20/19 1:29 AM, Richard Henderson wrote:
> This reduces the size of a release build by about 10k.
> Noticably, within the tlb miss helpers.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/cpu-all.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index a53b761b48..b11ee1f711 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -215,7 +215,11 @@ typedef struct {
> int bits;
> } TargetPageBits;
> extern const TargetPageBits target_page;
> -#define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
> +# ifdef CONFIG_DEBUG_TCG
> +# define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
> +# else
> +# define TARGET_PAGE_BITS target_page.bits
> +# endif
> #else
> #define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
> #endif
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/7] exec: Promote TARGET_PAGE_MASK to target_long
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
` (3 preceding siblings ...)
2019-09-19 23:29 ` [PATCH 4/7] exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG Richard Henderson
@ 2019-09-19 23:29 ` Richard Henderson
2019-09-19 23:29 ` [PATCH 6/7] exec: Tidy TARGET_PAGE_ALIGN Richard Henderson
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2019-09-19 23:29 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.maydell
There are some uint64_t uses that expect TARGET_PAGE_MASK to
extend for a 32-bit, so this must continue to be a signed type.
Define based on TARGET_PAGE_BITS not TARGET_PAGE_SIZE; this
will make a following patch more clear.
This should not have a functional effect so far.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index b11ee1f711..34d36cebca 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -225,7 +225,7 @@ extern const TargetPageBits target_page;
#endif
#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
-#define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
+#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS)
#define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK)
/* Using intptr_t ensures that qemu_*_page_mask is sign-extended even
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/7] exec: Tidy TARGET_PAGE_ALIGN
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
` (4 preceding siblings ...)
2019-09-19 23:29 ` [PATCH 5/7] exec: Promote TARGET_PAGE_MASK to target_long Richard Henderson
@ 2019-09-19 23:29 ` Richard Henderson
2019-09-19 23:29 ` [PATCH 7/7] exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY Richard Henderson
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2019-09-19 23:29 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.maydell
Use TARGET_PAGE_MASK twice instead of TARGET_PAGE_SIZE once.
This is functionally identical, but will help a following patch.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 34d36cebca..5246770271 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -226,7 +226,8 @@ extern const TargetPageBits target_page;
#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS)
-#define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK)
+#define TARGET_PAGE_ALIGN(addr) \
+ (((addr) + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK)
/* Using intptr_t ensures that qemu_*_page_mask is sign-extended even
* when intptr_t is 32-bit and we are aligning a long long.
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/7] exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
` (5 preceding siblings ...)
2019-09-19 23:29 ` [PATCH 6/7] exec: Tidy TARGET_PAGE_ALIGN Richard Henderson
@ 2019-09-19 23:29 ` Richard Henderson
2019-09-20 7:30 ` [PATCH 0/7] exec: Improve code " Paolo Bonzini
2019-09-21 0:32 ` no-reply
8 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2019-09-19 23:29 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.maydell
This eliminates a set of runtime shifts. It turns out that we
require TARGET_PAGE_MASK more often than TARGET_PAGE_SIZE, so
redefine TARGET_PAGE_SIZE based on TARGET_PAGE_MASK instead of
the other way around.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/exec/cpu-all.h | 8 ++++++--
exec-vary.c | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 5246770271..2db73c7a27 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -213,19 +213,23 @@ static inline void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val
typedef struct {
bool decided;
int bits;
+ target_long mask;
} TargetPageBits;
extern const TargetPageBits target_page;
# ifdef CONFIG_DEBUG_TCG
# define TARGET_PAGE_BITS (assert(target_page.decided), target_page.bits)
+# define TARGET_PAGE_MASK (assert(target_page.decided), target_page.mask)
# else
# define TARGET_PAGE_BITS target_page.bits
+# define TARGET_PAGE_MASK target_page.mask
# endif
+# define TARGET_PAGE_SIZE ((int)-TARGET_PAGE_MASK)
#else
#define TARGET_PAGE_BITS_MIN TARGET_PAGE_BITS
+#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
+#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS)
#endif
-#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
-#define TARGET_PAGE_MASK ((target_long)-1 << TARGET_PAGE_BITS)
#define TARGET_PAGE_ALIGN(addr) \
(((addr) + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK)
diff --git a/exec-vary.c b/exec-vary.c
index 67cdf57a9c..26daf281f2 100644
--- a/exec-vary.c
+++ b/exec-vary.c
@@ -83,5 +83,6 @@ void finalize_target_page_bits(void)
init_target_page.bits = TARGET_PAGE_BITS_MIN;
}
init_target_page.decided = true;
+ init_target_page.mask = (target_long)-1 << init_target_page.bits;
#endif
}
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
` (6 preceding siblings ...)
2019-09-19 23:29 ` [PATCH 7/7] exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY Richard Henderson
@ 2019-09-20 7:30 ` Paolo Bonzini
2019-09-21 0:32 ` no-reply
8 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2019-09-20 7:30 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: peter.maydell
On 20/09/19 01:29, Richard Henderson wrote:
> There's currently a fair amount of overhead in the way we currently
> treat TARGET_PAGE_{BITS,SIZE,MASK} with TARGET_PAGE_BITS_VARY.
>
> We have assertions that TARGET_PAGE_BITS has been finalized. Which
> is fine, but the variable that controls the assertion may be assumed
> to be modified by any function call, which means that we have lots
> of duplicate assertions.
>
> This re-arranges things using a const symbol, which allows the compiler
> to assume that the variable is not modified across calls. In order to
> allow initialization of the variable during startup, use an alias that
> is non-const and controls the allocation into a read-write section.
>
> Remove the assertion for release builds.
>
> Precompute TARGET_PAGE_MASK. This removes a runtime shift and allows
> the variable to be used as a direct memory operand on x86.
>
> Size reductions vs master for qemu-system-aarch64 for various hosts:
>
> PPC64LE:
> debug-tcg: -32264
> release: -44360
> AARCH64:
> debug-tcg: -33304
> relase: -77080
> X86_64:
> debug-tcg: -6685
> relase: -15597
>
>
> r~
>
>
> Richard Henderson (7):
> exec: Use TARGET_PAGE_BITS_MIN for TLB flags
> exec: Split out variable page size support to exec-vary.c
> exec: Use const alias for TARGET_PAGE_BITS_VARY
> exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG
> exec: Promote TARGET_PAGE_MASK to target_long
> exec: Tidy TARGET_PAGE_ALIGN
> exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY
>
> Makefile.target | 2 +-
> include/exec/cpu-all.h | 41 +++++++++++++-------
> include/qemu-common.h | 6 +++
> exec-vary.c | 88 ++++++++++++++++++++++++++++++++++++++++++
> exec.c | 34 ----------------
> 5 files changed, 123 insertions(+), 48 deletions(-)
> create mode 100644 exec-vary.c
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY
2019-09-19 23:29 [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY Richard Henderson
` (7 preceding siblings ...)
2019-09-20 7:30 ` [PATCH 0/7] exec: Improve code " Paolo Bonzini
@ 2019-09-21 0:32 ` no-reply
8 siblings, 0 replies; 12+ messages in thread
From: no-reply @ 2019-09-21 0:32 UTC (permalink / raw)
To: richard.henderson; +Cc: pbonzini, qemu-devel, peter.maydell
Patchew URL: https://patchew.org/QEMU/20190919232952.6382-1-richard.henderson@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20190919232952.6382-1-richard.henderson@linaro.org
Subject: [PATCH 0/7] exec: Improve code for TARGET_PAGE_BITS_VARY
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://github.com/patchew-project/qemu
* [new tag] patchew/20190919232952.6382-1-richard.henderson@linaro.org -> patchew/20190919232952.6382-1-richard.henderson@linaro.org
Switched to a new branch 'test'
02b5da1 exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY
a7aca78 exec: Tidy TARGET_PAGE_ALIGN
daeccb0 exec: Promote TARGET_PAGE_MASK to target_long
265eec7 exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG
583a7ed exec: Use const alias for TARGET_PAGE_BITS_VARY
a2dd788 exec: Split out variable page size support to exec-vary.c
cc49c84 exec: Use TARGET_PAGE_BITS_MIN for TLB flags
=== OUTPUT BEGIN ===
1/7 Checking commit cc49c847ff45 (exec: Use TARGET_PAGE_BITS_MIN for TLB flags)
2/7 Checking commit a2dd788d074f (exec: Split out variable page size support to exec-vary.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32:
new file mode 100644
total: 0 errors, 1 warnings, 125 lines checked
Patch 2/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/7 Checking commit 583a7eda4f9e (exec: Use const alias for TARGET_PAGE_BITS_VARY)
ERROR: externs should be avoided in .c files
#58: FILE: exec-vary.c:53:
+extern const TargetPageBits target_page
total: 1 errors, 0 warnings, 82 lines checked
Patch 3/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/7 Checking commit 265eec7a4f93 (exec: Restrict TARGET_PAGE_BITS_VARY assert to CONFIG_DEBUG_TCG)
5/7 Checking commit daeccb050033 (exec: Promote TARGET_PAGE_MASK to target_long)
6/7 Checking commit a7aca78b21b7 (exec: Tidy TARGET_PAGE_ALIGN)
7/7 Checking commit 02b5da125937 (exec: Cache TARGET_PAGE_MASK for TARGET_PAGE_BITS_VARY)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190919232952.6382-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 12+ messages in thread