Re: [PATCH v5 32/36] target/s390x: Use tcg_gen_atomic_cmpxchg_i128 for CDSG

[Richard Henderson]

On 1/26/23 01:27, David Hildenbrand wrote:
> >   static DisasJumpType op_csst(DisasContext *s, DisasOps *o)
> > @@ -5419,6 +5410,14 @@ static void prep_r1_P(DisasContext *s, DisasOps *o)
> >   }
> >   #define SPEC_prep_r1_P SPEC_r1_even
> > +static void prep_r1_D64(DisasContext *s, DisasOps *o)
> > +{
> > +    int r1 = get_field(s, r1);
> > +    o->out_128 = tcg_temp_new_i128();
> > +    tcg_gen_concat_i64_i128(o->out_128, regs[r1 + 1], regs[r1]);
>
> I really wonder if we should simply move the tcg_gen_concat_i64_i128() into the op and use 
> a generic "allocate out_128" instead.
>
> At least that part here confused me heavily.

Just the prep_r1_D64 bit of it?

Better for you as


    C(0xeb3e, CDSG,    RSY_a, Z,   la2, r3_D64, 0, r1_D64, cdsg, 0)


 static DisasJumpType op_cdsg(DisasContext *s, DisasOps *o)
 {
     int r1 = get_field(s, r1);
+    o->out_128 = tcg_temp_new_i128();
+    tcg_gen_concat_i64_i128(o->out_128, regs[r1 + 1], regs[r1]);


The existing generic "allocate out_128" is named "new_x", which I thought was also 
confusing, since this isn't an "x" format operation.  Since there's only one use, I did it 
inline.


r~