From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: Windows 64 port Date: Thu, 01 Mar 2012 00:58:02 -0800 Organization: UCLA Computer Science Department Message-ID: <4F4F3A1A.4020808@cs.ucla.edu> References: <20120219211800.0000558f@unknown> <834numv7js.fsf@gnu.org> <83ty2ltep0.fsf@gnu.org> <4F4EEBC2.5070704@cs.ucla.edu> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1330592312 24126 80.91.229.3 (1 Mar 2012 08:58:32 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Thu, 1 Mar 2012 08:58:32 +0000 (UTC) Cc: Eli Zaretskii , ajmr@ilovetortilladepatatas.com, emacs-devel@gnu.org To: Fabrice Popineau Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Mar 01 09:58:29 2012 Return-path: Envelope-to: ged-emacs-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 1S31qD-0005W6-I4 for ged-emacs-devel@m.gmane.org; Thu, 01 Mar 2012 09:58:26 +0100 Original-Received: from localhost ([::1]:46856 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S31qD-0004pb-27 for ged-emacs-devel@m.gmane.org; Thu, 01 Mar 2012 03:58:25 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:40281) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S31q5-0004oD-35 for emacs-devel@gnu.org; Thu, 01 Mar 2012 03:58:23 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S31py-0003t0-LU for emacs-devel@gnu.org; Thu, 01 Mar 2012 03:58:16 -0500 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:40501) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S31ps-0003pT-AE; Thu, 01 Mar 2012 03:58:04 -0500 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 478D1A6000A; Thu, 1 Mar 2012 00:58:00 -0800 (PST) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3DbfAlTZQcm7; Thu, 1 Mar 2012 00:57:59 -0800 (PST) Original-Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 8944DA60004; Thu, 1 Mar 2012 00:57:59 -0800 (PST) User-Agent: Mozilla/5.0 (X11; Linux i686; rv:10.0.2) Gecko/20120216 Thunderbird/10.0.2 In-Reply-To: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 131.179.128.62 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:148852 Archived-At: On 02/29/2012 11:24 PM, Fabrice Popineau wrote: > For example, src/s/ms-w32.h can do something like this: > > /* Include before defining tzname, to avoid DLL clash. */ > #include > #define tzname _tzname > > This should avoid the diagnostic, since tzname isn't defined until > after is included. > What you propose didn't work (resulting in either : some function > returns an array, that is tzname, or _tzname being undefined at link > time). Sorry, I don't follow. What exactly happens when you try the above suggestion, and why? And what does this have to do with functions returning arrays? Perhaps you can explain, by showing the preprocessor output of the failed attempt. > the definition needs to be put somewhere because an int is not the same > size as a pointer in windows 64. OK, thanks, it appears you fixed that in a different way in the last patch, so the point is moot. > unless the source code is fixed to avois the warnings, > it gets me a cleaner compilation listing. Surely there's some way to tell the compiler to not issue the warnings. Or you can simply ignore these bogus messages. > Here is a more-detailed review of the latest patch you sent. > > > - int i = 0, aligned = (intptr_t) ABLOCKS_BUSY (abase); > > + int i = 0, aligned = (ABLOCKS_BUSY (abase) != NULL); > > Not needed for Windows 64. The change slows the code down a bit, and > > > Prove it. I compiled it both ways (x86-64 GCC 4.6.2), and the "!= NULL" version has an extra "cmpl" instruction, so it is indeed a bit fatter. > it might not work on unusual hosts that have filler bits in their > pointers. > > The it is because the compiler is buggy, because this is a perfectly legal and much more > right than the double cascading cast (one explicit, one implicit). Sorry, it sounds like my suggestion wasn't clear. On some architectures, pointers have unused bits, there are multiple representations of NULL, and the test for whether a pointer is NULL is not the same as testing whether its bit-pattern is all zeros. Compilers on such systems are not buggy: they are simply implementing the machine's architecture. Admittedly this point is probably theoretical. As far as I know among Emacs targets only the s390 architecture has unused bits in its pointers, and I don't know whether its comparisons to NULL ignore those bits. Still, the point remains that one must be careful when making changes like this, and there's no need for this particular change for the Windows 64 port. > Not needed for Windows 64, since the arguments are all in int range. > > Who knows that ? Certainly not the compiler. *You* know it, because I just told you. (:-) I've read the code. Similarly for your other remarks about the compiler not knowing things. In all these cases, the compiler may not be smart enough to figure things out, but the values are indeed in range and there's no need to alter the types. > > - make_gap (-(nextb->text->gap_size - size)); > > + make_gap ((size - nextb->text->gap_size)); > > Not needed for Windows 64, since the expressions are equivalent. > > Depends on signed/unsigned. On Windows 64 the two expressions are equivalent, so there's no need for this patch in the Windows 64 port. > > - unsigned long int adj; > > + intptr_t adj; > > Not needed for Windows 64; the value is always in unsigned long range. > > I thank you for all those intptr_t -> uintptr_t catches. You're welcome, but this is not an example of that. The declaration does not need to be changed, because the value of 'adj' is always in the range 0..ULONG_MAX, on Windows 64 and on mainline hosts. > > +#ifdef min > > +#undef min > > +#endif > > Just because it is redefined when it is a commonly defined macro in > system headers. Which system header defined it, and what exactly was it defined to? If a system header defines 'min' it may not be wise to change it. > > - unsigned long int magic; /* Magic number to check header integrity. */ > > + uint64_t magic; /* Magic number to check header integrity. */ > > Not needed for Windows 64, since the magic number fits in 32 bits. > > If it is a 32bits integer, then better make it this way everywhere. Perhaps -- but then why use uint64_t for a 32-bit quantity? And anyway, the patch is not needed for Windows 64, regardless of what type (if any) is substituted for unsigned long. > I'm not even sure why there is a __malloc_ptrdiff_t at all. It's a relic of the older pre-C89 environment. These days we should just remove __malloc_ptrdiff_t and use ptrdiff_t. (But as part of a separate patch; it's not needed for Windows 64.)