Re: Re: [PATCH 1/2] target/s390x: Emulate CVDG

[Ilya Leoshkevich]

On Fri, Jan 19, 2024 at 08:12:18AM +1100, Richard Henderson wrote:
> On 1/16/24 07:21, Ilya Leoshkevich wrote:
> > CVDG is the same as CVD, except that it converts 64 bits into 128,
> > rather than 32 into 64. Use larger data types in the CVD helper and
> > reuse it.
> > 
> > Reported-by: Ido Plat <Ido.Plat@ibm.com>
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   target/s390x/helper.h            |  1 +
> >   target/s390x/tcg/insn-data.h.inc |  1 +
> >   target/s390x/tcg/int_helper.c    | 11 ++++++++---
> >   target/s390x/tcg/translate.c     |  8 ++++++++
> >   4 files changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> > index 05102578fc9..332a9a9c632 100644
> > --- a/target/s390x/helper.h
> > +++ b/target/s390x/helper.h
> > @@ -89,6 +89,7 @@ DEF_HELPER_FLAGS_2(sqeb, TCG_CALL_NO_WG, i64, env, i64)
> >   DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
> >   DEF_HELPER_FLAGS_2(sqxb, TCG_CALL_NO_WG, i128, env, i128)
> >   DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32)
> > +DEF_HELPER_FLAGS_1(cvdg, TCG_CALL_NO_RWG_SE, i128, s64)
> >   DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
> >   DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
> >   DEF_HELPER_FLAGS_4(pku, TCG_CALL_NO_WG, void, env, i64, i64, i32)
> > diff --git a/target/s390x/tcg/insn-data.h.inc 
> > b/target/s390x/tcg/insn-data.h.inc
> > index 2f07f39d9cb..388dcb8dbbc 100644
> > --- a/target/s390x/tcg/insn-data.h.inc
> > +++ b/target/s390x/tcg/insn-data.h.inc
> > @@ -296,6 +296,7 @@
> >   /* CONVERT TO DECIMAL */
> >       C(0x4e00, CVD,     RX_a,  Z,   r1_o, a2, 0, 0, cvd, 0)
> >       C(0xe326, CVDY,    RXY_a, LD,  r1_o, a2, 0, 0, cvd, 0)
> > +    C(0xe32e, CVDG,    RXY_a, Z,   r1_o, a2, 0, 0, cvdg, 0)
> >   /* CONVERT TO FIXED */
> >       F(0xb398, CFEBR,   RRF_e, Z,   0, e2, new, r1_32, cfeb, 0, IF_BFP)
> >       F(0xb399, CFDBR,   RRF_e, Z,   0, f2, new, r1_32, cfdb, 0, IF_BFP)
> > diff --git a/target/s390x/tcg/int_helper.c b/target/s390x/tcg/int_helper.c
> > index eb8e6dd1b57..defb8fc7681 100644
> > --- a/target/s390x/tcg/int_helper.c
> > +++ b/target/s390x/tcg/int_helper.c
> > @@ -99,10 +99,15 @@ Int128 HELPER(divu64)(CPUS390XState *env, uint64_t ah, 
> > uint64_t al, uint64_t b)
> >   }
> >   uint64_t HELPER(cvd)(int32_t reg)
> > +{
> > +    return helper_cvdg(reg);
> > +}
> > +
> > +Int128 HELPER(cvdg)(int64_t reg)
> >   {
> >       /* positive 0 */
> > -    uint64_t dec = 0x0c;
> > -    int64_t bin = reg;
> > +    Int128 dec = 0x0c;
> > +    Int128 bin = reg;
> >       int shift;
> >       if (bin < 0) {
> > @@ -110,7 +115,7 @@ uint64_t HELPER(cvd)(int32_t reg)
> >           dec = 0x0d;
> >       }
> > -    for (shift = 4; (shift < 64) && bin; shift += 4) {
> > +    for (shift = 4; (shift < 128) && bin; shift += 4) {
> >           dec |= (bin % 10) << shift;
> >           bin /= 10;
> >       }
> 
> None of this will work with the struct version of Int128 -- you need to use
> the int128_* functions for initialization and arithmetic.
> 
> I suggest you don't try to share code with CVD.
> 
> 
> r~

Hi,

I see, --cross-prefix=i686-linux-gnu- is very broken with this patch.
I will send a v2.

Best regards,
Ilya