From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:45966)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <aurelien@aurel32.net>) id 1YzrVL-0005bI-Ub
	for qemu-devel@nongnu.org; Tue, 02 Jun 2015 15:05:40 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <aurelien@aurel32.net>) id 1YzrVK-0004lI-WF
	for qemu-devel@nongnu.org; Tue, 02 Jun 2015 15:05:39 -0400
Received: from hall.aurel32.net ([2001:bc8:30d7:101::1]:34176)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <aurelien@aurel32.net>) id 1YzrVK-0004l6-Eu
	for qemu-devel@nongnu.org; Tue, 02 Jun 2015 15:05:38 -0400
Date: Tue, 2 Jun 2015 21:05:34 +0200
From: Aurelien Jarno <aurelien@aurel32.net>
Message-ID: <20150602190534.GD26129@aurel32.net>
References: <1433193897-24110-1-git-send-email-aurelien@aurel32.net>
	<1433193897-24110-10-git-send-email-aurelien@aurel32.net>
	<556DE2D7.5000005@twiddle.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <556DE2D7.5000005@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH 09/13] target-s390x: implement TRANSLATE
 EXTENDED instruction
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: Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>

On 2015-06-02 10:07, Richard Henderson wrote:
> On 06/01/2015 02:24 PM, Aurelien Jarno wrote:
> > +/* TRANSLATE EXTENDED */
> > +    C(0xb2a5, TRE,     RRE,   Z,   0, 0, 0, 0, tre, 0)
> ...
> > +static ExitStatus op_tre(DisasContext *s, DisasOps *o)
> > +{
> > +    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
> > +    TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2));
> > +    potential_page_fault(s);
> > +    gen_helper_tre(cpu_env, r1, r2);
> > +    tcg_temp_free_i32(r1);
> > +    tcg_temp_free_i32(r2);
> > +    set_cc_static(s);
> > +    return NO_EXIT;
> > +}
> 
> Missing the specification exception for odd r1.

Good catch.

> Easily fixable by using prep_r1_P.  You don't necessarily have to do anything
> else -- merely prepping out+out2 are sufficient.
> 
> But why don't we just pass and return (most) of the data to the helper?  Like
> 
>   C(0xb2a5, TRE,     RRE,   Z,   0, r2, r1_P, 0, tre, 0)
> 
>   potential_page_fault(s);
>   gen_helper_tre(o->out, cpu_env, o->out, o->out2, o->in2);
>   return_low128(o->out2);
>   set_cc_static(s);

My point was that we need to pass 4 values (reg0, r1, r1+1 and r2) and
return 3 values (r1, r1+1 and cc), so it's probably better to pass all
of them the same way. It's the strategy chosen for other similar
instructions (e.g mvcl), except for cc.

I'll change that in the next version.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net