Re: [PATCH v3 31/33] target/ppc/mmu-hash32.c: Drop a local variable

[Nicholas Piggin]

On Wed May 8, 2024 at 10:15 AM AEST, BALATON Zoltan wrote:
> In ppc_hash32_xlate() the value of need_prop is checked in two places
> but precalculating it does not help because when we reach the first
> check we always return and not reach the second place so the value
> will only be used once. We can drop the local variable and calculate
> it when needed, which makes these checks using it similar to other
> places with such checks.

Sure, why not.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  target/ppc/mmu-hash32.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> index 960751a50e..b5d7aeed4e 100644
> --- a/target/ppc/mmu-hash32.c
> +++ b/target/ppc/mmu-hash32.c
> @@ -347,7 +347,6 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, 
> MMUAccessType access_type,
>      hwaddr pte_offset;
>      ppc_hash_pte32_t pte;
>      int prot;
> -    int need_prot;
>      hwaddr raddr;
>  
>      /* There are no hash32 large pages. */
> @@ -361,13 +360,11 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, 
> MMUAccessType access_type,
>          return true;
>      }
>  
> -    need_prot = prot_for_access_type(access_type);
> -
>      /* 2. Check Block Address Translation entries (BATs) */
>      if (env->nb_BATs != 0) {
>          raddr = ppc_hash32_bat_lookup(cpu, eaddr, access_type, protp, 
> mmu_idx);
>          if (raddr != -1) {
> -            if (need_prot & ~*protp) {
> +            if (prot_for_access_type(access_type) & ~*protp) {
>                  if (guest_visible) {
>                      if (access_type == MMU_INST_FETCH) {
>                          cs->exception_index = POWERPC_EXCP_ISI;
> @@ -435,7 +432,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, 
> MMUAccessType access_type,
>  
>      prot = ppc_hash32_pte_prot(mmu_idx, sr, pte);
>  
> -    if (need_prot & ~prot) {
> +    if (prot_for_access_type(access_type) & ~prot) {
>          /* Access right violation */
>          qemu_log_mask(CPU_LOG_MMU, "PTE access rejected\n");
>          if (guest_visible) {