Re: [Qemu-devel] [PATCH v2 3/3] target-m68k: add cas/cas2 ops

[Richard Henderson]

On 11/02/2016 03:15 PM, Laurent Vivier wrote:
> +    if (c1 != l1) {
> +        env->cc_n = l1;
> +        env->cc_v = c1;
> +    } else {
> +        env->cc_n = l2;
> +        env->cc_v = c2;
> +    }
> +    env->cc_op = CC_OP_CMPL;
> +    env->dregs[Dc1] = deposit32(env->dregs[Dc1], 0, 16, l1);
> +    env->dregs[Dc2] = deposit32(env->dregs[Dc2], 0, 16, l2);

CC_OP_CMPW for cas2w.

> +void HELPER(cas2l)(CPUM68KState *env, uint32_t regs, uint32_t a1, uint32_t a2)
> +{
> +    uint32_t Dc1 = extract32(regs, 9, 3);
> +    uint32_t Dc2 = extract32(regs, 6, 3);
> +    uint32_t Du1 = extract32(regs, 3, 3);
> +    uint32_t Du2 = extract32(regs, 0, 3);
> +    uint32_t c1 = env->dregs[Dc1];
> +    uint32_t c2 = env->dregs[Dc2];
> +    uint32_t u1 = env->dregs[Du1];
> +    uint32_t u2 = env->dregs[Du2];
> +    uint32_t l1, l2;
> +    uint64_t c, u, l;
> +    uintptr_t ra = GETPC();
> +#ifndef CONFIG_USER_ONLY
> +    int mmu_idx = cpu_mmu_index(env, 0);
> +    TCGMemOpIdx oi;
> +#endif
> +
> +    if (parallel_cpus) {
> +        /* We're executing in a parallel context -- must be atomic.  */
> +        if ((a1 & 7) == 0 && a2 == a1 + 4) {
> +            c = deposit64(c2, 32, 32, c1);
> +            u = deposit64(u2, 32, 32, u1);
> +#ifdef CONFIG_USER_ONLY
> +            uint64_t *ha1 = g2h(a1);
> +            l = atomic_cmpxchg__nocheck(ha1, c, u);
> +#else
> +            oi = make_memop_idx(MO_BEQ, mmu_idx);
> +            l = helper_atomic_cmpxchgq_be_mmu(env, a1, c, u, oi, ra);
> +#endif

We do need a check here for CONFIG_ATOMIC64.  If that's not set, the host 
doesn't have 64-bit cmpxchg.  Probably arrange this as

if (parallel_cpus) {
#ifdef CONFIG_ATOMIC64
    if ((a1 & 7) ...) {
       ...
    } else if ((a2 & 7) ...) {
       ...
    } else
#endif
    {
        /* Tell the main loop we need to serialize this insn.  */
        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
    }
} else {
   ...
}

Which is pretty ugly, but the best we can do without re-organizing the helpers.

> +   regs = tcg_const_i32(REG(ext2, 6) |
> +                        (REG(ext1, 6) << 3) |
> +                        (REG(ext2, 0) << 6) |
> +                        (REG(ext1, 0) << 9));
> +   gen_helper_cas2w(cpu_env, regs, addr1, addr2);
> +   tcg_temp_free(regs);

Need

  /* Note that cas2w also assigned to env->cc_op.  */
  s->cc_op = CC_OP_CMPW;
  s->cc_op_synced = 1;

> +DISAS_INSN(cas2l)
> +{
...
> +   regs = tcg_const_i32(REG(ext2, 6) |
> +                        (REG(ext1, 6) << 3) |
> +                        (REG(ext2, 0) << 6) |
> +                        (REG(ext1, 0) << 9));
> +   gen_helper_cas2w(cpu_env, regs, addr1, addr2);

cas2l.

Also need to set cc_op to CC_OP_CMPL, as above.


r~