From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45241) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1dAz-0000FB-Nv for qemu-devel@nongnu.org; Tue, 01 Nov 2016 13:48:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1dAw-0008FQ-MN for qemu-devel@nongnu.org; Tue, 01 Nov 2016 13:48:45 -0400 Received: from mail-oi0-x241.google.com ([2607:f8b0:4003:c06::241]:35577) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c1dAw-0008FI-Gl for qemu-devel@nongnu.org; Tue, 01 Nov 2016 13:48:42 -0400 Received: by mail-oi0-x241.google.com with SMTP id v84so12025792oie.2 for ; Tue, 01 Nov 2016 10:48:42 -0700 (PDT) Sender: Richard Henderson References: <1477604609-2206-1-git-send-email-laurent@vivier.eu> <1477604609-2206-2-git-send-email-laurent@vivier.eu> From: Richard Henderson Message-ID: Date: Tue, 1 Nov 2016 11:48:36 -0600 MIME-Version: 1.0 In-Reply-To: <1477604609-2206-2-git-send-email-laurent@vivier.eu> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/3] target-m68k: add cmpm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier , qemu-devel@nongnu.org Cc: gerg@uclinux.org, schwab@linux-m68k.org, agraf@suse.de On 10/27/2016 03:43 PM, Laurent Vivier wrote: > +DISAS_INSN(cmpm) > +{ > + int opsize = insn_opsize(insn); > + TCGv tmp = tcg_temp_new(); > + TCGv src, dst, addr; > + > + src = gen_load(s, opsize, AREG(insn, 0), 1); > + /* delay the update after the second gen_load() */ > + tcg_gen_addi_i32(tmp, AREG(insn, 0), opsize_bytes(opsize)); > + > + /* but if the we use the same address register to > + * read the second value, we must use the updated address > + */ > + if (REG(insn, 0) == REG(insn, 9)) { > + addr = tmp; > + } else { > + addr = AREG(insn, 9); > + } > + > + dst = gen_load(s, opsize, addr, 1); > + tcg_gen_mov_i32(AREG(insn, 0), tmp); > + tcg_gen_addi_i32(AREG(insn, 9), addr, opsize_bytes(opsize)); I wonder if we should make this more generic. I'm thinking of transparently fixing case 3: /* Indirect postincrement. */ reg = AREG(insn, 0); result = gen_ldst(s, opsize, reg, val, what); /* ??? This is not exception safe. The instruction may still fault after this point. */ if (what == EA_STORE || !addrp) tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize)); return result; If we add to DisasContext unsigned writeback_mask; TCGv writeback[8]; Then static TCGv get_areg(DisasContext *s, unsigned regno) { if (s->writeback_mask & (1 << regno)) { return s->writeback[regno]; } else { return cpu_aregs[regno]; } } static void delay_set_areg(DisasContext *s, unsigned regno, TCGv val, bool give_temp) { if (s->writeback_mask & (1 << regno)) { tcg_gen_mov_i32(s->writeback[regno], val); if (give_temp) { tcg_temp_free(val); } } else { s->writeback_mask |= 1 << regno; if (give_temp) { s->writeback[regno] = val; } else { TCGv tmp = tcg_temp_new(); tcg_gen_mov_i32(tmp, val); s->writeback[regno] = tmp; } } } static void do_writebacks(DisasContext *s) { unsigned mask = s->writeback_mask; if (mask) { s->writeback_mask = 0; do { unsigned regno = ctz32(mask); tcg_gen_mov_i32(cpu_aregs[regno], s->writeback[regno]); tcg_temp_free(s->writeback[regno]); mask &= mask - 1; } while (mask); } } where get_areg is used within the AREG macro, delay_set_areg is used in those cases where pre- and post-increment are needed, and do_writebacks is invoked at the end of disas_m68k_insn. This all pre-supposes that cmpm is not a special-case within the ISA, that any time one reuses a register after modification one sees the new value. Given the existing code within gen_ea, this would seem to be the case. Thoughts? r~