From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38219 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PmYN8-0007nF-I7 for qemu-devel@nongnu.org; Mon, 07 Feb 2011 16:11:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PmVe7-0003cZ-7m for qemu-devel@nongnu.org; Mon, 07 Feb 2011 13:17:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52533) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PmVe6-0003cM-P0 for qemu-devel@nongnu.org; Mon, 07 Feb 2011 13:17:07 -0500 From: Markus Armbruster Subject: Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *)) References: <4D4D612F.2010904@mail.berlios.de> <4D4DC570.8080204@mail.berlios.de> <4D503132.3010807@mail.berlios.de> Date: Mon, 07 Feb 2011 19:16:46 +0100 In-Reply-To: <4D503132.3010807@mail.berlios.de> (Stefan Weil's message of "Mon, 07 Feb 2011 18:51:46 +0100") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: Blue Swirl , QEMU Developers Stefan Weil writes: > Am 07.02.2011 08:23, schrieb Markus Armbruster: >> Stefan Weil writes: >>> Am 05.02.2011 16:35, schrieb Blue Swirl: >> [...] >>>> The patch changes also signed longs to uintptr_t. That could introduce >>>> regressions, so please use signed/unsigned as original. >>> >>> I changed the code manually, and there was only one location where >>> signed/unsigned made a difference. That single case was an int >>> parameter passed in a void pointer, and I used intptr_t there. >>> >>> I had the impression that in the current code (long) was >>> sometimes used because it is shorter than (unsigned long) :-) >>> >>> As long as changes are made manually with the necessary care, >>> I'd recommend using uintptr_t where possible. >> >> I'd recommend not to mix up the intptr portability clean up with the >> signedness cleanup. Much easier to review separately. Moreover, >> cleaning up signedness changes generated code, while cleaning up the >> types shouldn't (except on hosts where the code doesn't work). >> Testable, just don't forget to strip the debug info. >> >> [...] > > > > Markus, your recommendation is ok for modifications which change the > generated code or which need more context for the review. > > I don't think that you will have any problem with reviewing > "signedness changes" like these ones: > > -#define saddr(x) (uint8_t *)(long)(x) > -#define laddr(x) (uint8_t *)(long)(x) > +#define saddr(x) (uint8_t *)(uintptr_t)(x) > +#define laddr(x) (uint8_t *)(uintptr_t)(x) > > Neither of these changes changes the binary code for the commonly used > hosts, > and the patch does not need further context for the review. > > I intend to split my patch in three parts: > > * one for tcg_gen_exit_tb calls which will be modified as Blue Swirl > has suggested > * one for the parameter passing of a signed value via pointer > * one for the rest which contains only a handful of trivial > "signedness changes", > all following the same pattern like the example given above > > Is that ok? Let's see the patches :)