| Message ID | 20191016073240.12473-1-mkysel@tachyum.com |
|---|---|
| State | New |
| Headers | show |
| Series | Added hardfloat conversion from float32 to float64 | expand |
On 10/16/19 12:32 AM, Matus Kysel wrote: > +float64 float32_to_float64(float32 a, float_status *status) > +{ > + if (unlikely(!float32_is_normal(a))) { > + return soft_float32_to_float64(a, status); > + } else if (float32_is_zero(a)) { > + return float64_set_sign(float64_zero, float32_is_neg(a)); > + } else { > + double r = *(float *)&a; > + return *(float64 *)&r; > + } > +} This is a good idea, since there are no issues with inexact or rounding when converting in this direction. Please use union_float{32,64} instead of casting. Your special case for 0 won't fire, since it is already filtered by !float32_is_normal. So I think this could be written as if (likely(float32_is_normal(a))) { union_float32 uf; union_float64 ud; uf.s = a; ud.h = uf.h; return ud.s; } else if (float32_is_zero(a)) { return float64_set_sign(float64_zero, float32_is_neg(a)); } else { return soft_float32_to_float64(a); } r~
Patchew URL: https://patchew.org/QEMU/20191016073240.12473-1-mkysel@tachyum.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH] Added hardfloat conversion from float32 to float64
Type: series
Message-id: 20191016073240.12473-1-mkysel@tachyum.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Switched to a new branch 'test'
4af0ff7 Added hardfloat conversion from float32 to float64
=== OUTPUT BEGIN ===
ERROR: spaces required around that '*' (ctx:WxV)
#27: FILE: fpu/softfloat.c:1924:
+soft_float32_to_float64(float32 a, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#34: FILE: fpu/softfloat.c:1931:
+float64 float32_to_float64(float32 a, float_status *status)
^
total: 2 errors, 0 warnings, 27 lines checked
Commit 4af0ff7322fb (Added hardfloat conversion from float32 to float64) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20191016073240.12473-1-mkysel@tachyum.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Hi, I have send patch v2 with proposed changes, please have a look https://lists.nongnu.org/archive/html/qemu-devel/2019-10/msg04264.html Matus -----Original Message----- From: Richard Henderson <richard.henderson@linaro.org> Sent: streda 16. októbra 2019 17:38 To: Matus Kysel <mkysel@tachyum.com>; qemu-devel@nongnu.org Cc: Peter Maydell <peter.maydell@linaro.org>; Alex Bennée <alex.bennee@linaro.org>; Aurelien Jarno <aurelien@aurel32.net> Subject: Re: [PATCH] Added hardfloat conversion from float32 to float64 On 10/16/19 12:32 AM, Matus Kysel wrote: > +float64 float32_to_float64(float32 a, float_status *status) { > + if (unlikely(!float32_is_normal(a))) { > + return soft_float32_to_float64(a, status); > + } else if (float32_is_zero(a)) { > + return float64_set_sign(float64_zero, float32_is_neg(a)); > + } else { > + double r = *(float *)&a; > + return *(float64 *)&r; > + } > +} This is a good idea, since there are no issues with inexact or rounding when converting in this direction. Please use union_float{32,64} instead of casting. Your special case for 0 won't fire, since it is already filtered by !float32_is_normal. So I think this could be written as if (likely(float32_is_normal(a))) { union_float32 uf; union_float64 ud; uf.s = a; ud.h = uf.h; return ud.s; } else if (float32_is_zero(a)) { return float64_set_sign(float64_zero, float32_is_neg(a)); } else { return soft_float32_to_float64(a); } r~
diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 0638c9f4e0..e28ea3f933 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1920,13 +1920,26 @@ float16 float32_to_float16(float32 a, bool ieee, float_status *s) return float16a_round_pack_canonical(pr, s, fmt16); } -float64 float32_to_float64(float32 a, float_status *s) +static float64 QEMU_SOFTFLOAT_ATTR +soft_float32_to_float64(float32 a, float_status *s) { FloatParts p = float32_unpack_canonical(a, s); FloatParts pr = float_to_float(p, &float64_params, s); return float64_round_pack_canonical(pr, s); } +float64 float32_to_float64(float32 a, float_status *status) +{ + if (unlikely(!float32_is_normal(a))) { + return soft_float32_to_float64(a, status); + } else if (float32_is_zero(a)) { + return float64_set_sign(float64_zero, float32_is_neg(a)); + } else { + double r = *(float *)&a; + return *(float64 *)&r; + } +} + float16 float64_to_float16(float64 a, bool ieee, float_status *s) { const FloatFmt *fmt16 = ieee ? &float16_params : &float16_params_ahp;
Reintroduce float32_to_float64 that was removed here: https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg00455.html - nbench test it not actually calling this function at all - SPECS 2006 significat number of tests impoved their runtime, just few of them showed small slowdown Signed-off-by: Matus Kysel <mkysel@tachyum.com> --- fpu/softfloat.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)