| Message ID | 20240202141318.95962-3-iii@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/s390x: Emulate CVDG and CVB* | expand |
On 02/02/2024 15.11, Ilya Leoshkevich wrote: > Convert to Binary - counterparts of the already implemented Convert > to Decimal (CVD*) instructions. > Example from the Principles of Operation: 25594C becomes 63FA. > > Co-developed-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > target/s390x/helper.h | 2 + > target/s390x/tcg/insn-data.h.inc | 4 ++ > target/s390x/tcg/int_helper.c | 72 ++++++++++++++++++++++++++++++++ > target/s390x/tcg/translate.c | 16 +++++++ > 4 files changed, 94 insertions(+) > > diff --git a/target/s390x/helper.h b/target/s390x/helper.h > index 332a9a9c632..cc1c20e9e3f 100644 > --- a/target/s390x/helper.h > +++ b/target/s390x/helper.h > @@ -88,6 +88,8 @@ DEF_HELPER_FLAGS_3(tcxb, TCG_CALL_NO_RWG_SE, i32, env, i128, i64) > 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_3(cvb, void, env, i32, i64) > +DEF_HELPER_FLAGS_2(cvbg, TCG_CALL_NO_WG, i64, 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) > diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc > index 388dcb8dbbc..e7d61cdec28 100644 > --- a/target/s390x/tcg/insn-data.h.inc > +++ b/target/s390x/tcg/insn-data.h.inc > @@ -293,6 +293,10 @@ > D(0xec73, CLFIT, RIE_a, GIE, r1_32u, i2_16u, 0, 0, ct, 0, 1) > D(0xec71, CLGIT, RIE_a, GIE, r1_o, i2_16u, 0, 0, ct, 0, 1) > > +/* CONVERT TO BINARY */ > + C(0x4f00, CVB, RX_a, Z, la2, 0, 0, 0, cvb, 0) > + C(0xe306, CVBY, RXY_a, LD, la2, 0, 0, 0, cvb, 0) > + C(0xe30e, CVBG, RXY_a, Z, la2, 0, r1, 0, cvbg, 0) > /* 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) > diff --git a/target/s390x/tcg/int_helper.c b/target/s390x/tcg/int_helper.c > index 121e3006a65..17974375e98 100644 > --- a/target/s390x/tcg/int_helper.c > +++ b/target/s390x/tcg/int_helper.c > @@ -25,6 +25,7 @@ > #include "exec/exec-all.h" > #include "qemu/host-utils.h" > #include "exec/helper-proto.h" > +#include "exec/cpu_ldst.h" > > /* #define DEBUG_HELPER */ > #ifdef DEBUG_HELPER > @@ -98,6 +99,77 @@ Int128 HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al, uint64_t b) > tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); > } > > +void HELPER(cvb)(CPUS390XState *env, uint32_t r1, uint64_t dec) > +{ > + int64_t pow10 = 1, bin = 0; > + int digit, sign; > + > + sign = dec & 0xf; > + if (sign < 0xa) { > + tcg_s390_data_exception(env, 0, GETPC()); > + } > + dec >>= 4; > + > + while (dec) { > + digit = dec & 0xf; > + if (digit > 0x9) { > + tcg_s390_data_exception(env, 0, GETPC()); > + } > + dec >>= 4; > + bin += digit * pow10; > + pow10 *= 10; > + } > + > + if (sign == 0xb || sign == 0xd) { > + bin = -bin; > + } > + > + /* R1 is updated even on fixed-point-divide exception. */ > + env->regs[r1] = (env->regs[r1] & 0xffffffff00000000ULL) | (uint32_t)bin; > + if (bin != (int32_t)bin) { > + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); > + } > +} > + > +uint64_t HELPER(cvbg)(CPUS390XState *env, Int128 dec) > +{ > + uint64_t dec64[] = {int128_getlo(dec), int128_gethi(dec)}; > + int64_t bin = 0, pow10, tmp; > + int digit, i, sign; > + > + sign = dec64[0] & 0xf; > + if (sign < 0xa) { > + tcg_s390_data_exception(env, 0, GETPC()); > + } > + dec64[0] >>= 4; > + pow10 = (sign == 0xb || sign == 0xd) ? -1 : 1; > + > + for (i = 1; i < 20; i++) { > + digit = dec64[i >> 4] & 0xf; > + if (digit > 0x9) { > + tcg_s390_data_exception(env, 0, GETPC()); > + } > + dec64[i >> 4] >>= 4; > + tmp = pow10 * digit; > + if (digit && ((tmp ^ pow10) < 0)) { That tmp ^ pow10 caused some frowning for me first, but it's just a check whether the sign changed, right? ... a comment in front of the line might be helpful. > + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); > + } > + tmp = bin + tmp; > + if (bin && ((tmp ^ bin) < 0)) { > + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); > + } > + bin = tmp; > + pow10 *= 10; > + } > + > + g_assert(!dec64[0]); > + if (dec64[1]) { > + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); > + } > + > + return bin; > +} Patch looks sane to me now, but I'd appreciate if Richard and/or David could have a look at this, too! Thomas
On Mon, Feb 05, 2024 at 06:04:27PM +0100, Thomas Huth wrote: > On 02/02/2024 15.11, Ilya Leoshkevich wrote: > > Convert to Binary - counterparts of the already implemented Convert > > to Decimal (CVD*) instructions. > > Example from the Principles of Operation: 25594C becomes 63FA. > > > > Co-developed-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > > --- > > target/s390x/helper.h | 2 + > > target/s390x/tcg/insn-data.h.inc | 4 ++ > > target/s390x/tcg/int_helper.c | 72 ++++++++++++++++++++++++++++++++ > > target/s390x/tcg/translate.c | 16 +++++++ > > 4 files changed, 94 insertions(+) [...] > > +uint64_t HELPER(cvbg)(CPUS390XState *env, Int128 dec) > > +{ > > + uint64_t dec64[] = {int128_getlo(dec), int128_gethi(dec)}; > > + int64_t bin = 0, pow10, tmp; > > + int digit, i, sign; > > + > > + sign = dec64[0] & 0xf; > > + if (sign < 0xa) { > > + tcg_s390_data_exception(env, 0, GETPC()); > > + } > > + dec64[0] >>= 4; > > + pow10 = (sign == 0xb || sign == 0xd) ? -1 : 1; > > + > > + for (i = 1; i < 20; i++) { > > + digit = dec64[i >> 4] & 0xf; > > + if (digit > 0x9) { > > + tcg_s390_data_exception(env, 0, GETPC()); > > + } > > + dec64[i >> 4] >>= 4; > > + tmp = pow10 * digit; > > + if (digit && ((tmp ^ pow10) < 0)) { > > That tmp ^ pow10 caused some frowning for me first, but it's just a check > whether the sign changed, right? ... a comment in front of the line might be > helpful. Good point about writing a comment, I tried to elaborate why checking the sign is justified, and realized that it's actually redundant. The int64_t bounds are roughly +-9.2E+18, and the last pow10 value in this loop is +-1E+18. The multiplication cannot overflow. > > + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); > > + } > > + tmp = bin + tmp; > > + if (bin && ((tmp ^ bin) < 0)) { The addition, however, can, e.g., bin=9E+17 and tmp=9E+18. So I'll send a v5 without the first check and with a comment. [...]
diff --git a/target/s390x/helper.h b/target/s390x/helper.h index 332a9a9c632..cc1c20e9e3f 100644 --- a/target/s390x/helper.h +++ b/target/s390x/helper.h @@ -88,6 +88,8 @@ DEF_HELPER_FLAGS_3(tcxb, TCG_CALL_NO_RWG_SE, i32, env, i128, i64) 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_3(cvb, void, env, i32, i64) +DEF_HELPER_FLAGS_2(cvbg, TCG_CALL_NO_WG, i64, 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) diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc index 388dcb8dbbc..e7d61cdec28 100644 --- a/target/s390x/tcg/insn-data.h.inc +++ b/target/s390x/tcg/insn-data.h.inc @@ -293,6 +293,10 @@ D(0xec73, CLFIT, RIE_a, GIE, r1_32u, i2_16u, 0, 0, ct, 0, 1) D(0xec71, CLGIT, RIE_a, GIE, r1_o, i2_16u, 0, 0, ct, 0, 1) +/* CONVERT TO BINARY */ + C(0x4f00, CVB, RX_a, Z, la2, 0, 0, 0, cvb, 0) + C(0xe306, CVBY, RXY_a, LD, la2, 0, 0, 0, cvb, 0) + C(0xe30e, CVBG, RXY_a, Z, la2, 0, r1, 0, cvbg, 0) /* 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) diff --git a/target/s390x/tcg/int_helper.c b/target/s390x/tcg/int_helper.c index 121e3006a65..17974375e98 100644 --- a/target/s390x/tcg/int_helper.c +++ b/target/s390x/tcg/int_helper.c @@ -25,6 +25,7 @@ #include "exec/exec-all.h" #include "qemu/host-utils.h" #include "exec/helper-proto.h" +#include "exec/cpu_ldst.h" /* #define DEBUG_HELPER */ #ifdef DEBUG_HELPER @@ -98,6 +99,77 @@ Int128 HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al, uint64_t b) tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); } +void HELPER(cvb)(CPUS390XState *env, uint32_t r1, uint64_t dec) +{ + int64_t pow10 = 1, bin = 0; + int digit, sign; + + sign = dec & 0xf; + if (sign < 0xa) { + tcg_s390_data_exception(env, 0, GETPC()); + } + dec >>= 4; + + while (dec) { + digit = dec & 0xf; + if (digit > 0x9) { + tcg_s390_data_exception(env, 0, GETPC()); + } + dec >>= 4; + bin += digit * pow10; + pow10 *= 10; + } + + if (sign == 0xb || sign == 0xd) { + bin = -bin; + } + + /* R1 is updated even on fixed-point-divide exception. */ + env->regs[r1] = (env->regs[r1] & 0xffffffff00000000ULL) | (uint32_t)bin; + if (bin != (int32_t)bin) { + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); + } +} + +uint64_t HELPER(cvbg)(CPUS390XState *env, Int128 dec) +{ + uint64_t dec64[] = {int128_getlo(dec), int128_gethi(dec)}; + int64_t bin = 0, pow10, tmp; + int digit, i, sign; + + sign = dec64[0] & 0xf; + if (sign < 0xa) { + tcg_s390_data_exception(env, 0, GETPC()); + } + dec64[0] >>= 4; + pow10 = (sign == 0xb || sign == 0xd) ? -1 : 1; + + for (i = 1; i < 20; i++) { + digit = dec64[i >> 4] & 0xf; + if (digit > 0x9) { + tcg_s390_data_exception(env, 0, GETPC()); + } + dec64[i >> 4] >>= 4; + tmp = pow10 * digit; + if (digit && ((tmp ^ pow10) < 0)) { + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); + } + tmp = bin + tmp; + if (bin && ((tmp ^ bin) < 0)) { + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); + } + bin = tmp; + pow10 *= 10; + } + + g_assert(!dec64[0]); + if (dec64[1]) { + tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC()); + } + + return bin; +} + uint64_t HELPER(cvd)(int32_t reg) { /* positive 0 */ diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index c2fdc920a50..325b25959d3 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -2223,6 +2223,22 @@ static DisasJumpType op_csp(DisasContext *s, DisasOps *o) } #endif +static DisasJumpType op_cvb(DisasContext *s, DisasOps *o) +{ + TCGv_i64 t = tcg_temp_new_i64(); + tcg_gen_qemu_ld_i64(t, o->addr1, get_mem_index(s), MO_TEUQ); + gen_helper_cvb(tcg_env, tcg_constant_i32(get_field(s, r1)), t); + return DISAS_NEXT; +} + +static DisasJumpType op_cvbg(DisasContext *s, DisasOps *o) +{ + TCGv_i128 t = tcg_temp_new_i128(); + tcg_gen_qemu_ld_i128(t, o->addr1, get_mem_index(s), MO_TE | MO_128); + gen_helper_cvbg(o->out, tcg_env, t); + return DISAS_NEXT; +} + static DisasJumpType op_cvd(DisasContext *s, DisasOps *o) { TCGv_i64 t1 = tcg_temp_new_i64();
Convert to Binary - counterparts of the already implemented Convert to Decimal (CVD*) instructions. Example from the Principles of Operation: 25594C becomes 63FA. Co-developed-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- target/s390x/helper.h | 2 + target/s390x/tcg/insn-data.h.inc | 4 ++ target/s390x/tcg/int_helper.c | 72 ++++++++++++++++++++++++++++++++ target/s390x/tcg/translate.c | 16 +++++++ 4 files changed, 94 insertions(+)