Re: [Qemu-devel] [PATCH 39/52] target-m68k: movem

[Richard Henderson]

A more verbose commit message is required here.

On 05/04/2016 11:08 AM, Laurent Vivier wrote:
> @@ -1724,21 +1726,40 @@ DISAS_INSN(movem)
>      addr = tcg_temp_new();
>      tcg_gen_mov_i32(addr, tmp);
>      is_load = ((insn & 0x0400) != 0);
> -    for (i = 0; i < 16; i++, mask >>= 1) {
> -        if (mask & 1) {
> -            if (i < 8)
> -                reg = DREG(i, 0);
> -            else
> -                reg = AREG(i, 0);
> -            if (is_load) {
> -                tmp = gen_load(s, OS_LONG, addr, 0);
> -                tcg_gen_mov_i32(reg, tmp);
> -            } else {
> -                gen_store(s, OS_LONG, addr, reg);
> -            }
> -            if (mask != 1)
> -                tcg_gen_addi_i32(addr, addr, 4);
> -        }
> +    opsize = (insn & 0x40) != 0 ? OS_LONG : OS_WORD;
> +    incr = opsize_bytes(opsize);
> +    if (!is_load && (insn & 070) == 040) {
> +       for (i = 15; i >= 0; i--, mask >>= 1) {
> +           if (mask & 1) {
> +               if (i < 8)
> +                   reg = DREG(i, 0);
> +               else
> +                   reg = AREG(i, 0);
> +               gen_store(s, opsize, addr, reg);
> +               if (mask != 1)
> +                   tcg_gen_subi_i32(addr, addr, incr);
> +           }
> +       }
> +       tcg_gen_mov_i32(AREG(insn, 0), addr);

Missing this bit from the manual:

For the MC68020, MC68030, MC68040, and CPU32, if the addressing register is 
also moved to memory, the value written is the initial register value 
decremented by the size of the operation. The MC68000 and MC68010 write the 
initial register value (not decremented).

You appear to be implementing only the latter.


> +    } else {
> +       for (i = 0; i < 16; i++, mask >>= 1) {
> +           if (mask & 1) {
> +               if (i < 8)
> +                   reg = DREG(i, 0);
> +               else
> +                   reg = AREG(i, 0);
> +               if (is_load) {
> +                   tmp = gen_load(s, opsize, addr, 1);
> +                   tcg_gen_mov_i32(reg, tmp);
> +               } else {
> +                   gen_store(s, opsize, addr, reg);
> +               }
> +               if (mask != 1 || (insn & 070) == 030)
> +                   tcg_gen_addi_i32(addr, addr, incr);
> +           }
> +       }

For loads, we surely should be doing something more in order to properly 
emulate the access trap that might occur here.  I seem to recall saving the 
effective address, a CM bit in the exception stack frame, and RTE restarting 
the movem with the saved effective address.


r~