From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Change module interface to no longer use GMP objects directly. Date: Mon, 18 Nov 2019 13:21:53 -0800 Organization: UCLA Computer Science Department Message-ID: <089f3d06-e227-27da-c8fe-afcbbbbc934a@cs.ucla.edu> References: <20191117183828.82379-1-phst@google.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------5382E6DFB2B0F89FEEF2CA99" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="243274"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0 Cc: Philipp Stephani To: Philipp Stephani , emacs-devel@gnu.org, monnier@iro.umontreal.ca Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Nov 18 22:26:38 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1iWoXe-0011AP-KJ for ged-emacs-devel@m.gmane.org; Mon, 18 Nov 2019 22:26:38 +0100 Original-Received: from localhost ([::1]:39730 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iWoXd-0003mY-Ed for ged-emacs-devel@m.gmane.org; Mon, 18 Nov 2019 16:26:37 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36862) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iWoTC-0007aB-3P for emacs-devel@gnu.org; Mon, 18 Nov 2019 16:22:03 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iWoTA-0001dp-1X for emacs-devel@gnu.org; Mon, 18 Nov 2019 16:22:01 -0500 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:54846) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iWoT8-0001cU-EU for emacs-devel@gnu.org; Mon, 18 Nov 2019 16:21:59 -0500 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 519DD1600CC; Mon, 18 Nov 2019 13:21:55 -0800 (PST) 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 5YGsdI5uOLPF; Mon, 18 Nov 2019 13:21:54 -0800 (PST) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 6757B16022E; Mon, 18 Nov 2019 13:21:54 -0800 (PST) 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 7a5_MUq_y027; Mon, 18 Nov 2019 13:21:54 -0800 (PST) Original-Received: from Penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 463911600CC; Mon, 18 Nov 2019 13:21:54 -0800 (PST) In-Reply-To: <20191117183828.82379-1-phst@google.com> Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 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:242400 Archived-At: This is a multi-part message in MIME format. --------------5382E6DFB2B0F89FEEF2CA99 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Thanks. I like the idea of decoupling Emacs modules from GMP. The proposed approach will typically require copying bignum data twice to do a conversion, even if modules use GMP themselves. Shouldn't there be a shortcut for modules using GMP, so that they can use mpz_limbs_write and mpz_limbs_finish and bypass one of the copies? We can do that even if the module interface itself doesn't include gmp.h or assume GMP. For example, the Emacs module interface could define a type emacs_limb_t that happens to be the same size and representation as mpz_limb_t when Emacs is implemented via GMP, and the example function could look like the attached handwavy file "bigt.c". (It might be possible to avoid even the remaining copy in some cases; I didn't investigate this.) Without such a shortcut, then we are to some extent giving up on efficiency, and in that case shouldn't we just ask people to convert to text and back, thus simplifying the interface? On 11/17/19 10:38 AM, Philipp Stephani wrote: > +@deftypefn Function bool extract_big_integer (emacs_env *@var{env}, emacs_value @var{arg}, int *@var{sign}, ptrdiff_t *@var{count}, unsigned long *@var{magnitude}) > +This function, which is available since Emacs 27, Should we be more systematic about saying which Emacs versions support which parts of the module API? Uses of 'unsigned long' should be changed to a more-abstract integer type like 'emacs_limb_t', to give the implementation more leeway to change in future versions. The extraction API looks a bit complicated. How about if we simplify it so that it (1) never signals an error and (2) it returns the limb count instead of always returning 'true' and storing the limb count via a pointer. Any too-small array can be reported merely by returning a limb count larger than the array size, without changing the array, as in the attached sample code. > extracts the > +integral value of @var{arg}. Need to make it clearer that ARG can be any integer; it need not be a bignum. > it stores the required array > +size into @code{*count} It should document that this size cannot exceed (min (PTRDIFF_MAX, SIZE_MAX) / sizeof (emacs_limb_t)). No matter what we do inside Emacs, it won't exceed that value for various reasons, and documenting this can simplify callers. > + bool success = env->extract_big_integer (env, value, &sign, &count, NULL); Shouldn't that 'value' be 'arg'? > + mpz_import (value, count, order, size, endian, nails, magnitude); Shouldn't that 'value' be 'result'? At this point I stopped detailed review because I wanted your thoughts on the bigger questions. --------------5382E6DFB2B0F89FEEF2CA99 Content-Type: text/x-csrc; charset=UTF-8; name="bigt.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="bigt.c" static bool extract_big_integer (emacs_env *env, emacs_value arg, mpz_t result) { int sign; unsigned long u; ptrdiff_t count = env->extract_big_integer (env, arg, &sign, 1, &u); if (count <= 1) mpz_set_ui (result, u); else { emacs_limb_t *magnitude = _Generic (*mpz_limbs_write (result, count), emacs_limb_t: mpz_limbs_write (result, count), default: checked_malloc (count * sizeof *magnitude)); env->extract_big_integer (env, arg, &sign, count, magnitude); if (_Generic (*mpz_limbs_write (result, count), emacs_limb_t: (mpz_limbs_finish (result, sign < 0 ? -count : count), true), default: false)) return true; mpz_import (result, count, -1, sizeof *magnitude, 0, 0, magnitude); free (magnitude); } if (sign < 0) mpz_neg (result, result); return true; } --------------5382E6DFB2B0F89FEEF2CA99--