[Qemu-devel] [PATCH 0/2] target-s390x: fixes for GMP testsuite

[Qemu-devel] [PATCH 0/2] target-s390x: fixes for GMP testsuite

[Paolo Bonzini]

Reported and tested by Torbjorn.  One patch is mine, one is his.

Paolo Bonzini (1):
  target-s390x: support OC and NC in the EX instruction

Torbjorn Granlund (1):
  target-s390x: fix and optimize slb* and slbg* computation of
    carry/borrow flag

 target-s390x/cc_helper.c  | 18 ++++--------------
 target-s390x/mem_helper.c |  8 ++++++++
 2 files changed, 12 insertions(+), 14 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] target-s390x: support OC and NC in the EX instruction

[Paolo Bonzini]

This is needed to run the GMP testsuite.

Reported-by: Torbjorn Granlund <torbjorng@google.com>
Tested-by: Torbjorn Granlund <torbjorng@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-s390x/mem_helper.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c
index 5a55de8..4de3fc2 100644
--- a/target-s390x/mem_helper.c
+++ b/target-s390x/mem_helper.c
@@ -490,10 +490,18 @@ uint32_t HELPER(ex)(CPUS390XState *env, uint32_t cc, uint64_t v1,
             helper_mvc(env, l, get_address(env, 0, b1, d1),
                        get_address(env, 0, b2, d2));
             break;
+        case 0x400:
+            cc = helper_nc(env, l, get_address(env, 0, b1, d1),
+                            get_address(env, 0, b2, d2));
+            break;
         case 0x500:
             cc = helper_clc(env, l, get_address(env, 0, b1, d1),
                             get_address(env, 0, b2, d2));
             break;
+        case 0x600:
+            cc = helper_oc(env, l, get_address(env, 0, b1, d1),
+                            get_address(env, 0, b2, d2));
+            break;
         case 0x700:
             cc = helper_xc(env, l, get_address(env, 0, b1, d1),
                            get_address(env, 0, b2, d2));
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] target-s390x: fix and optimize slb* and slbg* computation of carry/borrow flag

[Paolo Bonzini]

From: Torbjorn Granlund <torbjorng@google.com>

This patch fixes the bug with borrow_in being set incorrectly, but it
also simplifies the logic to be much more plain, improving speed.  It
fixes both the 32-bit SLB* and 64-bit SLBG*.

The SLBG* change has been well-tested.  I haven't tested the SLB* change
explicitly, but the code was copy-pasted from the tested code.

The error of these functions' current implementations would not likely
be triggered by compiler-generated code, since the only error was in the
state of the carry/borrow flag.  Compilers rarely generate an
instruction sequence such as carry-set -> carry-set-and-use ->
carry-use.

(With Paolo's fix and mine, there are still a couple of failures from
GMP's testsuite, but they are almost surely due to incorrect code
generation from gcc 4.9.  But since this gcc is running under qemu, it
might be qemu bugs.  I intend to investigate this.)

Signed-off-by: Torbjorn Granlund <torbjorng@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-s390x/cc_helper.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/target-s390x/cc_helper.c b/target-s390x/cc_helper.c
index 373eb17..00bc883 100644
--- a/target-s390x/cc_helper.c
+++ b/target-s390x/cc_helper.c
@@ -179,16 +179,11 @@ static uint32_t cc_calc_subu_64(uint64_t a1, uint64_t a2, uint64_t ar)
 
 static uint32_t cc_calc_subb_64(uint64_t a1, uint64_t a2, uint64_t ar)
 {
-    /* We had borrow-in if normal subtraction isn't equal.  */
-    int borrow_in = ar - (a1 - a2);
     int borrow_out;
 
-    /* If a2 was ULONG_MAX, and borrow_in, then a2 is logically 65 bits,
-       and we must have had borrow out.  */
-    if (borrow_in && a2 == (uint64_t)-1) {
-        borrow_out = 1;
+    if (ar != a1 - a2) {	/* difference means borrow-in */
+        borrow_out = (a2 >= a1);
     } else {
-        a2 += borrow_in;
         borrow_out = (a2 > a1);
     }
 
@@ -285,16 +280,11 @@ static uint32_t cc_calc_subu_32(uint32_t a1, uint32_t a2, uint32_t ar)
 
 static uint32_t cc_calc_subb_32(uint32_t a1, uint32_t a2, uint32_t ar)
 {
-    /* We had borrow-in if normal subtraction isn't equal.  */
-    int borrow_in = ar - (a1 - a2);
     int borrow_out;
 
-    /* If a2 was UINT_MAX, and borrow_in, then a2 is logically 65 bits,
-       and we must have had borrow out.  */
-    if (borrow_in && a2 == (uint32_t)-1) {
-        borrow_out = 1;
+    if (ar != a1 - a2) {	/* difference means borrow-in */
+        borrow_out = (a2 >= a1);
     } else {
-        a2 += borrow_in;
         borrow_out = (a2 > a1);
     }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/2] target-s390x: fixes for GMP testsuite

[Paolo Bonzini]

On 08/01/2015 18:01, Paolo Bonzini wrote:
> Reported and tested by Torbjorn.  One patch is mine, one is his.
> 
> Paolo Bonzini (1):
>   target-s390x: support OC and NC in the EX instruction
> 
> Torbjorn Granlund (1):
>   target-s390x: fix and optimize slb* and slbg* computation of
>     carry/borrow flag
> 
>  target-s390x/cc_helper.c  | 18 ++++--------------
>  target-s390x/mem_helper.c |  8 ++++++++
>  2 files changed, 12 insertions(+), 14 deletions(-)
> 

Alex, ping?

Paolo