[PATCH v1 6/6] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW COMPUTE 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 6/6] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW COMPUTE BORROW INDICATION
Posted by David Hildenbrand 6 years ago
The numbers are unsigned, the computation is wrong. "Each operand is
treated as an unsigned binary integer".
Let's implement as given 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 of the fourth operand."

Fixes: bc725e65152c ("s390x/tcg: Implement VECTOR SUBTRACT WITH BORROW COMPUTE BORROW INDICATION")
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/translate_vx.inc.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
index 87b5790db4..2015af9012 100644
--- a/target/s390x/translate_vx.inc.c
+++ b/target/s390x/translate_vx.inc.c
@@ -2240,17 +2240,30 @@ static void gen_sbcbi2_i64(TCGv_i64 dl, TCGv_i64 dh, TCGv_i64 al, TCGv_i64 ah,
 {
     TCGv_i64 th = tcg_temp_new_i64();
     TCGv_i64 tl = tcg_temp_new_i64();
+    TCGv_i64 sh = tcg_temp_new_i64();
+    TCGv_i64 sl = tcg_temp_new_i64();
     TCGv_i64 zero = tcg_const_i64(0);
 
     tcg_gen_andi_i64(tl, cl, 1);
-    tcg_gen_sub2_i64(tl, th, al, zero, tl, zero);
-    tcg_gen_sub2_i64(tl, th, tl, th, bl, zero);
+    tcg_gen_not_i64(sl, bl);
+    tcg_gen_not_i64(sh, bh);
+
+    /* Add the borrow to the low doubleword of a */
+    tcg_gen_add2_i64(tl, th, al, zero, tl, zero);
+    /* Add the bit-wise complement of b to the low doubleword */
+    tcg_gen_add2_i64(tl, th, tl, th, sl, zero);
+    /* Isolate the carry to the high doubleword */
     tcg_gen_andi_i64(th, th, 1);
-    tcg_gen_sub2_i64(tl, th, ah, zero, th, zero);
-    tcg_gen_sub2_i64(tl, th, tl, th, bh, zero);
+    /* Add the carry to the high doubleword of a */
+    tcg_gen_add2_i64(tl, th, ah, zero, th, zero);
+    /* Add the bit-wise complement of b to the high doubleword */
+    tcg_gen_add2_i64(tl, th, tl, th, sh, zero);
+    /* Isolate the carry to the next doubleword */
     tcg_gen_andi_i64(dl, th, 1);
     tcg_gen_mov_i64(dh, zero);
 
+    tcg_temp_free_i64(sl);
+    tcg_temp_free_i64(sh);
     tcg_temp_free_i64(tl);
     tcg_temp_free_i64(th);
     tcg_temp_free_i64(zero);
-- 
2.21.0


Re: [PATCH v1 6/6] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW COMPUTE BORROW INDICATION
Posted by Richard Henderson 6 years ago
On 10/18/19 9:10 AM, David Hildenbrand wrote:
> +    /* Isolate the carry to the next doubleword */
>      tcg_gen_andi_i64(dl, th, 1);

You can remove this now, since the only possible results are 0/1; it was only
our subtract implementation that produced -1/0.


r~

Re: [PATCH v1 6/6] s390x/tcg: Fix VECTOR SUBTRACT WITH BORROW COMPUTE BORROW INDICATION
Posted by David Hildenbrand 6 years ago
On 18.10.19 20:55, Richard Henderson wrote:
> On 10/18/19 9:10 AM, David Hildenbrand wrote:
>> +    /* Isolate the carry to the next doubleword */
>>       tcg_gen_andi_i64(dl, th, 1);
> 
> You can remove this now, since the only possible results are 0/1; it was only
> our subtract implementation that produced -1/0.
> 
> 
> r~
> 

Right, we can simply reuse the VACCC implementation now:

diff --git a/target/s390x/translate_vx.inc.c b/target/s390x/translate_vx.inc.c
index 87b5790db4..49f9916c37 100644
--- a/target/s390x/translate_vx.inc.c
+++ b/target/s390x/translate_vx.inc.c
@@ -2240,20 +2240,13 @@ static void gen_sbcbi2_i64(TCGv_i64 dl, TCGv_i64 dh, TCGv_i64 al, TCGv_i64 ah,
 {
     TCGv_i64 th = tcg_temp_new_i64();
     TCGv_i64 tl = tcg_temp_new_i64();
-    TCGv_i64 zero = tcg_const_i64(0);
 
-    tcg_gen_andi_i64(tl, cl, 1);
-    tcg_gen_sub2_i64(tl, th, al, zero, tl, zero);
-    tcg_gen_sub2_i64(tl, th, tl, th, bl, zero);
-    tcg_gen_andi_i64(th, th, 1);
-    tcg_gen_sub2_i64(tl, th, ah, zero, th, zero);
-    tcg_gen_sub2_i64(tl, th, tl, th, bh, zero);
-    tcg_gen_andi_i64(dl, th, 1);
-    tcg_gen_mov_i64(dh, zero);
+    tcg_gen_not_i64(tl, bl);
+    tcg_gen_not_i64(th, bh);
+    gen_accc2_i64(dl, dh, al, ah, tl, th, cl, ch);
 
     tcg_temp_free_i64(tl);
     tcg_temp_free_i64(th);
-    tcg_temp_free_i64(zero);
 }

This works as we only have to compute the bitwise complement.

-- 

Thanks,

David / dhildenb