Am 18.12.2010 um 13:15 schrieb Peter Maydell:
On 18 December 2010 11:49, Andreas Färber <andreas.faer...@web.de>
wrote:
IMO a lot of code in QEMU is cryptic because someone thinks that
someone
else must've thought something particular when doing it that way
and is thus
reluctant to touch it...
For a fact, [u]int8 und [u]int64 remain unchanged width-wise.
For [u]int16, only malc may know what that maps to on AIX, for
which they
are #ifndef'ed out. I doubt it's an int.
Unless there's an ILP64 platform we support, [u]int32 would stay
the same
width, too.
That's why I was saying, putting, e.g., a 33rd bit into int32 has
undefined
semantics, just as for the POSIX int_least32_t type. I don't see a
win in
declaring that information.
It's saying "it's OK to provide more than 32 bits if that would be
faster",
and indeed that's exactly how we typedef it. Some parts of softfloat
that use bits32 do rely on there being exactly 32 bits and not 64.
Okay, then I'll use int_least*_t instead. Performance is always a good
argument. :)
My patch tries to do three things in one:
1.) Fix mismatches between headers and sources, i.e. float32
int32_to_float32(int); vs. float32 int32_to_float32(int32) {...} etc.
In this case we do know the rationale for this mismatch:
http://www.jhauser.us/arithmetic/SoftFloat-2b/SoftFloat-source.txt
---begin---
Unlike the actual function definitions in `softfloat.c', the
declarations
in `softfloat.h' do not use any of the types defined by the
`processors'
header file. This is done so that clients will not have to include
the
`processors' header file in order to use SoftFloat. Nevertheless, the
target-specific declarations in `softfloat.h' must match what
`softfloat.c'
expects. For example, if `int32' is defined as `int' in the
`processors'
header file, then in `softfloat.h' the output of `float32_to_int32'
should
be stated as `int', although in `softfloat.c' it is given in target-
independent form as `int32'.
---endit---
...which doesn't really apply to the way qemu has taken softfloat,
so I agree it makes sense to change things here.
So, how far are we from the original? Given that the latest version is
from 2002, would it make any sense to move this to a Git submodule?
2.) Drop the unnecessary custom integer types in favor of standard
ones.
...but it doesn't do this because it isn't touching the 'bits32'
types which
are the ones which are exact synonyms for the posix int32_t &c.
If your aim is to remove unnecessary custom types this is the first
and
easiest target because you can do it as a pure search-and-replace.
OK, will do.
3.) Fix instances of lazyness where _t was forgotten and the
mistake was
hidden by the softfloat typedefs.
Renaming int32 to qint32 would defeat the second purpose. I got
around the
Haiku issue for now, so that's not a pressing need.
Had the softfloat code not been a real refactoring-unfriendly mess
of int,
int* and int*_t, I would've offered to do this in multiple steps
per type. I
could try splitting out part 1 above. Part 3 can easily be split
off by
cut-and-paste and could be applied independently.
I certainly think it would be easier to review as separate patches
which
did different things.
Promoting int16[_t] to int for things like shift counts is beyond
the scope
of my patch.
...but your patch changes what is currently an 'int' (hidden behind
the int16
typedef) to int16_t, so it is making a change here. It's a safe
change but
it doesn't actually make any sense.
No, it's an implementation detail and is only the way you describe
outside AIX currently. It may well be int16_t already there.
And yes, it doesn't make sense, but that's not my fault, it's the
author's use of int16 that's unnecessary from the start.
Andreas
