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

[Richard Henderson]

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~