Re: [PATCH v2 07/28] target/ppc/mmu_common.c: Remove unneeded local variable

[Nicholas Piggin]

On Thu May 2, 2024 at 9:43 AM AEST, BALATON Zoltan wrote:
> In mmubooke_check_tlb() and mmubooke206_check_tlb() we can assign the
> value directly the the destination, no need to have a separate local
> variable for it.
>

For a minute I thought this changed the interface to now update
ctx->prot even if the lookup failed, but it already does that in
some cases so... no issue.

If this was more widely used API we'd rather change it to never
update *prot on failure, but okay we can do this.

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

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  target/ppc/mmu_common.c | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index b0aca8ec02..74c3b814c9 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -627,8 +627,6 @@ static int mmubooke_check_tlb(CPUPPCState *env, 
> ppcemb_tlb_t *tlb,
>                                hwaddr *raddr, int *prot, target_ulong address,
>                                MMUAccessType access_type, int i)
>  {
> -    int prot2;
> -
>      if (!mmubooke_check_pid(env, tlb, raddr, address, i)) {
>          qemu_log_mask(CPU_LOG_MMU, "%s: TLB entry not found\n", __func__);
>          return -1;
> @@ -643,17 +641,16 @@ static int mmubooke_check_tlb(CPUPPCState *env, 
> ppcemb_tlb_t *tlb,
>      }
>  
>      if (FIELD_EX64(env->msr, MSR, PR)) {
> -        prot2 = tlb->prot & 0xF;
> +        *prot = tlb->prot & 0xF;
>      } else {
> -        prot2 = (tlb->prot >> 4) & 0xF;
> +        *prot = (tlb->prot >> 4) & 0xF;
>      }
> -    *prot = prot2;
> -    if (prot2 & prot_for_access_type(access_type)) {
> +    if (*prot & prot_for_access_type(access_type)) {
>          qemu_log_mask(CPU_LOG_MMU, "%s: good TLB!\n", __func__);
>          return 0;
>      }
>  
> -    qemu_log_mask(CPU_LOG_MMU, "%s: no prot match: %x\n", __func__, prot2);
> +    qemu_log_mask(CPU_LOG_MMU, "%s: no prot match: %x\n", __func__, *prot);
>      return access_type == MMU_INST_FETCH ? -3 : -2;
>  }
>  
> @@ -794,7 +791,6 @@ static int mmubooke206_check_tlb(CPUPPCState *env, 
> ppcmas_tlb_t *tlb,
>                                   target_ulong address,
>                                   MMUAccessType access_type, int mmu_idx)
>  {
> -    int prot2 = 0;
>      uint32_t epid;
>      bool as, pr;
>      bool use_epid = mmubooke206_get_as(env, mmu_idx, &epid, &as, &pr);
> @@ -840,34 +836,34 @@ found_tlb:
>          return -1;
>      }
>  
> +    *prot = 0;
>      if (pr) {
>          if (tlb->mas7_3 & MAS3_UR) {
> -            prot2 |= PAGE_READ;
> +            *prot |= PAGE_READ;
>          }
>          if (tlb->mas7_3 & MAS3_UW) {
> -            prot2 |= PAGE_WRITE;
> +            *prot |= PAGE_WRITE;
>          }
>          if (tlb->mas7_3 & MAS3_UX) {
> -            prot2 |= PAGE_EXEC;
> +            *prot |= PAGE_EXEC;
>          }
>      } else {
>          if (tlb->mas7_3 & MAS3_SR) {
> -            prot2 |= PAGE_READ;
> +            *prot |= PAGE_READ;
>          }
>          if (tlb->mas7_3 & MAS3_SW) {
> -            prot2 |= PAGE_WRITE;
> +            *prot |= PAGE_WRITE;
>          }
>          if (tlb->mas7_3 & MAS3_SX) {
> -            prot2 |= PAGE_EXEC;
> +            *prot |= PAGE_EXEC;
>          }
>      }
> -    *prot = prot2;
> -    if (prot2 & prot_for_access_type(access_type)) {
> +    if (*prot & prot_for_access_type(access_type)) {
>          qemu_log_mask(CPU_LOG_MMU, "%s: good TLB!\n", __func__);
>          return 0;
>      }
>  
> -    qemu_log_mask(CPU_LOG_MMU, "%s: no prot match: %x\n", __func__, prot2);
> +    qemu_log_mask(CPU_LOG_MMU, "%s: no prot match: %x\n", __func__, *prot);
>      return access_type == MMU_INST_FETCH ? -3 : -2;
>  }
>