From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: Bignum speedup patch causes crash at startup Date: Tue, 4 Sep 2018 11:59:53 -0700 Organization: UCLA Computer Science Department Message-ID: <54c97913-984d-6129-e126-3c7fb66ed6c6@cs.ucla.edu> References: <838t4hz4bd.fsf@gnu.org> <831sa9z0wa.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------57E7364A19DD4504FE9F0F9C" X-Trace: blaine.gmane.org 1536087532 15859 195.159.176.226 (4 Sep 2018 18:58:52 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 4 Sep 2018 18:58:52 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 Cc: andrewjmoreton@gmail.com, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Sep 04 20:58:48 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fxGXH-0003xZ-C6 for ged-emacs-devel@m.gmane.org; Tue, 04 Sep 2018 20:58:48 +0200 Original-Received: from localhost ([::1]:52271 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fxGZN-0005ba-Rh for ged-emacs-devel@m.gmane.org; Tue, 04 Sep 2018 15:00:57 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:56995) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fxGYY-0005aw-9S for emacs-devel@gnu.org; Tue, 04 Sep 2018 15:00:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fxGYW-0006Q9-6t for emacs-devel@gnu.org; Tue, 04 Sep 2018 15:00:06 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:37108) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fxGYQ-0006JG-KK; Tue, 04 Sep 2018 14:59:58 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id D52F11616B4; Tue, 4 Sep 2018 11:59:55 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id YLfvkEOF6vkC; Tue, 4 Sep 2018 11:59:55 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 025AB1616C2; Tue, 4 Sep 2018 11:59:55 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 1xikgyuwDzzN; Tue, 4 Sep 2018 11:59:54 -0700 (PDT) Original-Received: from [192.168.1.9] (cpe-23-242-74-103.socal.res.rr.com [23.242.74.103]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id CABF61616B4; Tue, 4 Sep 2018 11:59:54 -0700 (PDT) In-Reply-To: <831sa9z0wa.fsf@gnu.org> Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:229255 Archived-At: This is a multi-part message in MIME format. --------------57E7364A19DD4504FE9F0F9C Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Eli Zaretskii wrote: > I don't understand why it prefers GMP calls to a simple double > division, when both arguments are fixnums. What am I missing? First, converting a fixnum to a double can lose info. Second, the double division could round, which would mean the result would be double-rounded. We're already suffering from double-rounding when either argument is floating-point, and we don't want that bug to also infect integer division. So even the Emacs 26 code (which doesn't have bignums) must use integer division here, not double division. This is worth commenting so I installed the attached. While writing the comment I noticed there's a bug when one argument is a bignum and the other is a float, so I fixed that in the attached too. --------------57E7364A19DD4504FE9F0F9C Content-Type: text/x-patch; name="0001-Fix-round-FLOAT-BIGNUM-bug.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Fix-round-FLOAT-BIGNUM-bug.patch" >From 21637d5e5b29d5ec8fb966c0ddfbfba3eb33da38 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 4 Sep 2018 11:49:41 -0700 Subject: [PATCH] Fix (round FLOAT BIGNUM) bug * src/floatfns.c (rounding_driver): Fix bug when one argument is a float and the other is a bignum. * test/src/floatfns-tests.el (bignum-round): Test for the bug. --- src/floatfns.c | 7 +++++-- test/src/floatfns-tests.el | 5 +++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/floatfns.c b/src/floatfns.c index 2f33b8652b..13ab7b0359 100644 --- a/src/floatfns.c +++ b/src/floatfns.c @@ -355,6 +355,8 @@ rounding_driver (Lisp_Object arg, Lisp_Object divisor, CHECK_NUMBER (divisor); if (!FLOATP (arg) && !FLOATP (divisor)) { + /* Divide as integers. Converting to double might lose + info, even for fixnums; also see the FIXME below. */ if (EQ (divisor, make_fixnum (0))) xsignal0 (Qarith_error); int_divide (mpz[0], @@ -363,10 +365,11 @@ rounding_driver (Lisp_Object arg, Lisp_Object divisor, return make_integer_mpz (); } - double f1 = FLOATP (arg) ? XFLOAT_DATA (arg) : XFIXNUM (arg); - double f2 = FLOATP (divisor) ? XFLOAT_DATA (divisor) : XFIXNUM (divisor); + double f1 = XFLOATINT (arg); + double f2 = XFLOATINT (divisor); if (! IEEE_FLOATING_POINT && f2 == 0) xsignal0 (Qarith_error); + /* FIXME: This division rounds, so the result is double-rounded. */ d = f1 / f2; } diff --git a/test/src/floatfns-tests.el b/test/src/floatfns-tests.el index d41b08f796..9a382058b4 100644 --- a/test/src/floatfns-tests.el +++ b/test/src/floatfns-tests.el @@ -70,6 +70,11 @@ (should (= n (floor n))) (should (= n (round n))) (should (= n (truncate n))) + (let ((-n (- n)) + (f (float n)) + (-f (- (float n)))) + (should (= 1 (round n f) (round -n -f) (round f n) (round -f -n))) + (should (= -1 (round -n f) (round n -f) (round f -n) (round -f n)))) (dolist (d ns) (let ((q (/ n d)) (r (% n d)) -- 2.17.1 --------------57E7364A19DD4504FE9F0F9C--