configure | 2 + fpu/softfloat.c | 945 ++++++++++++++++++++++++++++++-- include/fpu/softfloat.h | 30 + target/tricore/fpu_helper.c | 9 +- tests/Makefile.include | 3 + tests/fp/.gitignore | 4 + tests/fp/Makefile | 36 ++ tests/fp/fp-bench.c | 528 ++++++++++++++++++ tests/fp/fp-test.c | 1183 ++++++++++++++++++++++++++++++++++++++++ tests/fp/muladd.fptest | 51 ++ 10 files changed, 2737 insertions(+), 54 deletions(-) create mode 100644 tests/fp/.gitignore create mode 100644 tests/fp/Makefile create mode 100644 tests/fp/fp-bench.c create mode 100644 tests/fp/fp-test.c create mode 100644 tests/fp/muladd.fptest
v2: https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06805.html
Changes since v2:
- Add R-b tags
- Add a patch to rename our canonicalize to sf_canonicalize,
to avoid clashing with glibc's.
- Add a patch to define float{32,64}_is_zero_or_normal
- Simplify the float{32,64}_input_flushX macros -- now the
macros are more verbose but the full function names are greppable.
- Move tests/fp-test to tests/fp, since now both fp-bench and fp-test
are under tests/fp.
+ Use tests/fp/fp-test.h for helpers common to both fp-bench and fp-test.
- Complete rewrite of fp-bench:
+ We can now directly call the softfloat functions, thereby
making the benchmark more sensitive to changes to those functions.
+ We can still use the native ops with "-t host".
+ The rewrite also has less macro trickery; we rely instead on
constant propagation by the compiler.
+ Alex: dropped your R-b since this changed a lot. I think you'll
like this version better though!
- Define a generic function to generate the hardfloat implementation
for ops with 2 inputs; add, sub, mul and div depend on it.
Instead of using macros, rely on the constant propagation done
by the compiler. [Alex: I dropped your R-b for the addsub
patch because it changed a lot]
+ I kept macros for other ops, because I think the subsequent
code duplication savings are worth the pain.
- Add #define's to select whether to use fpclassify etc. or
float32_is_zero etc.
+ Benchmark perf differences on x86_64, aarch64 and IBM Power8 hosts.
+ For 32-bit we don't use fpclassify etc. for any architectures,
so I was tempted to get rid of this option to save some code.
It's possible however that on some hosts I have not tested this option
might pay off, so I decided to keep it there.
- Add a #define to select whether to use isinf() or floatX_is_infinity().
Turns out this makes a big difference for power64.
- Remove float32_to_float64 support in hardfloat, since nbench or
SPEC actually showed a small yet measurable slowdown with it,
despite fp-bench showing a significant speedup for this operation.
- Do not flatten soft-fp functions; these are now slow paths.
This shrinks the size of the softfloat object below its original
size (see last patch's log).
- Add a #define to disable hardfloat for some targets. I noticed that
some targets (at least I noticed PPC, there might be others) do
clear the FP flags before calling softfloat. This precludes hardfloat
since it relies on inexact not being set. In the long run we should
fix these targets though.
Note: fp-bench can run _very_ slowly (~0.5 IPC) for -o fma on some x86_64
hosts. I have not pinned down what's going on, but from the few hosts
I have access to, it seems that machines that have been patched for
Spectre/Meltdown are susceptible to this slowdown.
Fortunately though:
1) when fma is run in QEMU (and not under a microbenchmark such as
fp-bench), fma performance is still very good (much better than with
soft-fp).
2) Compiling with -march=native gets rid of the problem.
I've reproduced this with both gcc 5.4.0 and gcc 7.1.0. The *very* same
fp-bench binary that performs very well for FMA on two machines (one
AMD, one Intel, neither patched against Meltdown/Spectre) performs
below soft-fp on another three machines (all Intel, all patched).
Note: there are some checkpatch errors, but they are false positives.
Perf numbers for fp-bench are in each commit log; numbers for several
benchmarks are in the last patch's commit log.
You can fetch this series from:
https://github.com/cota/qemu/tree/hardfloat-v3
Thanks,
Emilio
---
configure | 2 +
fpu/softfloat.c | 945 ++++++++++++++++++++++++++++++--
include/fpu/softfloat.h | 30 +
target/tricore/fpu_helper.c | 9 +-
tests/Makefile.include | 3 +
tests/fp/.gitignore | 4 +
tests/fp/Makefile | 36 ++
tests/fp/fp-bench.c | 528 ++++++++++++++++++
tests/fp/fp-test.c | 1183 ++++++++++++++++++++++++++++++++++++++++
tests/fp/muladd.fptest | 51 ++
10 files changed, 2737 insertions(+), 54 deletions(-)
create mode 100644 tests/fp/.gitignore
create mode 100644 tests/fp/Makefile
create mode 100644 tests/fp/fp-bench.c
create mode 100644 tests/fp/fp-test.c
create mode 100644 tests/fp/muladd.fptest
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 1522883475-27858-1-git-send-email-cota@braap.org
Subject: [Qemu-devel] [PATCH v3 00/15] fp-test + hardfloat
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
t [tag update] patchew/1522869306-17292-1-git-send-email-thuth@redhat.com -> patchew/1522869306-17292-1-git-send-email-thuth@redhat.com
t [tag update] patchew/1522873850-28733-1-git-send-email-eric.auger@redhat.com -> patchew/1522873850-28733-1-git-send-email-eric.auger@redhat.com
* [new tag] patchew/1522883475-27858-1-git-send-email-cota@braap.org -> patchew/1522883475-27858-1-git-send-email-cota@braap.org
Switched to a new branch 'test'
0160699351 hardfloat: support float32/64 comparison
6d049c226a hardfloat: support float32/64 square root
f3fe3550b9 hardfloat: support float32/64 fused multiply-add
46eb7c82ae hardfloat: support float32/64 division
ef7ad86525 hardfloat: support float32/64 multiplication
49d8c51e8d hardfloat: support float32/64 addition and subtraction
4c75ec0397 fpu: introduce hardfloat
a01d10e08c softfloat: add float{32, 64}_is_zero_or_normal
89e7dbd313 softfloat: rename canonicalize to sf_canonicalize
9ac9b3dbc8 tests/fp: add fp-bench, a collection of simple floating point microbenchmarks
d7ced9b1de target/tricore: use float32_is_denormal
17d8cfd693 softfloat: add float{32, 64}_is_{de, }normal
606b2bd962 fp-test: add muladd variants
0a1379a091 softfloat: fix {min, max}nummag for same-abs-value inputs
104af15d75 tests: add fp-test, a floating point test suite
=== OUTPUT BEGIN ===
Checking PATCH 1/15: tests: add fp-test, a floating point test suite...
ERROR: Macros with complex values should be enclosed in parenthesis
#380: FILE: tests/fp/fp-test.c:220:
+#define PR_EXCEPTIONS(x) \
+ ((x) & STANDARD_EXCEPTIONS ? "" : "none"), \
+ (((x) & float_flag_inexact) ? "x" : ""), \
+ (((x) & float_flag_underflow) ? "u" : ""), \
+ (((x) & float_flag_overflow) ? "o" : ""), \
+ (((x) & float_flag_divbyzero) ? "z" : ""), \
+ (((x) & float_flag_invalid) ? "i" : "")
ERROR: consider using qemu_strtoul in preference to strtoul
#841: FILE: tests/fp/fp-test.c:681:
+ significand = strtoul(&p[3], &pos, 16);
ERROR: consider using qemu_strtol in preference to strtol
#846: FILE: tests/fp/fp-test.c:686:
+ exponent = strtol(pos, &pos, 10) + 127;
ERROR: consider using qemu_strtoul in preference to strtoul
#871: FILE: tests/fp/fp-test.c:711:
+ significand = strtoul(&p[3], &pos, 16);
ERROR: consider using qemu_strtol in preference to strtol
#876: FILE: tests/fp/fp-test.c:716:
+ exponent = strtol(pos, &pos, 10) + 1023;
ERROR: consider using qemu_strtof in preference to strtof
#895: FILE: tests/fp/fp-test.c:735:
+ float f = strtof(p, &pos);
total: 6 errors, 0 warnings, 1219 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/15: softfloat: fix {min, max}nummag for same-abs-value inputs...
Checking PATCH 3/15: fp-test: add muladd variants...
Checking PATCH 4/15: softfloat: add float{32, 64}_is_{de, }normal...
Checking PATCH 5/15: target/tricore: use float32_is_denormal...
Checking PATCH 6/15: tests/fp: add fp-bench, a collection of simple floating point microbenchmarks...
ERROR: braces {} are necessary for all arms of this statement
#183: FILE: tests/fp/fp-bench.c:133:
+ } while (!float32_is_normal(r));
[...]
ERROR: braces {} are necessary for all arms of this statement
#187: FILE: tests/fp/fp-bench.c:137:
+ } while (!float64_is_normal(r));
[...]
total: 2 errors, 0 warnings, 547 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 7/15: softfloat: rename canonicalize to sf_canonicalize...
Checking PATCH 8/15: softfloat: add float{32, 64}_is_zero_or_normal...
Checking PATCH 9/15: fpu: introduce hardfloat...
ERROR: spaces required around that '*' (ctx:WxV)
#96: FILE: fpu/softfloat.c:154:
+ static inline void name(soft_t *a, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#110: FILE: fpu/softfloat.c:168:
+ static inline void name(soft_t *a, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#123: FILE: fpu/softfloat.c:181:
+ static inline void name(soft_t *a, soft_t *b, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#137: FILE: fpu/softfloat.c:195:
+ static inline void name(soft_t *a, soft_t *b, soft_t *c, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#151: FILE: fpu/softfloat.c:209:
+static inline bool can_use_fpu(const float_status *s)
^
WARNING: architecture specific defines should be avoided
#162: FILE: fpu/softfloat.c:220:
+#if defined(__x86_64__)
WARNING: architecture specific defines should be avoided
#184: FILE: fpu/softfloat.c:242:
+#if defined(__x86_64__) || defined(__aarch64__)
total: 5 errors, 2 warnings, 354 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 10/15: hardfloat: support float32/64 addition and subtraction...
ERROR: spaces required around that '*' (ctx:WxV)
#74: FILE: fpu/softfloat.c:1084:
+soft_float32_add(float32 a, float32 b, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#85: FILE: fpu/softfloat.c:1094:
+soft_float64_add(float64 a, float64 b, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#96: FILE: fpu/softfloat.c:1114:
+soft_float32_sub(float32 a, float32 b, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#107: FILE: fpu/softfloat.c:1124:
+soft_float64_sub(float64 a, float64 b, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#157: FILE: fpu/softfloat.c:1175:
+static float32 float32_addsub(float32 a, float32 b, float_status *s,
^
ERROR: spaces required around that '*' (ctx:WxV)
#169: FILE: fpu/softfloat.c:1187:
+static float64 float64_addsub(float64 a, float64 b, float_status *s,
^
ERROR: spaces required around that '*' (ctx:WxV)
#182: FILE: fpu/softfloat.c:1200:
+float32_add(float32 a, float32 b, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#188: FILE: fpu/softfloat.c:1206:
+float32_sub(float32 a, float32 b, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#194: FILE: fpu/softfloat.c:1212:
+float64_add(float64 a, float64 b, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#200: FILE: fpu/softfloat.c:1218:
+float64_sub(float64 a, float64 b, float_status *s)
^
total: 10 errors, 0 warnings, 136 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 11/15: hardfloat: support float32/64 multiplication...
ERROR: spaces required around that '*' (ctx:WxV)
#46: FILE: fpu/softfloat.c:1285:
+soft_float32_mul(float32 a, float32 b, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#57: FILE: fpu/softfloat.c:1295:
+soft_float64_mul(float64 a, float64 b, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#85: FILE: fpu/softfloat.c:1324:
+static float32 f32_mul_fast_op(float32 a, float32 b, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#92: FILE: fpu/softfloat.c:1331:
+static float64 f64_mul_fast_op(float64 a, float64 b, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#100: FILE: fpu/softfloat.c:1339:
+float32_mul(float32 a, float32 b, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#112: FILE: fpu/softfloat.c:1351:
+float64_mul(float64 a, float64 b, float_status *s)
^
total: 6 errors, 0 warnings, 84 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 12/15: hardfloat: support float32/64 division...
ERROR: spaces required around that '*' (ctx:WxV)
#48: FILE: fpu/softfloat.c:1670:
+soft_float32_div(float32 a, float32 b, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#58: FILE: fpu/softfloat.c:1680:
+soft_float64_div(float64 a, float64 b, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#125: FILE: fpu/softfloat.c:1748:
+float32_div(float32 a, float32 b, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#137: FILE: fpu/softfloat.c:1760:
+float64_div(float64 a, float64 b, float_status *s)
^
total: 4 errors, 0 warnings, 106 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 13/15: hardfloat: support float32/64 fused multiply-add...
ERROR: spaces required around that '*' (ctx:WxV)
#50: FILE: fpu/softfloat.c:1579:
+ float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#62: FILE: fpu/softfloat.c:1591:
+ float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#77: FILE: fpu/softfloat.c:1609:
+ name(soft_t a, soft_t b, soft_t c, int flags, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#137: FILE: fpu/softfloat.c:1669:
+ name(soft_t a, soft_t b, soft_t c, int flags, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#209: FILE: fpu/softfloat.c:1741:
+float32_muladd(float32 a, float32 b, float32 c, int flags, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#219: FILE: fpu/softfloat.c:1751:
+float64_muladd(float64 a, float64 b, float64 c, int flags, float_status *s)
^
total: 6 errors, 0 warnings, 187 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 14/15: hardfloat: support float32/64 square root...
ERROR: spaces required around that '*' (ctx:WxV)
#49: FILE: fpu/softfloat.c:2721:
+soft_float32_sqrt(float32 a, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#58: FILE: fpu/softfloat.c:2729:
+soft_float64_sqrt(float64 a, float_status *status)
^
ERROR: spaces required around that '*' (ctx:WxV)
#66: FILE: fpu/softfloat.c:2737:
+ static soft_t name(soft_t a, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#85: FILE: fpu/softfloat.c:2756:
+ static soft_t name(soft_t a, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#114: FILE: fpu/softfloat.c:2785:
+float32 __attribute__((flatten)) float32_sqrt(float32 a, float_status *s)
^
ERROR: spaces required around that '*' (ctx:WxV)
#123: FILE: fpu/softfloat.c:2794:
+float64 __attribute__((flatten)) float64_sqrt(float64 a, float_status *s)
^
total: 6 errors, 0 warnings, 91 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 15/15: hardfloat: support float32/64 comparison...
ERROR: spaces required around that '*' (ctx:WxV)
#113: FILE: fpu/softfloat.c:2586:
+name(float ## sz a, float ## sz b, bool is_quiet, float_status *s) \
^
ERROR: spaces required around that '*' (ctx:WxV)
#138: FILE: fpu/softfloat.c:2600:
+float16_compare(float16 a, float16 b, float_status *s)
^
total: 2 errors, 0 warnings, 88 lines checked
Your patch 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
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
© 2016 - 2024 Red Hat, Inc.