Re: [PATCH 02/23] Added CPU loop function

[Richard Henderson <richard.henderson@linaro.org>]

On 6/22/24 11:49, Warner Losh wrote:
> 
> 
> On Mon, Jun 17, 2024 at 10:24 PM Richard Henderson <richard.henderson@linaro.org 
> <mailto:richard.henderson@linaro.org>> wrote:
> 
>     On 6/17/24 11:57, Ajeet Singh wrote:
>      > +            /*
>      > +             * The carry bit is cleared for no error; set for error.
>      > +             * See arm64/arm64/vm_machdep.c cpu_set_syscall_retval()
>      > +             */
>      > +            pstate = pstate_read(env);
>      > +            if (ret >= 0) {
>      > +                pstate &= ~PSTATE_C;
>      > +                env->xregs[0] = ret;
>      > +            } else if (ret == -TARGET_ERESTART) {
>      > +                env->pc -= 4;
>      > +                break;
>      > +            } else if (ret != -TARGET_EJUSTRETURN) {
>      > +                pstate |= PSTATE_C;
>      > +                env->xregs[0] = -ret;
>      > +            }
>      > +            pstate_write(env, pstate);
> 
>     No need for full pstate read/write:
> 
>           env->CF = {0,1};
> 
> 
> If I understand what you're suggesting, the quoted code can be replaced
> by the following, faster construct:
> 
>              /*
>               * The carry bit is cleared for no error; set for error.
>               * See arm64/arm64/vm_machdep.c cpu_set_syscall_retval()
>               */
>              if (ret >= 0) {
>                  env->CF = 0;
>                  env->xregs[0] = ret;
>              } else if (ret == -TARGET_ERESTART) {
>                  env->pc -= 4;
>                  break;
>              } else if (ret != -TARGET_EJUSTRETURN) {
>                  env->CF = 1;
>                  env->xregs[0] = -ret;
>              }
>              break;
> 
> Is that what you're saying?

Yes.

> 
>      > +            break;
>      > +
>      > +        case EXCP_INTERRUPT:
>      > +            /* Just indicate that signals should be handle ASAP. */
>      > +            break;
>      > +
>      > +        case EXCP_UDEF:
>      > +            force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->pc);
>      > +            break;
>      > +
>      > +
>      > +        case EXCP_PREFETCH_ABORT:
>      > +        case EXCP_DATA_ABORT:
>      > +            /* We should only arrive here with EC in {DATAABORT, INSNABORT}. */
>      > +            ec = syn_get_ec(env->exception.syndrome);
> 
>     Nevermind about my question about syndrome.h vs patch 1.
> 
> 
> Ah, Since we have to re-roll this patch anyway, maybe moving it is a good idea?
> Honestly, I'm good either way.

Least effort is called for.  :-)


r~