Re: [Qemu-devel] [PATCH v2 0/4] softfloat: Fix build on Darwin (uint16)

[Blue Swirl]

On Wed, Apr 25, 2012 at 22:15, Andreas Färber <address@hidden> wrote:
> Hello,
>
> Since shortly before 1.0 the build on Mac OS X host is broken due to a type
> conflict for uint16 between SoftFloat and system headers. The conflict stems
> from SoftFloat using int for uint16 (i.e., 32 bits) whereas Apple wants 16 
> bits,
> and no correct workaround was found in time.
>
> Since these integer types also potentially conflict with those of BeOS/Haiku
> and kept creeping into others parts of code accidentally, we concluded that 
> the
> SoftFloat [u]intX types were doing more bad than good and should be replaced.
> Options considered were renaming them to unique qemu_[u]intX or reusing POSIX
> [u]int_leastX_t, [u]int_fastX_t or [u]intX_t.
>
> Renaming raised the question of why the types were defined as they are today.
> If they are intended to be performance-tuned then [u]int_fastX_t would allow
> to leave that responsibility to the OS.
>
> [u]int_leastX_t matches the described semantics of [u]intX but was discarded
> in favor of [u]int_fastX_t.
>
> That leaves a choice between [u]intX_t and [u]int_fastX_t.
>
> Having already introduced the use of [u]intX_t in an earlier series (bitsX),
> using [u]intX_t for these formerly separate types would make it hard to
> move from that to another type in case of problems.
> However, introducing [u]int_fast32_t might turn 32-bit types into 64-bit 
> types,
> unexpected by code that may be misusing these types (missing masks/casts).
>
> This series therefore strips down my RFC series to only replace uint16, where
> Mac OS X users are seeing problems, and for symmetry int16. Peter and me
> consider this limited change safe for 1.1 since a) sizeof(uint16) != 2 already
> and b) uint16 is signed, without bug reports all those years.
>
> (i) uint16: signed int -> unsigned short / int / long / long long depending 
> on host
> (ii) int16: signed int ->   signed short / int / long / long long depending 
> on host
>
> That would fix the reported build problem and leave the door open to sed the
> types to something more performant or smaller at a later point in time.
>
> As before, constructive feedback and in particular...
> * tests on the various platforms,
> * benchmark results that show performance improving (or not significantly
>  degrading), as well as
> * test suites beyond ARM guests to make sure we introduce no regressions
> ...for the remaining types from the previous RFC series (especially [u]int32)
> are much more appreciated than reposting old hacky workarounds, which I will
> continue to nack.

Thanks, applied all.

>
> Regards,
> Andreas
>
> Cc: Peter Maydell <address@hidden>
> Cc: Bernhard Walle <address@hidden>
>
> v1 -> v2:
> * Change comment moved from target-mips to reflect further types being added.
>  (suggested by Peter)
> * Drop all non-essential patches that need further testing/benchmarking.
>
> Andreas Färber (4):
>  softfloat: Fix mixups of int and int16
>  target-mips: Move definition of uint_fast{8,16}_t to osdep.h
>  softfloat: Replace uint16 type with uint_fast16_t
>  softfloat: Replace int16 type with int_fast16_t
>
>  fpu/softfloat-macros.h |   18 ++++----
>  fpu/softfloat.c        |  122 
> ++++++++++++++++++++++++------------------------
>  fpu/softfloat.h        |   13 ++---
>  osdep.h                |    7 +++
>  target-mips/cpu.h      |    7 ---
>  5 files changed, 82 insertions(+), 85 deletions(-)
>
> --
> 1.7.7
>
>