Richard Henderson <richard.hender...@linaro.org> writes:
> 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.
>
> Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
> ---
> v2: Notice CONFIG_ATTRIBUTE_ALIAS, and work around Xcode 9 lossage.
> ---
> include/exec/cpu-all.h | 14 +++++++---
> exec-vary.c | 60 ++++++++++++++++++++++++++++++++++++------
> 2 files changed, 62 insertions(+), 12 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 255bb186ac..76515dc8d9 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -210,10 +210,16 @@ 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;
> +# if defined(CONFIG_ATTRIBUTE_ALIAS) || !defined(IN_EXEC_VARY)
> +extern const TargetPageBits target_page;
> +#else
> +extern TargetPageBits target_page;
> +# endif
> +#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..e0befd502a 100644
> --- a/exec-vary.c
> +++ b/exec-vary.c
> @@ -19,11 +19,55 @@
>
> #include "qemu/osdep.h"
> #include "qemu-common.h"
> +
> +#define IN_EXEC_VARY 1
> +
> #include "exec/exec-all.h"
>
> #ifdef TARGET_PAGE_BITS_VARY
> -int target_page_bits;
> -bool target_page_bits_decided;
> +# ifdef CONFIG_ATTRIBUTE_ALIAS
> +/*
> + * 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
> +# else
> +/*
> + * When aliases are not supported then we force two different declarations,
> + * by way of suppressing the header declaration with IN_EXEC_VARY.
> + * We assume that on such an old compiler, LTO cannot be used, and so the
> + * compiler cannot not detect the mismatched declarations, and all is well.
> + */
> +TargetPageBits target_page;
> +# define init_target_page target_page
> +# endif
> #endif
>
> bool set_preferred_target_page_bits(int bits)
> @@ -36,11 +80,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 +93,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
> }
--
Alex Bennée
