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

[Richard Henderson]

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.


r~