unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
Subject: Re: bookkeeping to prepare for a 64-bit EMACS_INT on 32-bit hosts
Date: Fri, 29 Apr 2011 13:04:07 -0300	[thread overview]
Message-ID: <jwvy62thxsh.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <4DBA7F87.5040609@cs.ucla.edu> (Paul Eggert's message of "Fri, 29 Apr 2011 02:06:15 -0700")

Let me first say that I'm not terribly excited by this idea of
a 32+64bit compilation option: ignoring the fact that many 32bit OSes do
not provide a full 4GB virtual address space to the Emacs process, such
a change can only bump the limit from 512MB to less than 4GB.
Of course, it will satisfy some particular uses, but it won't remove the
fundamental problem.

> The name "intptr" may be a bit confusing to a reader who doesn't
> know C99, but once you're used to the standard meaning, any other name
> would sound weird.

Makes me wonder: why use EMACS_INTPTR rather than intptr_t?

>>> > -  if (data != NULL && data == (void*) XHASH (QCdbus_session_bus))
>>> > +  if (data != NULL && data == (void *) XPNTR (QCdbus_session_bus))
>> I wonder if we aren't obfuscating the code a bit too much, since XHASH
>> says something about its argument, while XPNTR is too general to
>> convey any such useful information.  Unless, that is, you are saying
>> that the use of XHASH here was bogus to begin with, and all that was
>> needed was a pointer.
> Yes, that's what it's saying.  void * is supposed to hold only a
> pointer: in general it can't hold a pointer plus a tag.  So the
> old code wasn't correct.  (It was good enough for the current ports,
> but not good enough for a 32+64-bit port.)

The old code is correct all right: void* can hold a 32bit value, and
XHASH takes a 32bit value and returns a 32bit value (it's a perfect hash
function, in a sense).  OTOH XPNTR takes a 32bit value and returns
a 29bit value, so there's loss of information, whereas XHASH was
specially designed to return an integer value without any loss of
information.  And indeed, if the user passes the right Lisp integer at
the right place, the above new test may return true even though an
integer should clearly not be treated as equal to a symbol.

I.e. XPNTR should never be used on an INTEGERP value.  For other values
it's OK since they're all boxed as pointers, and the 29bit returned by
XPNTR has only thrown away redundant info (tho this redundancy may be
difficult to recover).

So it might be OK to replace XHASH with XPNTR, but only if you catch the
INTEGERP case beforehand.

> @@ -2144,7 +2144,7 @@
>         We used to use 0 here, but that leads to accidental sharing in
>         purecopy's hash-consing, so we use a (hopefully) unique integer
>         instead.  */
> -    docstring = make_number (XHASH (function));
> +    docstring = make_number (XPNTR (function));
>    return Ffset (function,
>  		Fpurecopy (list5 (Qautoload, file, docstring,
>  				  interactive, type)));

You lost me here.  make_number doesn't take a pointer as argument.
Even tho it's called "hash" it should not lose any information, so XHASH
is the right thing to use here, AFAICT.

>    if (wimage)
>      {
> -      /* The EMACS_INT cast avoids a warning. */
> +      EMACS_INTPTR ii = i;
> +      gpointer gi = (gpointer) ii;
> +

I saw various places where you do something similar.  Is there
a particular reason why you use an intermediate var rather than use the
more concise "(gpointer) (EMACS_INTPTR) i"?


        Stefan



  parent reply	other threads:[~2011-04-29 16:04 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-29  8:08 bookkeeping to prepare for a 64-bit EMACS_INT on 32-bit hosts Paul Eggert
2011-04-29  8:49 ` Eli Zaretskii
2011-04-29  9:06   ` Paul Eggert
2011-04-29  9:17     ` Eli Zaretskii
2011-04-29 16:04     ` Stefan Monnier [this message]
2011-04-29 17:10       ` Eli Zaretskii
2011-04-29 17:32         ` Lars Magne Ingebrigtsen
2011-04-29 17:45           ` Paul Eggert
2011-04-29 18:50         ` David De La Harpe Golden
2011-04-30  5:00           ` Stephen J. Turnbull
2011-04-30  1:37       ` Paul Eggert
2011-05-02 14:46         ` Stefan Monnier
2011-05-02 16:09           ` Paul Eggert
2011-05-02 18:12             ` Stefan Monnier
2011-05-02 19:23               ` Paul Eggert
2011-05-02 19:49                 ` Stefan Monnier
2011-04-30  6:54     ` Paul Eggert
2011-04-30  7:30       ` Eli Zaretskii
2011-04-29  8:56 ` support " Paul Eggert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwvy62thxsh.fsf-monnier+emacs@gnu.org \
    --to=monnier@iro.umontreal.ca \
    --cc=eggert@cs.ucla.edu \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).