diff mbox series

[v4,2/5] target/ppc: used ternary operator when registering MAS

Message ID 20210524135908.47505-3-bruno.larsen@eldorado.org.br (mailing list archive)
State New, archived
Headers show
Series target/ppc: add support to disable-tcg | expand

Commit Message

Bruno Larsen (billionai) May 24, 2021, 1:59 p.m. UTC 
The write calback decision when registering the MAS SPR has been turned
into a ternary operation, rather than an if-then-else block.

Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/cpu_init.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Richard Henderson May 24, 2021, 5:32 p.m. UTC | #1 
On 5/24/21 6:59 AM, Bruno Larsen (billionai) wrote:
> The write calback decision when registering the MAS SPR has been turned
> into a ternary operation, rather than an if-then-else block.
> 
> Signed-off-by: Bruno Larsen (billionai)<bruno.larsen@eldorado.org.br>
> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> ---
>   target/ppc/cpu_init.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)

The commit message here says what, but it doesn't say why.

The important part of the change is making the references to spr_write_generic* 
conditional, via SYS_ARG(), so that the code compiles out for !CONFIG_TCG.

The actual code change is fine:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
David Gibson May 25, 2021, 5:10 a.m. UTC | #2 
On Mon, May 24, 2021 at 10:32:18AM -0700, Richard Henderson wrote:
> On 5/24/21 6:59 AM, Bruno Larsen (billionai) wrote:
> > The write calback decision when registering the MAS SPR has been turned
> > into a ternary operation, rather than an if-then-else block.
> > 
> > Signed-off-by: Bruno Larsen (billionai)<bruno.larsen@eldorado.org.br>
> > Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> > ---
> >   target/ppc/cpu_init.c | 9 +++------
> >   1 file changed, 3 insertions(+), 6 deletions(-)
> 
> The commit message here says what, but it doesn't say why.

Right, and "why" is generally the more important thing to be in a
commit message.

> The important part of the change is making the references to
> spr_write_generic* conditional, via SYS_ARG(), so that the code compiles out
> for !CONFIG_TCG.
> 
> The actual code change is fine:
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
>
diff mbox series

Patch

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index b696469d1a..40719f6480 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -1205,15 +1205,12 @@  static void register_BookE206_sprs(CPUPPCState *env, uint32_t mas_mask,
     /* TLB assist registers */
     /* XXX : not implemented */
     for (i = 0; i < 8; i++) {
-        void (*uea_write)(DisasContext *ctx, int sprn, int gprn) =
-            &spr_write_generic32;
-        if (i == 2 && (mas_mask & (1 << i)) && (env->insns_flags & PPC_64B)) {
-            uea_write = &spr_write_generic;
-        }
         if (mas_mask & (1 << i)) {
             spr_register(env, mas_sprn[i], mas_names[i],
                          SPR_NOACCESS, SPR_NOACCESS,
-                         &spr_read_generic, uea_write,
+                         &spr_read_generic,
+                         (i == 2 && (env->insns_flags & PPC_64B))
+                         ? &spr_write_generic : &spr_write_generic32,
                          0x00000000);
         }
     }