From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:44592)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1Ys1Ge-0008Lo-QP
	for qemu-devel@nongnu.org; Mon, 11 May 2015 23:54:05 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1Ys1Gb-0004Ab-L5
	for qemu-devel@nongnu.org; Mon, 11 May 2015 23:54:04 -0400
Received: from mail-pa0-x22d.google.com ([2607:f8b0:400e:c03::22d]:33203)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rth7680@gmail.com>) id 1Ys1Gb-0004AV-Cr
	for qemu-devel@nongnu.org; Mon, 11 May 2015 23:54:01 -0400
Received: by pacwv17 with SMTP id wv17so129559257pac.0
	for <qemu-devel@nongnu.org>; Mon, 11 May 2015 20:54:00 -0700 (PDT)
Sender: Richard Henderson <rth7680@gmail.com>
Message-ID: <55517954.8010308@twiddle.net>
Date: Mon, 11 May 2015 20:53:56 -0700
From: Richard Henderson <rth@twiddle.net>
MIME-Version: 1.0
References: <1431047528-81504-1-git-send-email-agraf@suse.de>
	<1431047528-81504-2-git-send-email-agraf@suse.de>
In-Reply-To: <1431047528-81504-2-git-send-email-agraf@suse.de>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 2/2] s390x: Add laa and laag instructions
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alexander Graf <agraf@suse.de>, qemu-devel@nongnu.org

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.

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.


r~