From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50631) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsFkr-00050V-L8 for qemu-devel@nongnu.org; Tue, 12 May 2015 15:22:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsFkm-0001KT-Lh for qemu-devel@nongnu.org; Tue, 12 May 2015 15:22:13 -0400 Received: from cantor2.suse.de ([195.135.220.15]:34044 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsFkm-0001KC-G5 for qemu-devel@nongnu.org; Tue, 12 May 2015 15:22:08 -0400 Message-ID: <555252DD.2040102@suse.de> Date: Tue, 12 May 2015 21:22:05 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1431047528-81504-1-git-send-email-agraf@suse.de> <1431047528-81504-2-git-send-email-agraf@suse.de> <55517954.8010308@twiddle.net> In-Reply-To: <55517954.8010308@twiddle.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] s390x: Add laa and laag instructions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , qemu-devel@nongnu.org On 05/12/2015 05:53 AM, Richard Henderson wrote: > On 05/07/2015 06:12 PM, Alexander Graf wrote: >> +static ExitStatus op_laa(DisasContext *s, DisasOps *o) >> +{ >> + TCGv_i64 m2 = tcg_temp_new_i64(); >> + >> + /* XXX should be atomic */ >> + tcg_gen_qemu_ld32s(m2, o->in2, get_mem_index(s)); >> + >> + /* Set r1 to the unmodified contents of m2 */ >> + tcg_gen_mov_i64(o->out, m2); >> + >> + /* m2 = r3 + m2 */ >> + tcg_gen_add_i64(m2, o->in1, m2); >> + tcg_gen_qemu_st32(m2, o->in2, get_mem_index(s)); >> + >> + /* Set in2 to the input operand for cc calculation */ >> + tcg_gen_mov_i64(o->in2, o->out); >> + >> + tcg_temp_free_i64(m2); >> + return NO_EXIT; >> +} > I don't believe that this computes the right cc. > You need to place m2 into in2, not out. > > That said, maybe > > tcg_temp_free_i64(o->in2); > o->in2 = m2; > > is the right way to perform that copy. Since we > know that in2 was in2_a2, allocating a temp for an > address, we also know we can both (1) free it and > (2) put another temp in there that will be freed. Well, alternatively I could try to make out be the memory value at the end of the op and invent new load/store helpers for atomic instructions. That way the op bit should shrink significantly and we get the correct operand in s->out automatically. > > Don't worry about the atomic-ness of the operation > until some of the many patch sets going round on > that very subject are resolved. This will be good > enough for system mode for now. > > I think maybe the whole target ought to be cleaned > up for the new TCGMemOps. For this, you could share > the code for LAA and LAAG with the TCGMemOp in insn->data. That's an interesting idea. Let me see what I can pull off. Alex