Re: [PATCH 1/2] cpu: use ROUND_UP() to define xxx_PAGE_ALIGN

[Richard Henderson]

On 10/12/19 10:11 PM, Wei Yang wrote:
> Use ROUND_UP() to define, which is a little bit easy to read.
> 
> Signed-off-by: Wei Yang <address@hidden>
> ---
>  include/exec/cpu-all.h | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index ad9ab85eb3..255bb186ac 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -220,7 +220,7 @@ extern int target_page_bits;
>  
>  #define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
>  #define TARGET_PAGE_MASK ~(TARGET_PAGE_SIZE - 1)
> -#define TARGET_PAGE_ALIGN(addr) (((addr) + TARGET_PAGE_SIZE - 1) & 
> TARGET_PAGE_MASK)
> +#define TARGET_PAGE_ALIGN(addr) ROUND_UP((addr), TARGET_PAGE_SIZE)
>  
>  /* 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.
> @@ -228,9 +228,8 @@ extern int target_page_bits;
>  extern uintptr_t qemu_host_page_size;
>  extern intptr_t qemu_host_page_mask;
>  
> -#define HOST_PAGE_ALIGN(addr) (((addr) + qemu_host_page_size - 1) & 
> qemu_host_page_mask)
> -#define REAL_HOST_PAGE_ALIGN(addr) (((addr) + qemu_real_host_page_size - 1) 
> & \
> -                                    qemu_real_host_page_mask)
> +#define HOST_PAGE_ALIGN(addr) ROUND_UP((addr), qemu_host_page_size)
> +#define REAL_HOST_PAGE_ALIGN(addr) ROUND_UP((addr), qemu_real_host_page_size)


No, please.

(1) The compiler does not know that qemu_*host_page_size is a power of 2, and
will generate a real division at runtime.  The same is true for
TARGET_PAGE_SIZE when TARGET_PAGE_BITS_VARY.

(2) The first hunk conflicts with an in-flight patch of mine:

https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg04526.html


r~