Re: [Qemu-devel] [PATCH v2 2/3] target-m68k: implement 680x0 movem

[Richard Henderson]

On 11/04/2016 01:59 AM, Laurent Vivier wrote:
> Le 03/11/2016 à 21:47, Richard Henderson a écrit :
> > On 11/02/2016 03:15 PM, Laurent Vivier wrote:
> > > +            for (i = 15; i >= 0; i--, mask >>= 1) {
> > > +                if (mask & 1) {
> > > +                    if ((insn & 7) + 8 == i &&
> > > +                        m68k_feature(s->env, M68K_FEATURE_EXT_FULL)) {
> > > +                        /* M68020+: if the addressing register is the
> > > +                         * register moved to memory, the value written
> > > +                         * is the initial value decremented by the
> > > size of
> > > +                         * the operation
> > > +                         * M68000/M68010: the value is the initial value
> > > +                         */
> > > +                        TCGv tmp = tcg_temp_new();
> > > +                        tcg_gen_sub_i32(tmp, mreg(i), incr);
> > > +                        gen_store(s, opsize, addr, tmp);
> > > +                        tcg_temp_free(tmp);
> > > +                    } else {
> > > +                        gen_store(s, opsize, addr, mreg(i));
> > > +                    }
> > > +                    if (mask != 1) {
> > > +                        tcg_gen_sub_i32(addr, addr, incr);
> > > +                    }
> > > +                }
> >
> > One more thing: This is pre-decrement.  Why are you decrementing after
> > the store?  Seems to me this should be
> >
> >    if (mask & 1) {
> >        tcg_gen_sub_i32(addr, addr, incr);
> >        if (REG(insn, 0) + 8 == i ...)
> >        ...
> >    }
>
> Because it has already been decremented by gen_lea()... so this a
> problem if we have page fault, except if we use your "areg writeback"
> series, and we will.

Ah, I see.  No, gen_lea doesn't writeback; only gen_ea does.  But I wonder if 
this couldn't be cleaned up a tad.  I'll get back to you.


r~