From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Mark H Weaver Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCHES] Numeric improvements Date: Wed, 06 Mar 2013 15:30:44 -0500 Message-ID: <87k3pk9sp7.fsf@tines.lan> References: <87vc96ds5w.fsf@tines.lan> <87mwugd3o8.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1362601886 15736 80.91.229.3 (6 Mar 2013 20:31:26 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 6 Mar 2013 20:31:26 +0000 (UTC) Cc: guile-devel@gnu.org To: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Wed Mar 06 21:31:47 2013 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1UDL06-0000jk-Lu for guile-devel@m.gmane.org; Wed, 06 Mar 2013 21:31:46 +0100 Original-Received: from localhost ([::1]:36076 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDKzl-0006kd-1M for guile-devel@m.gmane.org; Wed, 06 Mar 2013 15:31:25 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:38078) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDKze-0006kI-HP for guile-devel@gnu.org; Wed, 06 Mar 2013 15:31:23 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDKzZ-0005n6-Tn for guile-devel@gnu.org; Wed, 06 Mar 2013 15:31:18 -0500 Original-Received: from world.peace.net ([96.39.62.75]:52885) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDKzZ-0005lC-Oh; Wed, 06 Mar 2013 15:31:13 -0500 Original-Received: from 209-6-91-212.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com ([209.6.91.212] helo=tines.lan) by world.peace.net with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1UDKzF-0007RT-GO; Wed, 06 Mar 2013 15:30:53 -0500 In-Reply-To: <87mwugd3o8.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Wed, 06 Mar 2013 15:05:27 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 96.39.62.75 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:15878 Archived-At: Hi Ludovic, ludo@gnu.org (Ludovic Court=C3=A8s) writes: > Which of these patches are needed for mini-gmp integration? It would > probably be easier to discuss things separately, by small chunks. Mini-gmp integration depends directly on patches 5 and 7, and those depend on patches 2, 3, and 4. > Overall I think I=E2=80=99m mostly incompetent on the numeric stuff, so I= =E2=80=99d > mostly comment on form. > > At first sight, it seems that there are few tests added, in particular > for the number printer. I think tests must be added along with the > patches that claim to fix something. Agreed. I'll work on that. >> From cd9784ed33d78e6647a752123bf7be91d65b5c96 Mon Sep 17 00:00:00 2001 >> From: Mark H Weaver >> Date: Sun, 3 Mar 2013 04:34:17 -0500 >> Subject: [PATCH 01/10] Improve code in scm_gcd for inum/inum case >> >> * libguile/numbers.c (scm_gcd): Improve implementation of inum/inum case >> to be more clear and efficient. > > This one looks OK, and should be covered by the tests, according to > . > > Did you measure the performance difference? Yes. On my x86_64 machine it improves gcd performance on fixnums by about 4.25%. I went ahead and pushed this one. > Can you fold the explanations as comments? [...] > Please use imperative-tense sentences above functions to describe what > they do, without repeating the function name; also make sure to > introduce the arguments in the description, written in capital letters > (info "(standards) Comments"). > > Some helper functions introduced by the patches lack a comment. Will do. >> +@deffn {Scheme Procedure} round-ash n count >> +@deffnx {C Function} scm_round_ash (n, count) >> +Return @math{round(@var{n} * 2^@var{count})}, but computed >> +more efficiently. @var{n} and @var{count} must be exact >> +integers. >> + >> +With @var{n} viewed as an infinite-precision twos-complement >> +integer, @code{round-ash} means a left shift introducing zero >> +bits when @var{count} is positive, or a right shift rounding >> +to the nearest integer (with ties going to the nearest even >> +integer) when @var{count} is negative. This is a rounded >> +``arithmetic'' shift. >> + >> +@lisp >> +(number->string (round-ash #b1 3) 2) @result{} \"1000\" >> +(number->string (round-ash #b1010 -1) 2) @result{} \"101\" >> +(number->string (round-ash #b1010 -2) 2) @result{} \"10\" >> +(number->string (round-ash #b1011 -2) 2) @result{} \"11\" >> +(number->string (round-ash #b1101 -2) 2) @result{} \"11\" >> +(number->string (round-ash #b1110 -2) 2) @result{} \"100\" >> +@end lisp >> +@end deffn > > What was the rationale for =E2=80=98round-ash=E2=80=99? Well, we need it internally to efficiently convert large integers to floating-point with proper rounding (see the call to 'round_right_shift_exact_integer' in patch 5). Given that, it seemed reasonable to export it to the user, since it's not possible to do that job efficiently with our current primitives. However, I don't feel strongly about it. > It seems to address to do two things differently from =E2=80=98ash=E2=80= =99: it=E2=80=99s more > efficient, and it rounds when right-shifting. The =E2=80=9Cmore efficien= t=E2=80=9D bit > is an implementation detail that should also benefit to =E2=80=98ash=E2= =80=99 (as a user > I don=E2=80=99t want to have to choose between efficient and non-rounding= .) No, 'round-ash' is not more efficient than 'ash'. It's more efficient than the equivalent (round (* n (expt 2 count))). Thanks for looking through the patches. I'll work on improving them. Mark