Re: [PATCH 13/14] target/arm: Implement the CPY* instructions

[Peter Maydell]

On Sat, 9 Sept 2023 at 18:06, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 9/7/23 09:03, Peter Maydell wrote:
> > +void HELPER(cpyp)(CPUARMState *env, uint32_t syndrome, uint32_t wdesc,
> > +                  uint32_t rdesc, uint32_t move)
> > +{
> > +    int rd = mops_destreg(syndrome);
> > +    int rs = mops_srcreg(syndrome);
> > +    int rn = mops_sizereg(syndrome);
> > +    uint32_t rmemidx = FIELD_EX32(rdesc, MTEDESC, MIDX);
> > +    uint32_t wmemidx = FIELD_EX32(wdesc, MTEDESC, MIDX);
> > +    bool forwards = true;
> > +    uintptr_t ra = GETPC();
> > +    uint64_t toaddr = env->xregs[rd];
> > +    uint64_t fromaddr = env->xregs[rs];
> > +    uint64_t copysize = env->xregs[rn];
> > +    uint64_t stagecopysize, step;
> > +
> > +    check_mops_enabled(env, ra);
> > +
> > +    if (copysize > 0x007FFFFFFFFFFFFFULL) {
> > +        copysize = 0x007FFFFFFFFFFFFFULL;
> > +    }
>
> CPYFP does not have the same saturation as CPYP.
>
> Again, you would do better if 'move' was a parameter for an inline, so that 
> the tests can
> be folded away.
>
> > +void HELPER(cpym)(CPUARMState *env, uint32_t syndrome, uint32_t wdesc,
> > +                  uint32_t rdesc, uint32_t move)
> > +{
> > +    /* Main: we choose to copy until less than a page remaining */
> > +    CPUState *cs = env_cpu(env);
> > +    int rd = mops_destreg(syndrome);
> > +    int rs = mops_srcreg(syndrome);
> > +    int rn = mops_sizereg(syndrome);
> > +    uint32_t rmemidx = FIELD_EX32(rdesc, MTEDESC, MIDX);
> > +    uint32_t wmemidx = FIELD_EX32(wdesc, MTEDESC, MIDX);
> > +    uintptr_t ra = GETPC();
> > +    bool forwards;
> > +    uint64_t toaddr, fromaddr, copysize, step;
> > +
> > +    check_mops_enabled(env, ra);
> > +
> > +    /* We choose to NOP out "no data to copy" before consistency checks */
> > +    if (env->xregs[rn] == 0) {
> > +        return;
> > +    }
> > +
> > +    check_mops_wrong_option(env, syndrome, ra);
> > +
> > +    if ((int64_t)env->xregs[rn] < 0) {
> > +        forwards = true;
> > +        toaddr = env->xregs[rd] + env->xregs[rn];
> > +        fromaddr = env->xregs[rs] + env->xregs[rn];
> > +        copysize = -env->xregs[rn];
> > +    } else {
> > +        forwards = false;
> > +        copysize = env->xregs[rn];
> > +        /* This toaddr and fromaddr point to the *last* byte to copy */
> > +        toaddr = env->xregs[rd] + copysize - 1;
> > +        fromaddr = env->xregs[rs] + copysize - 1;
> > +    }
>
> You're passing 'move' but not using it.  I would have expected that here.

Whoops. You can't tell the difference for correct guest code,
because CPYFP will always set up Xn so that it is negative,
but yes, CPYFM and CPYFE should be forwards always.

thanks
-- PMM