Re: [PATCH v3 09/10] target/ppc: simplify var usage in ppc_next_unmasked_interrupt

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

On 9/13/24 18:20, BALATON Zoltan wrote:
> On Fri, 13 Sep 2024, Harsh Prateek Bora wrote:
>> As previously done for arch specific handlers, simplify var usage in
>> ppc_next_unmasked_interrupt by caching the env->pending_interrupts and
>> env->spr[SPR_LPCR] in local vars and using it later at multiple places.
>>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>> target/ppc/excp_helper.c | 54 ++++++++++++++++++++--------------------
>> 1 file changed, 27 insertions(+), 27 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index d0e0f609a0..4eeeedff5b 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -2022,31 +2022,31 @@ static int 
>> p9_next_unmasked_interrupt(CPUPPCState *env,
>>
>> static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>> {
>> +    uint32_t pending_interrupts = env->pending_interrupts;
>> +    target_ulong lpcr = env->spr[SPR_LPCR];
>> +    bool async_deliver;
> 
> Maybe easier to review if split into one patch for each variable added 
> so it's easier to see what's replaced and that nothing is missed.
> 

Thanks for the reviews. I shall split as suggested in v4.

regards,
Harsh

> Regards,
> BALATON Zoltan
> 
>> +
>> #ifdef TARGET_PPC64
>>     switch (env->excp_model) {
>>     case POWERPC_EXCP_POWER7:
>> -        return p7_next_unmasked_interrupt(env, env->pending_interrupts,
>> -                                          env->spr[SPR_LPCR]);
>> +        return p7_next_unmasked_interrupt(env, pending_interrupts, 
>> lpcr);
>>     case POWERPC_EXCP_POWER8:
>> -        return p8_next_unmasked_interrupt(env, env->pending_interrupts,
>> -                                          env->spr[SPR_LPCR]);
>> +        return p8_next_unmasked_interrupt(env, pending_interrupts, 
>> lpcr);
>>     case POWERPC_EXCP_POWER9:
>>     case POWERPC_EXCP_POWER10:
>>     case POWERPC_EXCP_POWER11:
>> -        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
>> -                              env->spr[SPR_LPCR]);
>> +        return p9_next_unmasked_interrupt(env, pending_interrupts, 
>> lpcr);
>>     default:
>>         break;
>>     }
>> #endif
>> -    bool async_deliver;
>>
>>     /* External reset */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
>> +    if (pending_interrupts & PPC_INTERRUPT_RESET) {
>>         return PPC_INTERRUPT_RESET;
>>     }
>>     /* Machine check exception */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
>> +    if (pending_interrupts & PPC_INTERRUPT_MCK) {
>>         return PPC_INTERRUPT_MCK;
>>     }
>> #if 0 /* TODO */
>> @@ -2065,9 +2065,9 @@ static int 
>> ppc_next_unmasked_interrupt(CPUPPCState *env)
>>     async_deliver = FIELD_EX64(env->msr, MSR, EE) || 
>> env->resume_as_sreset;
>>
>>     /* Hypervisor decrementer exception */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
>> +    if (pending_interrupts & PPC_INTERRUPT_HDECR) {
>>         /* LPCR will be clear when not supported so this will work */
>> -        bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
>> +        bool hdice = !!(lpcr & LPCR_HDICE);
>>         if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
>>             /* HDEC clears on delivery */
>>             return PPC_INTERRUPT_HDECR;
>> @@ -2075,18 +2075,18 @@ static int 
>> ppc_next_unmasked_interrupt(CPUPPCState *env)
>>     }
>>
>>     /* Hypervisor virtualization interrupt */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
>> +    if (pending_interrupts & PPC_INTERRUPT_HVIRT) {
>>         /* LPCR will be clear when not supported so this will work */
>> -        bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
>> +        bool hvice = !!(lpcr & LPCR_HVICE);
>>         if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
>>             return PPC_INTERRUPT_HVIRT;
>>         }
>>     }
>>
>>     /* External interrupt can ignore MSR:EE under some circumstances */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
>> -        bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>> -        bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>> +    if (pending_interrupts & PPC_INTERRUPT_EXT) {
>> +        bool lpes0 = !!(lpcr & LPCR_LPES0);
>> +        bool heic = !!(lpcr & LPCR_HEIC);
>>         /* HEIC blocks delivery to the hypervisor */
>>         if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
>>             !FIELD_EX64(env->msr, MSR, PR))) ||
>> @@ -2096,45 +2096,45 @@ static int 
>> ppc_next_unmasked_interrupt(CPUPPCState *env)
>>     }
>>     if (FIELD_EX64(env->msr, MSR, CE)) {
>>         /* External critical interrupt */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
>> +        if (pending_interrupts & PPC_INTERRUPT_CEXT) {
>>             return PPC_INTERRUPT_CEXT;
>>         }
>>     }
>>     if (async_deliver != 0) {
>>         /* Watchdog timer on embedded PowerPC */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
>> +        if (pending_interrupts & PPC_INTERRUPT_WDT) {
>>             return PPC_INTERRUPT_WDT;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>> +        if (pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>>             return PPC_INTERRUPT_CDOORBELL;
>>         }
>>         /* Fixed interval timer on embedded PowerPC */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
>> +        if (pending_interrupts & PPC_INTERRUPT_FIT) {
>>             return PPC_INTERRUPT_FIT;
>>         }
>>         /* Programmable interval timer on embedded PowerPC */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
>> +        if (pending_interrupts & PPC_INTERRUPT_PIT) {
>>             return PPC_INTERRUPT_PIT;
>>         }
>>         /* Decrementer exception */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
>> +        if (pending_interrupts & PPC_INTERRUPT_DECR) {
>>             return PPC_INTERRUPT_DECR;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
>> +        if (pending_interrupts & PPC_INTERRUPT_DOORBELL) {
>>             return PPC_INTERRUPT_DOORBELL;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
>> +        if (pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
>>             return PPC_INTERRUPT_HDOORBELL;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
>> +        if (pending_interrupts & PPC_INTERRUPT_PERFM) {
>>             return PPC_INTERRUPT_PERFM;
>>         }
>>         /* Thermal interrupt */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
>> +        if (pending_interrupts & PPC_INTERRUPT_THERM) {
>>             return PPC_INTERRUPT_THERM;
>>         }
>>         /* EBB exception */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
>> +        if (pending_interrupts & PPC_INTERRUPT_EBB) {
>>             /*
>>              * EBB exception must be taken in problem state and
>>              * with BESCR_GE set.
>>