Re: [Qemu-devel] [PATCH 09/13] target-s390x: implement TRANSLATE EXTENDED instruction

[Aurelien Jarno]

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
address@hidden                 http://www.aurel32.net