[Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
On 02/11/2011 06:05 AM, Rob Landley wrote: While this assumption works on QEMU's major hosts, it is not generally true. It is generally true. There is exactly one operating system that decided to go its own way, and the insane legacy reasons they did so are explained here: http://blogs.msdn.com/oldnewthing/archive/2005/01/31/363790.aspx Unix could do that because it had the luxury of having introduced 64-bit when they already were using int=long=32. So really nobody was using long until 64-bit systems came along. Windows instead has to deal with the legacy of 16-bit, when long was the only 32-bit type. I have always agreed with you, but as much as I like LP64, I recently changed my mind on this stance. stdint.h means that there is _no reason_ why a program cannot be written portably so that it runs on both I32LP64 and IL32LLP64 models. Someone has to do the work, of course, and it's surprising that two people (Filip Navara and now Stefan) were brave enough to try it. :) It has to be a well-audited change though, not a quick attempt at making it work. Paolo ps: HP-UX also uses IL32 on ia64. Now _that_ is hard to understand.
Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
On Feb 11, 2011, at 1:47 PM, Paolo Bonzini wrote: ps: HP-UX also uses IL32 on ia64. Now _that_ is hard to understand. Backward compatibility with hppa... VMS also uses IL32 on alpha and ia64, but it has both P32 and P64.
Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
On Fri, Feb 11, 2011 at 2:47 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 02/11/2011 06:05 AM, Rob Landley wrote: While this assumption works on QEMU's major hosts, it is not generally true. It is generally true. There is exactly one operating system that decided to go its own way, and the insane legacy reasons they did so are explained here: http://blogs.msdn.com/oldnewthing/archive/2005/01/31/363790.aspx Unix could do that because it had the luxury of having introduced 64-bit when they already were using int=long=32. So really nobody was using long until 64-bit systems came along. Windows instead has to deal with the legacy of 16-bit, when long was the only 32-bit type. IIRC also Unix was in that situation once (short = int =16, long = 32 bits). I have always agreed with you, but as much as I like LP64, I recently changed my mind on this stance. stdint.h means that there is _no reason_ why a program cannot be written portably so that it runs on both I32LP64 and IL32LLP64 models. Using intptr_t is not different from using long. There's also the advantage that it is a bit more specific. Someone has to do the work, of course, and it's surprising that two people (Filip Navara and now Stefan) were brave enough to try it. :) It has to be a well-audited change though, not a quick attempt at making it work. I'd still be interested to know if QEMU runs on win64. But even if it doesn't, changing longs to intptr_t and unsigned longs to uintptr_t is harmless enough that it should be applied nevertheless. Even if everybody stopped all win32/64 work after that, nothing would be lost except maybe some beauty in some beholder's eyes.
Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
On Fri, 11 Feb 2011, Blue Swirl wrote: On Fri, Feb 11, 2011 at 2:47 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 02/11/2011 06:05 AM, Rob Landley wrote: While this assumption works on QEMU's major hosts, it is not generally true. It is generally true. There is exactly one operating system that decided to go its own way, and the insane legacy reasons they did so are explained here: http://blogs.msdn.com/oldnewthing/archive/2005/01/31/363790.aspx Unix could do that because it had the luxury of having introduced 64-bit when they already were using int=long=32. So really nobody was using long until 64-bit systems came along. Windows instead has to deal with the legacy of 16-bit, when long was the only 32-bit type. IIRC also Unix was in that situation once (short = int =16, long = 32 bits). I have always agreed with you, but as much as I like LP64, I recently changed my mind on this stance. stdint.h means that there is _no reason_ why a program cannot be written portably so that it runs on both I32LP64 and IL32LLP64 models. Using intptr_t is not different from using long. There's also the advantage that it is a bit more specific. Someone has to do the work, of course, and it's surprising that two people (Filip Navara and now Stefan) were brave enough to try it. :) It has to be a well-audited change though, not a quick attempt at making it work. I'd still be interested to know if QEMU runs on win64. But even if it doesn't, changing longs to intptr_t and unsigned longs to uintptr_t is harmless enough that it should be applied nevertheless. Even if everybody stopped all win32/64 work after that, nothing would be lost except maybe some beauty in some beholder's eyes. Filips port did run on win64 last time i tried. -- mailto:av1...@comtv.ru
Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
Stefan Weil w...@mail.berlios.de writes: Am 07.02.2011 08:23, schrieb Markus Armbruster: Stefan Weil w...@mail.berlios.de 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 :)
Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
Am 07.02.2011 08:23, schrieb Markus Armbruster: Stefan Weil w...@mail.berlios.de 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? Regards, Stefan
Re: [Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
Stefan Weil w...@mail.berlios.de 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. [...]
[Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
On Sat, Feb 5, 2011 at 2:39 PM, Stefan Weil w...@mail.berlios.de wrote: Currently, most QEMU code assumes that pointers and long integers have the same size, typically 32 bit on 32 bit hosts, 64 bit on 64 bit hosts. While this assumption works on QEMU's major hosts, it is not generally true. There exist 64 bit host OS which use an ABI with 32 bit long integers, maybe to be more compatible with an older 32 bit OS version, so here is sizeof(long) sizeof(void *). Oh, that OS-Which-Must-Not-Be-Named. Other ABIs might use near pointers which may reduce code size and improve code speed. This results in sizeof(long) sizeof(void *). Both cases will break current QEMU, because lots of code lines use type casts from pointer to long or vice versa like these two examples: start = (long)mmap((void *)host_start, host_len ... code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + ...)) Both variants (unsigned long) and (long) can be found (relation 3:2). Changing the existing limitation of QEMU's code simply needs replacing all those type casts, variable declarations and printf format specifiers by portable code. The standard integral type which matches the size of a pointer is defined in stdint.h (which also defines int8_t, ...). It is intptr_t (signed version) or uintptr_t (unsigned version). There is no need to use both. = Replace (unsigned long), (long) type casts of pointers by (uintptr_t). All variables (auto, struct members, parameters) which hold such values must be fixed, too. In the following examples, ptr_var is of that kind. = Replace unsigned long ptr_var, long ptr_var by uintptr_t ptr_var. Finally, the fixed variables are used in printf-like statements, so here the format specifier needs a change. inttypes.h defines PRIxPTR which should be used. = Replace %lx by % PRIxPTR to print the integer value ptr_var. A single patch which includes all these changes touches 39 files. Splitting it into smaller patches is not trivial because of cross dependencies. Because of its size, it will raise merge conflicts when it is not applied soon. Would these kind of changes be interesting for QEMU? Does QEMU work in OS-Which-Must-Not-Be-Named when the patch is applied? Are there suggestions how it should be done? The change, done properly, should not cause any problems for most other hosts, where unsigned long size equals pointer type size. What about ram_addr_t? Should it be replaced by uintptr_t? I'd use typedef uintptr_t ram_addr_t; Should we use macros like QEMU_PTR2UINT(p), QEMU_UINT2PTR(u)? Rather, inline functions if open coding is not clear. My current version of the patch is available from http://qemu.weilnetz.de/0001-Fix-conversions-from-pointer-to-integral-type-and-vi.patch. Some comments: The patch changes also signed longs to uintptr_t. That could introduce regressions, so please use signed/unsigned as original. For example in cpu_physical_memory_reset_dirty() you didn't change length, but that should probably become uintptr_t too. */translate.c: exit_tb() helper uses tcg_target_long, so the cast should use that instead.
[Qemu-devel] Re: Porting QEMU to new hosts with unusual ABI (sizeof(long) != sizeof(void *))
Am 05.02.2011 16:35, schrieb Blue Swirl: On Sat, Feb 5, 2011 at 2:39 PM, Stefan Weil w...@mail.berlios.de wrote: Currently, most QEMU code assumes that pointers and long integers have the same size, typically 32 bit on 32 bit hosts, 64 bit on 64 bit hosts. While this assumption works on QEMU's major hosts, it is not generally true. There exist 64 bit host OS which use an ABI with 32 bit long integers, maybe to be more compatible with an older 32 bit OS version, so here is sizeof(long) sizeof(void *). Oh, that OS-Which-Must-Not-Be-Named. Right. Other ABIs might use near pointers which may reduce code size and improve code speed. This results in sizeof(long) sizeof(void *). Both cases will break current QEMU, because lots of code lines use type casts from pointer to long or vice versa like these two examples: start = (long)mmap((void *)host_start, host_len ... code_gen_ptr = (void *)(((unsigned long)code_gen_ptr + ...)) Both variants (unsigned long) and (long) can be found (relation 3:2). Changing the existing limitation of QEMU's code simply needs replacing all those type casts, variable declarations and printf format specifiers by portable code. The standard integral type which matches the size of a pointer is defined in stdint.h (which also defines int8_t, ...). It is intptr_t (signed version) or uintptr_t (unsigned version). There is no need to use both. = Replace (unsigned long), (long) type casts of pointers by (uintptr_t). All variables (auto, struct members, parameters) which hold such values must be fixed, too. In the following examples, ptr_var is of that kind. = Replace unsigned long ptr_var, long ptr_var by uintptr_t ptr_var. Finally, the fixed variables are used in printf-like statements, so here the format specifier needs a change. inttypes.h defines PRIxPTR which should be used. = Replace %lx by % PRIxPTR to print the integer value ptr_var. A single patch which includes all these changes touches 39 files. Splitting it into smaller patches is not trivial because of cross dependencies. Because of its size, it will raise merge conflicts when it is not applied soon. Would these kind of changes be interesting for QEMU? Does QEMU work in OS-Which-Must-Not-Be-Named when the patch is applied? Up to now, I never used OS-Which-Must-Not-Be-Named (64 bit) :-) The patch allows to build executables (which I have put on my web page). I noticed two remaining problems with time_t and GetProcessAffinityMask. Maybe these are not critical, but of course they have to be fixed, too. Are there suggestions how it should be done? The change, done properly, should not cause any problems for most other hosts, where unsigned long size equals pointer type size. That's correct. The risk of breaking current hosts is minimal. What about ram_addr_t? Should it be replaced by uintptr_t? I'd use typedef uintptr_t ram_addr_t; So did I. We could go further and eliminate ram_addr_t. Should we use macros like QEMU_PTR2UINT(p), QEMU_UINT2PTR(u)? Rather, inline functions if open coding is not clear. My current version of the patch is available from http://qemu.weilnetz.de/0001-Fix-conversions-from-pointer-to-integral-type-and-vi.patch. Some comments: 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. For example in cpu_physical_memory_reset_dirty() you didn't change length, but that should probably become uintptr_t too. Yes, that would be better (even if dirty memory ranges 4 GiB are not common). I'll change that. */translate.c: exit_tb() helper uses tcg_target_long, so the cast should use that instead. Obviously tcg_target_long has the same size as uintptr_t, so I can change this, too. Regards, Stefan
