[PATCH v3 12/33] target/ppc/mmu_common.c: Eliminate ret from mmu6xx_get_physical_address()

BALATON Zoltan posted 33 patches 6 months, 3 weeks ago
Maintainers: BALATON Zoltan <balaton@eik.bme.hu>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>
There is a newer version of this series
[PATCH v3 12/33] target/ppc/mmu_common.c: Eliminate ret from mmu6xx_get_physical_address()
Posted by BALATON Zoltan 6 months, 3 weeks ago
Return directly, which is simpler than dragging a return value through
multpile if and else blocks.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/mmu_common.c | 84 +++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 45 deletions(-)

diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index b2f2cee1a8..2b6252f3f9 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -389,7 +389,6 @@ static int mmu6xx_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
     target_ulong vsid, sr, pgidx;
     int ds, target_page_bits;
     bool pr;
-    int ret;
 
     /* First try to find a BAT entry if there are any */
     if (env->nb_BATs && get_bat_6xx_tlb(env, ctx, eaddr, access_type) == 0) {
@@ -422,7 +421,6 @@ static int mmu6xx_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
     qemu_log_mask(CPU_LOG_MMU,
             "pte segment: key=%d ds %d nx %d vsid " TARGET_FMT_lx "\n",
             ctx->key, ds, ctx->nx, vsid);
-    ret = -1;
     if (!ds) {
         /* Check if instruction fetch is allowed, if needed */
         if (type == ACCESS_CODE && ctx->nx) {
@@ -439,51 +437,47 @@ static int mmu6xx_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
         /* Initialize real address with an invalid value */
         ctx->raddr = (hwaddr)-1ULL;
         /* Software TLB search */
-        ret = ppc6xx_tlb_check(env, ctx, eaddr, access_type);
-    } else {
-        qemu_log_mask(CPU_LOG_MMU, "direct store...\n");
-        /* Direct-store segment : absolutely *BUGGY* for now */
-
-        switch (type) {
-        case ACCESS_INT:
-            /* Integer load/store : only access allowed */
-            break;
-        case ACCESS_CODE:
-            /* No code fetch is allowed in direct-store areas */
-            return -4;
-        case ACCESS_FLOAT:
-            /* Floating point load/store */
-            return -4;
-        case ACCESS_RES:
-            /* lwarx, ldarx or srwcx. */
-            return -4;
-        case ACCESS_CACHE:
-            /*
-             * dcba, dcbt, dcbtst, dcbf, dcbi, dcbst, dcbz, or icbi
-             *
-             * Should make the instruction do no-op.  As it already do
-             * no-op, it's quite easy :-)
-             */
-            ctx->raddr = eaddr;
-            return 0;
-        case ACCESS_EXT:
-            /* eciwx or ecowx */
-            return -4;
-        default:
-            qemu_log_mask(CPU_LOG_MMU, "ERROR: instruction should not need "
-                          "address translation\n");
-            return -4;
-        }
-        if ((access_type == MMU_DATA_STORE || ctx->key != 1) &&
-            (access_type == MMU_DATA_LOAD || ctx->key != 0)) {
-            ctx->raddr = eaddr;
-            ret = 2;
-        } else {
-            ret = -2;
-        }
+        return ppc6xx_tlb_check(env, ctx, eaddr, access_type);
     }
 
-    return ret;
+    /* Direct-store segment : absolutely *BUGGY* for now */
+    qemu_log_mask(CPU_LOG_MMU, "direct store...\n");
+    switch (type) {
+    case ACCESS_INT:
+        /* Integer load/store : only access allowed */
+        break;
+    case ACCESS_CODE:
+        /* No code fetch is allowed in direct-store areas */
+        return -4;
+    case ACCESS_FLOAT:
+        /* Floating point load/store */
+        return -4;
+    case ACCESS_RES:
+        /* lwarx, ldarx or srwcx. */
+        return -4;
+    case ACCESS_CACHE:
+        /*
+         * dcba, dcbt, dcbtst, dcbf, dcbi, dcbst, dcbz, or icbi
+         *
+         * Should make the instruction do no-op.  As it already do
+         * no-op, it's quite easy :-)
+         */
+        ctx->raddr = eaddr;
+        return 0;
+    case ACCESS_EXT:
+        /* eciwx or ecowx */
+        return -4;
+    default:
+        qemu_log_mask(CPU_LOG_MMU, "ERROR: instruction should not need address"
+                                   " translation\n");
+        return -4;
+    }
+    if ((access_type == MMU_DATA_STORE || ctx->key != 1) &&
+        (access_type == MMU_DATA_LOAD || ctx->key != 0)) {
+        ctx->raddr = eaddr;
+        return 2;
+    }
+    return -2;
 }
 
 /* Generic TLB check function for embedded PowerPC implementations */
-- 
2.30.9
Re: [PATCH v3 12/33] target/ppc/mmu_common.c: Eliminate ret from mmu6xx_get_physical_address()
Posted by Nicholas Piggin 6 months, 2 weeks ago
On Wed May 8, 2024 at 10:15 AM AEST, BALATON Zoltan wrote:
> Return directly, which is simpler than dragging a return value through
> multpile if and else blocks.

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

>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  target/ppc/mmu_common.c | 84 +++++++++++++++++++----------------------
>  1 file changed, 39 insertions(+), 45 deletions(-)
>
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index b2f2cee1a8..2b6252f3f9 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -389,7 +389,6 @@ static int mmu6xx_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>      target_ulong vsid, sr, pgidx;
>      int ds, target_page_bits;
>      bool pr;
> -    int ret;
>  
>      /* First try to find a BAT entry if there are any */
>      if (env->nb_BATs && get_bat_6xx_tlb(env, ctx, eaddr, access_type) == 0) {
> @@ -422,7 +421,6 @@ static int mmu6xx_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>      qemu_log_mask(CPU_LOG_MMU,
>              "pte segment: key=%d ds %d nx %d vsid " TARGET_FMT_lx "\n",
>              ctx->key, ds, ctx->nx, vsid);
> -    ret = -1;
>      if (!ds) {
>          /* Check if instruction fetch is allowed, if needed */
>          if (type == ACCESS_CODE && ctx->nx) {
> @@ -439,51 +437,47 @@ static int mmu6xx_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>          /* Initialize real address with an invalid value */
>          ctx->raddr = (hwaddr)-1ULL;
>          /* Software TLB search */
> -        ret = ppc6xx_tlb_check(env, ctx, eaddr, access_type);
> -    } else {
> -        qemu_log_mask(CPU_LOG_MMU, "direct store...\n");
> -        /* Direct-store segment : absolutely *BUGGY* for now */
> -
> -        switch (type) {
> -        case ACCESS_INT:
> -            /* Integer load/store : only access allowed */
> -            break;
> -        case ACCESS_CODE:
> -            /* No code fetch is allowed in direct-store areas */
> -            return -4;
> -        case ACCESS_FLOAT:
> -            /* Floating point load/store */
> -            return -4;
> -        case ACCESS_RES:
> -            /* lwarx, ldarx or srwcx. */
> -            return -4;
> -        case ACCESS_CACHE:
> -            /*
> -             * dcba, dcbt, dcbtst, dcbf, dcbi, dcbst, dcbz, or icbi
> -             *
> -             * Should make the instruction do no-op.  As it already do
> -             * no-op, it's quite easy :-)
> -             */
> -            ctx->raddr = eaddr;
> -            return 0;
> -        case ACCESS_EXT:
> -            /* eciwx or ecowx */
> -            return -4;
> -        default:
> -            qemu_log_mask(CPU_LOG_MMU, "ERROR: instruction should not need "
> -                          "address translation\n");
> -            return -4;
> -        }
> -        if ((access_type == MMU_DATA_STORE || ctx->key != 1) &&
> -            (access_type == MMU_DATA_LOAD || ctx->key != 0)) {
> -            ctx->raddr = eaddr;
> -            ret = 2;
> -        } else {
> -            ret = -2;
> -        }
> +        return ppc6xx_tlb_check(env, ctx, eaddr, access_type);
>      }
>  
> -    return ret;
> +    /* Direct-store segment : absolutely *BUGGY* for now */
> +    qemu_log_mask(CPU_LOG_MMU, "direct store...\n");
> +    switch (type) {
> +    case ACCESS_INT:
> +        /* Integer load/store : only access allowed */
> +        break;
> +    case ACCESS_CODE:
> +        /* No code fetch is allowed in direct-store areas */
> +        return -4;
> +    case ACCESS_FLOAT:
> +        /* Floating point load/store */
> +        return -4;
> +    case ACCESS_RES:
> +        /* lwarx, ldarx or srwcx. */
> +        return -4;
> +    case ACCESS_CACHE:
> +        /*
> +         * dcba, dcbt, dcbtst, dcbf, dcbi, dcbst, dcbz, or icbi
> +         *
> +         * Should make the instruction do no-op.  As it already do
> +         * no-op, it's quite easy :-)
> +         */
> +        ctx->raddr = eaddr;
> +        return 0;
> +    case ACCESS_EXT:
> +        /* eciwx or ecowx */
> +        return -4;
> +    default:
> +        qemu_log_mask(CPU_LOG_MMU, "ERROR: instruction should not need address"
> +                                   " translation\n");
> +        return -4;
> +    }
> +    if ((access_type == MMU_DATA_STORE || ctx->key != 1) &&
> +        (access_type == MMU_DATA_LOAD || ctx->key != 0)) {
> +        ctx->raddr = eaddr;
> +        return 2;
> +    }
> +    return -2;
>  }
>  
>  /* Generic TLB check function for embedded PowerPC implementations */