[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/6] Get rid of confusing softfloat-specific int
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH 0/6] Get rid of confusing softfloat-specific integer types |
Date: |
Wed, 13 Jan 2016 16:59:46 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On 2016-01-12 12:55, Peter Maydell wrote:
> This patchset removes the confusing softfloat-specific integer
> types int8, uint8, int32, uint32, int64 and uint64, replacing
> them with the standard _t types that they were typedef'd as.
> These frequently got accidentally used outside the softfloat
> code as a simple typo for the standard types (as you can see
> from the various files touched in the diffstat).
>
> Although there is technically a semantic difference (the
> softfloat types are "at least X bits" whereas the standard
> types are "exactly X bits", the distinction is unlikely to
> make much performance difference and "upgrading" the types to
> use int_fast*_t would require careful code analysis to check we
> weren't accidentally relying on the type width. It also means
> we might potentially have subtle bugs on only some host platforms,
> which is worth avoiding I think.
>
> (In particular glibc defines int_fast32_t as a 64 bit type
> on 64 bit systems, which is unlikely to be the most sensible
> type to actually use for performance. I was reading a discussion
> about the _fast_ types from the musl irc channel recently:
> https://gist.github.com/andrewrk/ac66b24a0a202d87cea7
> which suggests that they're in practice not very useful.)
Thanks for doing this change. I hope this time we'll reach a consensus.
> This is admittedly a different decision to the one we made in
> the past for int16/uint16 (commits 94a49d86c536af3, 5aea4c589aa).
> I can do a followup patch which converts our int_fast16_t/uint_fast16_t
> usage to int16_t/uint16_t if people would like.
> (I think the difference is partly that the old int16/uint16 types
> really were bigger than 16 bits so we knew the code was not
> accidentally relying on exactly-16-bitness. Also I have a feeling
> that I was one of those suggesting the _fast_ types, but I have
> changed my mind and think I was wrong back then.)
Yes please it would be nice if we can use standard consistent type
everywhere.
> I have left the 'flag' type alone. This could reasonably be changed
> to 'bool' if we checked all the uses to make sure they weren't
> accidentally relying on it being an integer type. The type name
> is not such that it will be accidentally used outside softfloat,
> so it's less of an irritant.
Indeed.
> thanks
> -- PMM
>
> Peter Maydell (6):
> fpu: Replace int64 typedef with int64_t
> fpu: Replace uint64 typedef with uint64_t
> fpu: Replace int32 typedef with int32_t
> fpu: Replace uint32 typedef with uint32_t
> fpu: Replace int8 typedef with int8_t
> fpu: Replace uint8 typedef with uint8_t
>
> crypto/secret.c | 2 +-
> fpu/softfloat-macros.h | 26 +++---
> fpu/softfloat-specialize.h | 2 +-
> fpu/softfloat.c | 218
> ++++++++++++++++++++++-----------------------
> hw/i386/pc.c | 2 +-
> hw/ipmi/isa_ipmi_bt.c | 2 +-
> hw/ipmi/isa_ipmi_kcs.c | 2 +-
> hw/misc/imx25_ccm.c | 2 +-
> hw/misc/imx31_ccm.c | 2 +-
> hw/net/vmware_utils.h | 2 +-
> hw/net/vmxnet3.c | 2 +-
> hw/ppc/spapr_events.c | 4 +-
> include/fpu/softfloat.h | 68 ++++++--------
> include/hw/i386/pc.h | 2 +-
> migration/ram.c | 2 +-
> target-alpha/fpu_helper.c | 2 +-
> target-mips/kvm.c | 4 +-
> target-mips/msa_helper.c | 36 ++++----
> target-s390x/kvm.c | 2 +-
> tests/vhost-user-test.c | 2 +-
> 20 files changed, 187 insertions(+), 197 deletions(-)
So I am happy to give a:
Reviewed-by: Aurelien Jarno <address@hidden>
Regards,
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
address@hidden http://www.aurel32.net
- [Qemu-devel] [PATCH 4/6] fpu: Replace uint32 typedef with uint32_t, (continued)
- [Qemu-devel] [PATCH 4/6] fpu: Replace uint32 typedef with uint32_t, Peter Maydell, 2016/01/12
- [Qemu-devel] [PATCH 2/6] fpu: Replace uint64 typedef with uint64_t, Peter Maydell, 2016/01/12
- [Qemu-devel] [PATCH 6/6] fpu: Replace uint8 typedef with uint8_t, Peter Maydell, 2016/01/12
- [Qemu-devel] [PATCH 3/6] fpu: Replace int32 typedef with int32_t, Peter Maydell, 2016/01/12
- [Qemu-devel] [PATCH 1/6] fpu: Replace int64 typedef with int64_t, Peter Maydell, 2016/01/12
- [Qemu-devel] [PATCH 5/6] fpu: Replace int8 typedef with int8_t, Peter Maydell, 2016/01/12
- Re: [Qemu-devel] [PATCH 0/6] Get rid of confusing softfloat-specific integer types, Richard Henderson, 2016/01/12
- Re: [Qemu-devel] [PATCH 0/6] Get rid of confusing softfloat-specific integer types, Leon Alrae, 2016/01/12
- Re: [Qemu-devel] [PATCH 0/6] Get rid of confusing softfloat-specific integer types,
Aurelien Jarno <=
- Re: [Qemu-devel] [PATCH 0/6] Get rid of confusing softfloat-specific integer types, Peter Maydell, 2016/01/19