Re: [PATCH v1 4/6] s390x/tcg: Fix VECTOR SUBTRACT COMPUTE BORROW INDICATION

[David Hildenbrand]

On 18.10.19 18:10, David Hildenbrand wrote:
> Looks like my idea of what a "borrow" is was wrong. We are dealing with
> unsigned numbers. A subtraction is simply an addition with the bitwise
> complement. If we get a carry during the addition, that's the borrow.
> "The operands are treated as unsigned binary integers."
>
> This is nice, as we can reuse the VECTOR ADD COMPUTE CARRY functions
> and avoid helpers, all we have to do is compute the bitwise complement.
>
> Fixes: 1ee2d7ba72f6 ("s390x/tcg: Implement VECTOR SUBTRACT COMPUTE BORROW 
> INDICATION")
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>   target/s390x/helper.h           |  2 --
>   target/s390x/translate_vx.inc.c | 45 ++++++++++++++++++++++++---------
>   target/s390x/vec_int_helper.c   | 16 ------------
>   3 files changed, 33 insertions(+), 30 deletions(-)
>
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 56e8149866..ca1e08100a 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -207,8 +207,6 @@ DEF_HELPER_FLAGS_4(gvec_verim16, TCG_CALL_NO_RWG, void, 
> ptr, cptr, cptr, i32)
>   DEF_HELPER_FLAGS_4(gvec_vsl, TCG_CALL_NO_RWG, void, ptr, cptr, i64, i32)
>   DEF_HELPER_FLAGS_4(gvec_vsra, TCG_CALL_NO_RWG, void, ptr, cptr, i64, i32)
>   DEF_HELPER_FLAGS_4(gvec_vsrl, TCG_CALL_NO_RWG, void, ptr, cptr, i64, i32)
> -DEF_HELPER_FLAGS_4(gvec_vscbi8, TCG_CALL_NO_RWG, void, ptr, cptr, cptr, i32)
> -DEF_HELPER_FLAGS_4(gvec_vscbi16, TCG_CALL_NO_RWG, void, ptr, cptr, cptr, i32)
>   DEF_HELPER_4(gvec_vtm, void, ptr, cptr, env, i32)
>   
>   /* === Vector String Instructions === */
> diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
> index 5ce7bfb0af..40bcc1604e 100644
> --- a/target/s390x/translate_vx.inc.c
> +++ b/target/s390x/translate_vx.inc.c
> @@ -2130,14 +2130,40 @@ static DisasJumpType op_vs(DisasContext *s, DisasOps *o)
>       return DISAS_NEXT;
>   }
>   
> +static void gen_scbi8_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
> +{
> +    TCGv_i64 t = tcg_temp_new_i64();
> +
> +    tcg_gen_not_i64(t, b);
> +    gen_acc(d, a, t, ES_8);
> +    tcg_temp_free_i64(t);
> +}

BTW, I would have thought that we need the 2nd complement in all these 
cases. However, the description of the other functions confused me 
(VECTOR SUBTRACT WITH BORROW INDICATION) - add bitwise complement and 
add the borrow.

This passes my test cases (that are verified against real HW), but I am 
not sure if I check all the corner cases.

@Richard, do you have any idea how to do it the right way for this 
instruction?

--

Thanks,

David / dhildenb