[PATCH v1 5/6] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW INDICATION

David Hildenbrand posted 6 patches 6 years ago
Maintainers: Cornelia Huck <cohuck@redhat.com>, Richard Henderson <rth@twiddle.net>, David Hildenbrand <david@redhat.com>
There is a newer version of this series
[PATCH v1 5/6] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW INDICATION
Posted by David Hildenbrand 6 years ago
Testing this, there seems to be something messed up. We are dealing with
unsigned numbers. "Each operand is treated as an unsigned binary integer."
Let's just implement as written in the PoP:

"A subtraction is performed by adding the contents of
 the second operand with the bitwise complement of
 the third operand along with a borrow indication from
 the rightmost bit position of the fourth operand and
 the result is placed in the first operand."

Fixes: 48390a7c2716 ("s390x/tcg: Implement VECTOR SUBTRACT WITH BORROW INDICATION")
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/translate_vx.inc.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
index 40bcc1604e..87b5790db4 100644
--- a/target/s390x/translate_vx.inc.c
+++ b/target/s390x/translate_vx.inc.c
@@ -2207,12 +2207,18 @@ static void gen_sbi2_i64(TCGv_i64 dl, TCGv_i64 dh, TCGv_i64 al, TCGv_i64 ah,
                          TCGv_i64 bl, TCGv_i64 bh, TCGv_i64 cl, TCGv_i64 ch)
 {
     TCGv_i64 tl = tcg_temp_new_i64();
+    TCGv_i64 th = tcg_temp_new_i64();
+    TCGv_i64 t = tcg_temp_new_i64();
     TCGv_i64 zero = tcg_const_i64(0);
 
-    tcg_gen_andi_i64(tl, cl, 1);
-    tcg_gen_sub2_i64(dl, dh, al, ah, bl, bh);
-    tcg_gen_sub2_i64(dl, dh, dl, dh, tl, zero);
+    tcg_gen_andi_i64(t, cl, 1);
+    tcg_gen_not_i64(tl, bl);
+    tcg_gen_not_i64(th, bh);
+    tcg_gen_add2_i64(dl, dh, al, ah, tl, th);
+    tcg_gen_add2_i64(dl, dh, dl, dh, t, zero);
     tcg_temp_free_i64(tl);
+    tcg_temp_free_i64(th);
+    tcg_temp_free_i64(t);
     tcg_temp_free_i64(zero);
 }
 
-- 
2.21.0


Re: [PATCH v1 5/6] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW INDICATION
Posted by Richard Henderson 6 years ago
On 10/18/19 9:10 AM, David Hildenbrand wrote:
> Testing this, there seems to be something messed up. We are dealing with
> unsigned numbers. "Each operand is treated as an unsigned binary integer."
> Let's just implement as written in the PoP:
> 
> "A subtraction is performed by adding the contents of
>  the second operand with the bitwise complement of
>  the third operand along with a borrow indication from
>  the rightmost bit position of the fourth operand and
>  the result is placed in the first operand."
> 
> Fixes: 48390a7c2716 ("s390x/tcg: Implement VECTOR SUBTRACT WITH BORROW INDICATION")
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/translate_vx.inc.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


Re: [PATCH v1 5/6] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW INDICATION
Posted by David Hildenbrand 6 years ago
On 18.10.19 21:11, Richard Henderson wrote:
> On 10/18/19 9:10 AM, David Hildenbrand wrote:
>> Testing this, there seems to be something messed up. We are dealing with
>> unsigned numbers. "Each operand is treated as an unsigned binary integer."
>> Let's just implement as written in the PoP:
>>
>> "A subtraction is performed by adding the contents of
>>   the second operand with the bitwise complement of
>>   the third operand along with a borrow indication from
>>   the rightmost bit position of the fourth operand and
>>   the result is placed in the first operand."
>>
>> Fixes: 48390a7c2716 ("s390x/tcg: Implement VECTOR SUBTRACT WITH BORROW INDICATION")
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>   target/s390x/translate_vx.inc.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 

I'll convert this patch to

diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
index 40bcc1604e..d9403a8163 100644
--- a/target/s390x/translate_vx.inc.c
+++ b/target/s390x/translate_vx.inc.c
@@ -2207,13 +2207,13 @@ static void gen_sbi2_i64(TCGv_i64 dl, TCGv_i64 dh, TCGv_i64 al, TCGv_i64 ah,
                          TCGv_i64 bl, TCGv_i64 bh, TCGv_i64 cl, TCGv_i64 ch)
 {
     TCGv_i64 tl = tcg_temp_new_i64();
-    TCGv_i64 zero = tcg_const_i64(0);
+    TCGv_i64 th = tcg_temp_new_i64();
 
-    tcg_gen_andi_i64(tl, cl, 1);
-    tcg_gen_sub2_i64(dl, dh, al, ah, bl, bh);
-    tcg_gen_sub2_i64(dl, dh, dl, dh, tl, zero);
+    tcg_gen_not_i64(tl, bl);
+    tcg_gen_not_i64(th, bh);
+    gen_ac2_i64(dl, dh, al, ah, tl, th, cl, ch);
     tcg_temp_free_i64(tl);
-    tcg_temp_free_i64(zero);
+    tcg_temp_free_i64(th);
 }



-- 

Thanks,

David / dhildenb