| Message ID | 20240115202308.1930675-2-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/s390x: Emulate CVDG | expand |
On 15/01/2024 21.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(-) Looks sane to me! Reviewed-by: Thomas Huth <thuth@redhat.com>
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~
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
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; } diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index 8df00b7df9f..2d417337695 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -2233,6 +2233,14 @@ static DisasJumpType op_cvd(DisasContext *s, DisasOps *o) return DISAS_NEXT; } +static DisasJumpType op_cvdg(DisasContext *s, DisasOps *o) +{ + TCGv_i128 t = tcg_temp_new_i128(); + gen_helper_cvdg(t, o->in1); + tcg_gen_qemu_st_i128(t, o->in2, get_mem_index(s), MO_TE | MO_128); + return DISAS_NEXT; +} + static DisasJumpType op_ct(DisasContext *s, DisasOps *o) { int m3 = get_field(s, m3);
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(-)