Re: [PATCH 04/10] target/ppc: Use env_cpu for cpu_abort in excp_helper

[Nicholas Piggin]

On Wed Jun 14, 2023 at 8:13 PM AEST, BALATON Zoltan wrote:
> On Wed, 14 Jun 2023, Nicholas Piggin wrote:
> > On Mon Jun 12, 2023 at 8:42 AM AEST, BALATON Zoltan wrote:
> >> Use the env_cpu function to get the CPUState for cpu_abort. These are
> >> only needed in case of fatal errors so this allows to avoid casting
> >> and storing CPUState in a local variable wnen not needed.
> >
> > I don't entirely mind keeping cs around as a variable...
>
> What for? This was only needed for error messages so most of the time 
> useless and having less locals make the function a bit simpler. If this is 

Declaring cs, cpu, env, etc at the start of the function is very common
though so it doesn't take much thinking. You don't even need to look at
the declaration to know what it is.

Thanks,
Nick

> needed for something else than errors later it could be added back but 
> then it will only be in the model specific function that needs it.
>
> What I could change is to use CPU(cpu) instead of env_cpu(env) now that 
> I've dropped the rest of the series that also changed the func parameter 
> to get env instead of PowerPCCPU which may avoid some line breaks but not 
> sure if that's any better.
>
> Regards,
> BALATON Zoltan
>
> > Thanks,
> > Nick
> >
> >>
> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >> ---
> >>  target/ppc/excp_helper.c | 118 +++++++++++++++++++++------------------
> >>  1 file changed, 63 insertions(+), 55 deletions(-)
> >>
> >> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> >> index e4532f5088..51202f7028 100644
> >> --- a/target/ppc/excp_helper.c
> >> +++ b/target/ppc/excp_helper.c
> >> @@ -422,7 +422,6 @@ static void powerpc_checkstop_state(CPUPPCState *env)
> >>
> >>  static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
> >>  {
> >> -    CPUState *cs = CPU(cpu);
> >>      CPUPPCState *env = &cpu->env;
> >>      target_ulong msr, new_msr, vector;
> >>      int srr0, srr1;
> >
> > [snip]
> >
> >