From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#65491: [PATCH] Improve performance allocating vectors Date: Sun, 17 Sep 2023 08:18:41 +0300 Message-ID: <83led5gyxa.fsf@gnu.org> References: <6B2EDD07-AAEB-40E8-B369-F634296BD3D9@gmail.com> <83v8cagkqv.fsf@gnu.org> <83ttrugkj2.fsf@gnu.org> <83r0mygi4y.fsf@gnu.org> <8af6fa1c-4873-bd6e-e896-ab5bb8d012a2@cs.ucla.edu> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="19159"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 65491@debbugs.gnu.org, mattias.engdegard@gmail.com, monnier@iro.umontreal.ca To: Paul Eggert Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Sep 17 07:20:28 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qhkCp-0004pP-5M for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 17 Sep 2023 07:20:27 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qhkCK-0005gM-72; Sun, 17 Sep 2023 01:19:56 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qhkCJ-0005g2-Iv for bug-gnu-emacs@gnu.org; Sun, 17 Sep 2023 01:19:55 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qhkCJ-0007rW-AZ for bug-gnu-emacs@gnu.org; Sun, 17 Sep 2023 01:19:55 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qhkCQ-00048T-BS for bug-gnu-emacs@gnu.org; Sun, 17 Sep 2023 01:20:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 17 Sep 2023 05:20:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 65491 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 65491-submit@debbugs.gnu.org id=B65491.169492794415816 (code B ref 65491); Sun, 17 Sep 2023 05:20:02 +0000 Original-Received: (at 65491) by debbugs.gnu.org; 17 Sep 2023 05:19:04 +0000 Original-Received: from localhost ([127.0.0.1]:48866 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qhkBT-000470-Qt for submit@debbugs.gnu.org; Sun, 17 Sep 2023 01:19:04 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45284) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qhkBP-00046V-OU for 65491@debbugs.gnu.org; Sun, 17 Sep 2023 01:19:02 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qhkBB-0007c4-Rj; Sun, 17 Sep 2023 01:18:45 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=sb/x9mAO21Q1ThZAEyJwerAp4dBQ0A/zA8pioBj08Q8=; b=PQpUNl3vxp8p YA2XnckD2a3PC0fH6/v0lYtSh//xURN90PqOziwIYeKxL/thDt/9/HUTnx5QyYS6DStYw8lOvsTIe wBVF2giPm5nt0T5vCNXPdHsaqqdDqNp4ReLUmr4RufiAlSD1FL1KrShxJj+97PP+jv2XwGACBvhcS OZ7a71ncO4YiwKlWzV9MC8LzfwRIRf4xMg/2xmK0M84dPT8AhspBBE6IyxaTEhSJhpHnO0bKmrVc/ 92lBNwKI4PtllSeC0/00gJxNSVT9KEHv8UvWkWivOVxn2DFgao3yanE00r+QLa8bcg+KGsGCPobmR Kep8RrMzL7Zqk4iQFXeHEA==; In-Reply-To: <8af6fa1c-4873-bd6e-e896-ab5bb8d012a2@cs.ucla.edu> (message from Paul Eggert on Sat, 16 Sep 2023 12:46:34 -0700) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:270663 Archived-At: > Date: Sat, 16 Sep 2023 12:46:34 -0700 > Cc: 65491@debbugs.gnu.org, monnier@iro.umontreal.ca > From: Paul Eggert > > On 2023-09-16 10:09, Eli Zaretskii wrote: > > > I also added Paul to the discussion, since he wrote most of the > > involved macros. > > Mattias and I had a private email discussion about XUNTAG last month, > and he's correct that Emacs's current code, which #defines XUNTAG(a, > type, ctype) to ((ctype *) ((char *) XLP (a) - LISP_WORD_TAG (type))), > violates the C standard due to calculating a pointer outside its > containing object's bounds, whereas the patch P that you just reverted, > which defines the same macro to ((ctype *) ((uintptr_t) XLP (a) - > (uintptr_t) LISP_WORD_TAG (type))), does not have that particular > undefined behavior. First, may I ask that in the future such discussions be held on the list, or at least their summary be posted? You can see right here and now how this practice of discussing purely technical issues in private causes unnecessary friction and aggravation, let alone breakage. Next, even if this was discussed, due to our experience with changes in this area, it would have been prudent to post the proposed patch before installing it, so that it could be tested in all the supported configuration before risking such a fundamental breakage of the master branch. You personally do that all the time, and for good reasons, but Mattias has yet to learn that, it seems. More to the point: can you explain how this part ((uintptr_t) XLP (a) - (uintptr_t) LISP_WORD_TAG (type)) works in a 32-bit build --with-wide-int? AFAIU, XLP(a) yields a 32-bit value, and LISP_WORD_TAG has all of its low 32 bits zero, so why do we need the subtraction at all? Aren't we subtracting a zero from what XLP yields? It seems to me that in a 32-bit build with wide ints just #define XUNTAG(a, type, ctype) ((ctype *) XLP (a)) should be enough, since XLP yields a 'void *', no? > I would not favor a two-pronged approach, in which patch P is present > but is used optionally. This would likely cause more trouble than it'd > cure, due to lack of testing of the extra combinations. Let's use just > one approach and keep it simple and more testable. Not sure what you are alluding to here, but if by "two-pronged approach" you mean my suggestion to use cpp conditionals, then what I meant was to have a separate definition of XUNTAG for 32-bit builds with wide ints (which could still remove the undefined behavior), just without all the juggling of integer-to-pointer-and-back conversions, which completely obscure what should be a simple operation of extracting a pointer from an integer by masking some of its bits. Why do I need to fight with these macros each time I read this code, when the job it does is so simple? Thanks.