[Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions

Alex Bennée posted 22 patches 6 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180124131315.30567-1-alex.bennee@linaro.org
Test checkpatch failed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppc passed
Test s390x passed
There is a newer version of this series
fpu/softfloat-macros.h          |   44 +
fpu/softfloat-specialize.h      |  109 +-
fpu/softfloat.c                 | 4538 ++++++++++++++++-----------------------
include/fpu/softfloat-types.h   |  179 ++
include/fpu/softfloat.h         |  202 +-
include/qemu/bswap.h            |    2 +-
target/alpha/cpu.h              |    2 -
target/arm/cpu.c                |    1 +
target/arm/cpu.h                |    2 -
target/arm/helper-a64.c         |    1 +
target/arm/helper.c             |    1 +
target/arm/neon_helper.c        |    1 +
target/hppa/cpu.c               |    1 +
target/hppa/cpu.h               |    1 -
target/hppa/op_helper.c         |    1 +
target/i386/cpu.h               |    4 -
target/i386/fpu_helper.c        |    1 +
target/m68k/cpu.c               |    2 +-
target/m68k/cpu.h               |    1 -
target/m68k/fpu_helper.c        |    1 +
target/m68k/helper.c            |    1 +
target/m68k/translate.c         |    2 +
target/microblaze/cpu.c         |    1 +
target/microblaze/cpu.h         |    2 +-
target/microblaze/op_helper.c   |    1 +
target/moxie/cpu.h              |    1 -
target/nios2/cpu.h              |    1 -
target/openrisc/cpu.h           |    1 -
target/openrisc/fpu_helper.c    |    1 +
target/ppc/cpu.h                |    1 -
target/ppc/fpu_helper.c         |    1 +
target/ppc/int_helper.c         |    1 +
target/ppc/translate_init.c     |    1 +
target/s390x/cpu.c              |    1 +
target/s390x/cpu.h              |    2 -
target/s390x/fpu_helper.c       |    1 +
target/sh4/cpu.c                |    1 +
target/sh4/cpu.h                |    2 -
target/sh4/op_helper.c          |    1 +
target/sparc/cpu.h              |    2 -
target/sparc/fop_helper.c       |    1 +
target/tricore/cpu.h            |    1 -
target/tricore/fpu_helper.c     |    1 +
target/tricore/helper.c         |    1 +
target/unicore32/cpu.c          |    1 +
target/unicore32/cpu.h          |    1 -
target/unicore32/ucf64_helper.c |    1 +
target/xtensa/cpu.h             |    1 -
target/xtensa/op_helper.c       |    1 +
49 files changed, 2188 insertions(+), 2940 deletions(-)
create mode 100644 include/fpu/softfloat-types.h
[Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
Posted by Alex Bennée 6 years, 2 months ago
This is the third iteration of my softfloat re-factor patches. You can
see the discussion about v2 here:

    https://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg01475.html

Most of the changes are addressing review comments but I also have
added another patch early on to try and reduce the re-builds triggered
by touching softfloat.h. This involves a bit of mechanical churn but
is worth it to avoid the complete re-build. There is still a bit of a
dependency in the MIPS cpu.h due to it's need to tweak the signalling
NaN bit.

The biggest change from the review comments are shorter names and a
reduction of the size of the function names and related structures.

I've fixed most of the checkpatch warnings but there is one line which
tops 80 chars by one character. The other I think is a false positive
about the position of a -.

As usual the details of the changes are in the individual commit
messages.

There are a few comments where I haven't made changes yet as I'm
pondering the best way forward. The main unresolved bit is
re-factoring the specialisation code that deals with the variation of
NaN behaviour. I'm wary of exposing the internals to the wider world
so perhaps a softfloat-internals.h and
target/foo/softfloat-specialize.o?

I've added one simple helper (is_nan(x)) internal to softfloat. I
haven't used it more widely than asked in the comments to avoid too
much churn. Maybe is_inf and is_zero would be additional helpful
internal helpers or for softfloat-internals.h.

I suppose the question is do we want to do that in this series or have
a follow-up as we fix the existing bugs? If we can follow-up then I
think this series is ready to go.


Alex Bennée (22):
  fpu/softfloat: implement float16_squash_input_denormal
  include/fpu/softfloat: remove USE_SOFTFLOAT_STRUCT_TYPES
  fpu/softfloat-types: new header to prevent excessive re-builds
  target/*/cpu.h: remove softfloat.h
  include/fpu/softfloat: implement float16_abs helper
  include/fpu/softfloat: implement float16_chs helper
  include/fpu/softfloat: implement float16_set_sign helper
  include/fpu/softfloat: add some float16 constants
  fpu/softfloat: improve comments on ARM NaN propagation
  fpu/softfloat: move the extract functions to the top of the file
  fpu/softfloat: define decompose structures
  fpu/softfloat: re-factor add/sub
  fpu/softfloat: re-factor mul
  fpu/softfloat: re-factor div
  fpu/softfloat: re-factor muladd
  fpu/softfloat: re-factor round_to_int
  fpu/softfloat: re-factor float to int/uint
  fpu/softfloat: re-factor int/uint to float
  fpu/softfloat: re-factor scalbn
  fpu/softfloat: re-factor minmax
  fpu/softfloat: re-factor compare
  fpu/softfloat: re-factor sqrt

 fpu/softfloat-macros.h          |   44 +
 fpu/softfloat-specialize.h      |  109 +-
 fpu/softfloat.c                 | 4538 ++++++++++++++++-----------------------
 include/fpu/softfloat-types.h   |  179 ++
 include/fpu/softfloat.h         |  202 +-
 include/qemu/bswap.h            |    2 +-
 target/alpha/cpu.h              |    2 -
 target/arm/cpu.c                |    1 +
 target/arm/cpu.h                |    2 -
 target/arm/helper-a64.c         |    1 +
 target/arm/helper.c             |    1 +
 target/arm/neon_helper.c        |    1 +
 target/hppa/cpu.c               |    1 +
 target/hppa/cpu.h               |    1 -
 target/hppa/op_helper.c         |    1 +
 target/i386/cpu.h               |    4 -
 target/i386/fpu_helper.c        |    1 +
 target/m68k/cpu.c               |    2 +-
 target/m68k/cpu.h               |    1 -
 target/m68k/fpu_helper.c        |    1 +
 target/m68k/helper.c            |    1 +
 target/m68k/translate.c         |    2 +
 target/microblaze/cpu.c         |    1 +
 target/microblaze/cpu.h         |    2 +-
 target/microblaze/op_helper.c   |    1 +
 target/moxie/cpu.h              |    1 -
 target/nios2/cpu.h              |    1 -
 target/openrisc/cpu.h           |    1 -
 target/openrisc/fpu_helper.c    |    1 +
 target/ppc/cpu.h                |    1 -
 target/ppc/fpu_helper.c         |    1 +
 target/ppc/int_helper.c         |    1 +
 target/ppc/translate_init.c     |    1 +
 target/s390x/cpu.c              |    1 +
 target/s390x/cpu.h              |    2 -
 target/s390x/fpu_helper.c       |    1 +
 target/sh4/cpu.c                |    1 +
 target/sh4/cpu.h                |    2 -
 target/sh4/op_helper.c          |    1 +
 target/sparc/cpu.h              |    2 -
 target/sparc/fop_helper.c       |    1 +
 target/tricore/cpu.h            |    1 -
 target/tricore/fpu_helper.c     |    1 +
 target/tricore/helper.c         |    1 +
 target/unicore32/cpu.c          |    1 +
 target/unicore32/cpu.h          |    1 -
 target/unicore32/ucf64_helper.c |    1 +
 target/xtensa/cpu.h             |    1 -
 target/xtensa/op_helper.c       |    1 +
 49 files changed, 2188 insertions(+), 2940 deletions(-)
 create mode 100644 include/fpu/softfloat-types.h

-- 
2.15.1


Re: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
Posted by no-reply@patchew.org 6 years, 2 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180124131315.30567-1-alex.bennee@linaro.org
Subject: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions

=== 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

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
 * [new tag]               patchew/20180124131315.30567-1-alex.bennee@linaro.org -> patchew/20180124131315.30567-1-alex.bennee@linaro.org
Switched to a new branch 'test'
e2df8d5690 fpu/softfloat: re-factor sqrt
2ef88f148d fpu/softfloat: re-factor compare
d777d185d9 fpu/softfloat: re-factor minmax
fd6b209b41 fpu/softfloat: re-factor scalbn
4d819f387b fpu/softfloat: re-factor int/uint to float
ab1c4b39a3 fpu/softfloat: re-factor float to int/uint
7a69edbad9 fpu/softfloat: re-factor round_to_int
486fd17cc5 fpu/softfloat: re-factor muladd
012ac416af fpu/softfloat: re-factor div
3dfe048a3e fpu/softfloat: re-factor mul
b4b5e1b9e7 fpu/softfloat: re-factor add/sub
b2078b836f fpu/softfloat: define decompose structures
b7bcbee5af fpu/softfloat: move the extract functions to the top of the file
5ed9d4ff0e fpu/softfloat: improve comments on ARM NaN propagation
b2197bbb83 include/fpu/softfloat: add some float16 constants
5df4251a02 include/fpu/softfloat: implement float16_set_sign helper
bb6790a949 include/fpu/softfloat: implement float16_chs helper
a5e278bfc9 include/fpu/softfloat: implement float16_abs helper
99d5952be5 target/*/cpu.h: remove softfloat.h
b63e326c55 fpu/softfloat-types: new header to prevent excessive re-builds
23e7320285 include/fpu/softfloat: remove USE_SOFTFLOAT_STRUCT_TYPES
a71a661a9f fpu/softfloat: implement float16_squash_input_denormal

=== OUTPUT BEGIN ===
Checking PATCH 1/22: fpu/softfloat: implement float16_squash_input_denormal...
Checking PATCH 2/22: include/fpu/softfloat: remove USE_SOFTFLOAT_STRUCT_TYPES...
Checking PATCH 3/22: fpu/softfloat-types: new header to prevent excessive re-builds...
Checking PATCH 4/22: target/*/cpu.h: remove softfloat.h...
Checking PATCH 5/22: include/fpu/softfloat: implement float16_abs helper...
Checking PATCH 6/22: include/fpu/softfloat: implement float16_chs helper...
Checking PATCH 7/22: include/fpu/softfloat: implement float16_set_sign helper...
Checking PATCH 8/22: include/fpu/softfloat: add some float16 constants...
Checking PATCH 9/22: fpu/softfloat: improve comments on ARM NaN propagation...
Checking PATCH 10/22: fpu/softfloat: move the extract functions to the top of the file...
Checking PATCH 11/22: fpu/softfloat: define decompose structures...
Checking PATCH 12/22: fpu/softfloat: re-factor add/sub...
ERROR: space prohibited after that open parenthesis '('
#524: FILE: fpu/softfloat.c:2560:
+    aSign = extractFloat32Sign( a );

ERROR: space prohibited before that close parenthesis ')'
#524: FILE: fpu/softfloat.c:2560:
+    aSign = extractFloat32Sign( a );

ERROR: space prohibited after that open parenthesis '('
#539: FILE: fpu/softfloat.c:2563:
+    bSign = extractFloat32Sign( b );

ERROR: space prohibited before that close parenthesis ')'
#539: FILE: fpu/softfloat.c:2563:
+    bSign = extractFloat32Sign( b );

ERROR: space prohibited after that open parenthesis '('
#541: FILE: fpu/softfloat.c:2565:
+    if ( aExp == 0xFF ) {

ERROR: space prohibited before that close parenthesis ')'
#541: FILE: fpu/softfloat.c:2565:
+    if ( aExp == 0xFF ) {

ERROR: space prohibited after that open parenthesis '('
#542: FILE: fpu/softfloat.c:2566:
+        if ( aSig || ( ( bExp == 0xFF ) && bSig ) ) {

ERROR: space prohibited before that close parenthesis ')'
#542: FILE: fpu/softfloat.c:2566:
+        if ( aSig || ( ( bExp == 0xFF ) && bSig ) ) {

ERROR: space prohibited after that open parenthesis '('
#547: FILE: fpu/softfloat.c:2569:
+        if ( ( bExp | bSig ) == 0 ) {

ERROR: space prohibited before that close parenthesis ')'
#547: FILE: fpu/softfloat.c:2569:
+        if ( ( bExp | bSig ) == 0 ) {

ERROR: space prohibited after that open parenthesis '('
#553: FILE: fpu/softfloat.c:2573:
+        return packFloat32( zSign, 0xFF, 0 );

ERROR: space prohibited before that close parenthesis ')'
#553: FILE: fpu/softfloat.c:2573:
+        return packFloat32( zSign, 0xFF, 0 );

ERROR: space prohibited after that open parenthesis '('
#564: FILE: fpu/softfloat.c:2575:
+    if ( bExp == 0xFF ) {

ERROR: space prohibited before that close parenthesis ')'
#564: FILE: fpu/softfloat.c:2575:
+    if ( bExp == 0xFF ) {

ERROR: space prohibited after that open parenthesis '('
#570: FILE: fpu/softfloat.c:2579:
+        if ( ( aExp | aSig ) == 0 ) {

ERROR: space prohibited before that close parenthesis ')'
#570: FILE: fpu/softfloat.c:2579:
+        if ( ( aExp | aSig ) == 0 ) {

ERROR: space prohibited after that open parenthesis '('
#780: FILE: fpu/softfloat.c:2583:
+        return packFloat32( zSign, 0xFF, 0 );

ERROR: space prohibited before that close parenthesis ')'
#780: FILE: fpu/softfloat.c:2583:
+        return packFloat32( zSign, 0xFF, 0 );

total: 18 errors, 0 warnings, 988 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/22: fpu/softfloat: re-factor mul...
Checking PATCH 14/22: fpu/softfloat: re-factor div...
Checking PATCH 15/22: fpu/softfloat: re-factor muladd...
Checking PATCH 16/22: fpu/softfloat: re-factor round_to_int...
WARNING: line over 80 characters
#91: FILE: fpu/softfloat.c:1243:
+                inc = ((a.frac & roundeven_mask) != frac_lsbm1 ? frac_lsbm1 : 0);

total: 0 errors, 1 warnings, 327 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 17/22: fpu/softfloat: re-factor float to int/uint...
ERROR: space prohibited after that '-' (ctx:WxW)
#62: FILE: fpu/softfloat.c:1342:
+            if (r < - (uint64_t) min) {
                     ^

ERROR: space prohibited after that open parenthesis '('
#716: FILE: fpu/softfloat.c:3402:
+    if (    ( ( extractFloat32Exp( a ) == 0xFF ) && extractFloat32Frac( a ) )

ERROR: space prohibited before that close parenthesis ')'
#716: FILE: fpu/softfloat.c:3402:
+    if (    ( ( extractFloat32Exp( a ) == 0xFF ) && extractFloat32Frac( a ) )

ERROR: space prohibited after that open parenthesis '('
#717: FILE: fpu/softfloat.c:3403:
+         || ( ( extractFloat32Exp( b ) == 0xFF ) && extractFloat32Frac( b ) )

ERROR: space prohibited before that close parenthesis ')'
#717: FILE: fpu/softfloat.c:3403:
+         || ( ( extractFloat32Exp( b ) == 0xFF ) && extractFloat32Frac( b ) )

ERROR: space prohibited after that open parenthesis '('
#738: FILE: fpu/softfloat.c:3411:
+    aSign = extractFloat32Sign( a );

ERROR: space prohibited before that close parenthesis ')'
#738: FILE: fpu/softfloat.c:3411:
+    aSign = extractFloat32Sign( a );

ERROR: space prohibited after that open parenthesis '('
#739: FILE: fpu/softfloat.c:3412:
+    bSign = extractFloat32Sign( b );

ERROR: space prohibited before that close parenthesis ')'
#739: FILE: fpu/softfloat.c:3412:
+    bSign = extractFloat32Sign( b );

WARNING: line over 80 characters
#742: FILE: fpu/softfloat.c:3415:
+    if ( aSign != bSign ) return aSign && ( (uint32_t) ( ( av | bv )<<1 ) != 0 );

ERROR: spaces required around that '<<' (ctx:VxV)
#742: FILE: fpu/softfloat.c:3415:
+    if ( aSign != bSign ) return aSign && ( (uint32_t) ( ( av | bv )<<1 ) != 0 );
                                                                     ^

ERROR: space prohibited after that open parenthesis '('
#742: FILE: fpu/softfloat.c:3415:
+    if ( aSign != bSign ) return aSign && ( (uint32_t) ( ( av | bv )<<1 ) != 0 );

ERROR: space prohibited before that close parenthesis ')'
#742: FILE: fpu/softfloat.c:3415:
+    if ( aSign != bSign ) return aSign && ( (uint32_t) ( ( av | bv )<<1 ) != 0 );

ERROR: trailing statements should be on next line
#742: FILE: fpu/softfloat.c:3415:
+    if ( aSign != bSign ) return aSign && ( (uint32_t) ( ( av | bv )<<1 ) != 0 );

ERROR: braces {} are necessary for all arms of this statement
#742: FILE: fpu/softfloat.c:3415:
+    if ( aSign != bSign ) return aSign && ( (uint32_t) ( ( av | bv )<<1 ) != 0 );
[...]

ERROR: space prohibited after that open parenthesis '('
#743: FILE: fpu/softfloat.c:3416:
+    return ( av != bv ) && ( aSign ^ ( av < bv ) );

ERROR: space prohibited before that close parenthesis ')'
#743: FILE: fpu/softfloat.c:3416:
+    return ( av != bv ) && ( aSign ^ ( av < bv ) );

ERROR: space prohibited after that open parenthesis '('
#803: FILE: fpu/softfloat.c:3432:
+    if (    ( ( extractFloat32Exp( a ) == 0xFF ) && extractFloat32Frac( a ) )

ERROR: space prohibited before that close parenthesis ')'
#803: FILE: fpu/softfloat.c:3432:
+    if (    ( ( extractFloat32Exp( a ) == 0xFF ) && extractFloat32Frac( a ) )

ERROR: space prohibited after that open parenthesis '('
#804: FILE: fpu/softfloat.c:3433:
+         || ( ( extractFloat32Exp( b ) == 0xFF ) && extractFloat32Frac( b ) )

ERROR: space prohibited before that close parenthesis ')'
#804: FILE: fpu/softfloat.c:3433:
+         || ( ( extractFloat32Exp( b ) == 0xFF ) && extractFloat32Frac( b ) )

total: 20 errors, 1 warnings, 1068 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 18/22: fpu/softfloat: re-factor int/uint to float...
Checking PATCH 19/22: fpu/softfloat: re-factor scalbn...
Checking PATCH 20/22: fpu/softfloat: re-factor minmax...
Checking PATCH 21/22: fpu/softfloat: re-factor compare...
Checking PATCH 22/22: fpu/softfloat: re-factor sqrt...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org