| Message ID | 20191023154505.30521-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | exec: Improve code for TARGET_PAGE_BITS_VARY | expand |
On 10/23/19 5:44 PM, Richard Henderson wrote: > From: Wei Yang <richardw.yang@linux.intel.com> > > Use ROUND_UP() to define, which is a little bit easy to read. > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> ^ one is enough ;) > Reviewed-by: Juan Quintela <quintela@redhat.com> > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com> > Message-Id: <20191013021145.16011-2-richardw.yang@linux.intel.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > 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) > > /* same as PROT_xxx */ > #define PAGE_READ 0x0001 >
On 24/10/19 13:52, Philippe Mathieu-Daudé wrote: >> 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) >> /* same as PROT_xxx */ >> #define PAGE_READ 0x0001 >> Isn't this the patch where Richard pointed out that the compiler generates worse code? Paolo
On 10/24/19 8:04 AM, Paolo Bonzini wrote: > On 24/10/19 13:52, Philippe Mathieu-Daudé wrote: >>> 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) >>> /* same as PROT_xxx */ >>> #define PAGE_READ 0x0001 >>> > > Isn't this the patch where Richard pointed out that the compiler > generates worse code? Richard confused ROUND_UP with QEMU_ALIGN_UP. r~
On 24/10/19 16:06, Richard Henderson wrote: > On 10/24/19 8:04 AM, Paolo Bonzini wrote: >> On 24/10/19 13:52, Philippe Mathieu-Daudé wrote: >>>> 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) >>>> /* same as PROT_xxx */ >>>> #define PAGE_READ 0x0001 >>>> >> >> Isn't this the patch where Richard pointed out that the compiler >> generates worse code? > > Richard confused ROUND_UP with QEMU_ALIGN_UP. Uh, those are both really badly named. Especially considering that DIV_ROUND_UP divides the result of QEMU_ALIGN_UP. Should we consider swapping them?!? Paolo
On 10/24/19 10:14 AM, Paolo Bonzini wrote: > On 24/10/19 16:06, Richard Henderson wrote: >> Richard confused ROUND_UP with QEMU_ALIGN_UP. > > Uh, those are both really badly named. Especially considering that > DIV_ROUND_UP divides the result of QEMU_ALIGN_UP. Should we consider > swapping them?!? Perhaps not swapping, but perhaps renaming ROUND_UP to POW2_ROUND_UP or something. I dunno, naming is hard. :-P r~
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) /* same as PROT_xxx */ #define PAGE_READ 0x0001