From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39758) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJ2Te-0002eG-IW for qemu-devel@nongnu.org; Tue, 12 Jan 2016 12:11:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJ2Tb-00012x-BI for qemu-devel@nongnu.org; Tue, 12 Jan 2016 12:11:26 -0500 Received: from mailapp01.imgtec.com ([195.59.15.196]:20559) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJ2Tb-00011X-5G for qemu-devel@nongnu.org; Tue, 12 Jan 2016 12:11:23 -0500 References: <1452603315-27030-1-git-send-email-peter.maydell@linaro.org> From: Leon Alrae Message-ID: <569530F3.4010302@imgtec.com> Date: Tue, 12 Jan 2016 16:59:31 +0000 MIME-Version: 1.0 In-Reply-To: <1452603315-27030-1-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/6] Get rid of confusing softfloat-specific integer types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , qemu-devel@nongnu.org Cc: Richard Henderson , James Hogan , =?UTF-8?Q?Andreas_F=c3=a4rber?= , Aurelien Jarno , patches@linaro.org On 12/01/16 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.) > > 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.) > > 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. > > 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(-) > Looks good to me; for MIPS part in patch 1 and 3: Acked-by: Leon Alrae Thanks, Leon