From patchwork Mon Dec 15 23:44:49 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Torbjorn Granlund X-Patchwork-Id: 421723 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 11E1714009B for ; Tue, 16 Dec 2014 10:45:24 +1100 (AEDT) Received: from localhost ([::1]:42457 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0fKL-0008RX-Go for incoming@patchwork.ozlabs.org; Mon, 15 Dec 2014 18:45:21 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42589) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0fJy-00086J-Mc for qemu-devel@nongnu.org; Mon, 15 Dec 2014 18:45:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y0fJs-0001uC-Ie for qemu-devel@nongnu.org; Mon, 15 Dec 2014 18:44:58 -0500 Received: from servus.gmplib.org ([193.10.5.126]:59162 helo=shell.gmplib.org) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0fJs-0001tQ-7k for qemu-devel@nongnu.org; Mon, 15 Dec 2014 18:44:52 -0500 Received: by shell.gmplib.org (Postfix, from userid 1001) id 5A4DE1D621; Tue, 16 Dec 2014 00:44:49 +0100 (CET) To: Paolo Bonzini References: <86lhmo303u.fsf@shell.gmplib.org> <86egs32obj.fsf@shell.gmplib.org> <548E04EA.1030804@redhat.com> From: tg@gmplib.org (=?utf-8?Q?Torbj=C3=B6rn?= Granlund) Date: Tue, 16 Dec 2014 00:44:49 +0100 In-Reply-To: <548E04EA.1030804@redhat.com> (Paolo Bonzini's message of "Sun\, 14 Dec 2014 22\:45\:14 +0100") Message-ID: <861to032by.fsf@shell.gmplib.org> User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.3 (berkeley-unix) MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: FreeBSD 9.x X-Received-From: 193.10.5.126 Cc: qemu-devel Subject: Re: [Qemu-devel] Bug in s390 instruction emulation X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Paolo Bonzini writes: Something like this? 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)); That seems to work as per the needs of GMP. I had expected a bigger change to be needed. Thanks! Below is a more complete patch for the SLB* and SLBG* bugs. This patch is to be attributed to 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.) --- target-s390x/.~/cc_helper.c.~1~ 2014-12-09 15:45:44.000000000 +0100 +++ target-s390x/cc_helper.c 2014-12-14 23:02:31.605725763 +0100 @@ -179,16 +179,11 @@ 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_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); }