mbox series

[v2,00/20] re-factor softfloat and add fp16 functions

Message ID 20180109122252.17670-1-alex.bennee@linaro.org
Headers show
Series re-factor softfloat and add fp16 functions | expand

Message

Alex Bennée Jan. 9, 2018, 12:22 p.m. UTC
Hi,

Here is version two of the softfloat re-factoring. See the previous
posting for details of the approach:

  https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01708.html

There is only one new patch to remove USE_SOFTFLOAT_STRUCT_TYPES which
had bit-rotted to irrelevance. I did run into a problem with inclusion
of softfloat.h by bswap.h - which is likely the cause of the excessive
rebuilds when touching softfloat headers. We probably want to think
about moving the the type definitions to somewhere common
(qemu/typedefs.h?) but I haven't done it here to avoid too much churn.

This work is part of the larger chunk of adding half-precision ops to
the ARM front-end. However I've split the series up to make for a less
messy review. This tree can be found at:

  https://github.com/stsquad/qemu/tree/softfloat-refactor-and-fp16-v2

While I have been testing the half-precision stuff in the ARM
specific tree this series is all common code. It has however been
tested with ARM RISU which exercises the float32/64 code paths quite
nicely.

Any additional testing appreciated.

Changes for v2
--------------

 - added rth's s-o-b tags
 - added review tags
 - clean-ups for compare, minmax and float to int

As usual the details are in the individual commit messages.

Alex Bennée (20):
  fpu/softfloat: implement float16_squash_input_denormal
  include/fpu/softfloat: remove USE_SOFTFLOAT_STRUCT_TYPES
  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: propagate signalling NaNs in MINMAX
  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-macros.h     |   44 +
 fpu/softfloat-specialize.h |  115 +-
 fpu/softfloat.c            | 7417 ++++++++++++++++++++------------------------
 include/fpu/softfloat.h    |  117 +-
 4 files changed, 3437 insertions(+), 4256 deletions(-)

-- 
2.15.1

Comments

no-reply@patchew.org Jan. 9, 2018, 1:07 p.m. UTC | #1
Hi,

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

Type: series
Message-id: 20180109122252.17670-1-alex.bennee@linaro.org
Subject: [Qemu-devel] [PATCH v2 00/20] 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
Switched to a new branch 'test'
d5caeaa855 fpu/softfloat: re-factor compare
5f49459801 fpu/softfloat: re-factor minmax
94a4708c20 fpu/softfloat: re-factor scalbn
1df6e40a58 fpu/softfloat: re-factor int/uint to float
03f3d9611e fpu/softfloat: re-factor float to int/uint
b07b449ca1 fpu/softfloat: re-factor round_to_int
cbba796889 fpu/softfloat: re-factor muladd
d8ef56eff6 fpu/softfloat: re-factor div
2674f58839 fpu/softfloat: re-factor mul
212fe78e1f fpu/softfloat: re-factor add/sub
51f0c9463a fpu/softfloat: define decompose structures
24138b374d fpu/softfloat: move the extract functions to the top of the file
a9279e4a7b fpu/softfloat: improve comments on ARM NaN propagation
c2c68502de fpu/softfloat: propagate signalling NaNs in MINMAX
830590963d include/fpu/softfloat: add some float16 constants
1ec2148d81 include/fpu/softfloat: implement float16_set_sign helper
10028d5591 include/fpu/softfloat: implement float16_chs helper
f522785e08 include/fpu/softfloat: implement float16_abs helper
dacd77a7f6 include/fpu/softfloat: remove USE_SOFTFLOAT_STRUCT_TYPES
f895c43593 fpu/softfloat: implement float16_squash_input_denormal

=== OUTPUT BEGIN ===
Checking PATCH 1/20: fpu/softfloat: implement float16_squash_input_denormal...
Checking PATCH 2/20: include/fpu/softfloat: remove USE_SOFTFLOAT_STRUCT_TYPES...
Checking PATCH 3/20: include/fpu/softfloat: implement float16_abs helper...
Checking PATCH 4/20: include/fpu/softfloat: implement float16_chs helper...
Checking PATCH 5/20: include/fpu/softfloat: implement float16_set_sign helper...
Checking PATCH 6/20: include/fpu/softfloat: add some float16 constants...
Checking PATCH 7/20: fpu/softfloat: propagate signalling NaNs in MINMAX...
Checking PATCH 8/20: fpu/softfloat: improve comments on ARM NaN propagation...
Checking PATCH 9/20: fpu/softfloat: move the extract functions to the top of the file...
Checking PATCH 10/20: fpu/softfloat: define decompose structures...
ERROR: spaces prohibited around that ':' (ctx:WxW)
#53: FILE: fpu/softfloat.c:210:
+    uint64_t frac   : 64;
                     ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#54: FILE: fpu/softfloat.c:211:
+    int exp         : 32;
                     ^

ERROR: space prohibited before that ':' (ctx:WxW)
#56: FILE: fpu/softfloat.c:213:
+    int             : 23;
                     ^

ERROR: spaces prohibited around that ':' (ctx:WxW)
#57: FILE: fpu/softfloat.c:214:
+    bool sign       : 1;
                     ^

total: 4 errors, 0 warnings, 82 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/20: fpu/softfloat: re-factor add/sub...
WARNING: line over 80 characters
#141: FILE: fpu/softfloat.c:364:
+                                                   const decomposed_params *parm)

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

total: 0 errors, 1 warnings, 329 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 16/20: fpu/softfloat: re-factor float to int/uint...
ERROR: space prohibited after that '-' (ctx:WxW)
#59: FILE: fpu/softfloat.c:1347:
+            return r < - (uint64_t) INT64_MIN ? -r : INT64_MIN;
                        ^

WARNING: line over 80 characters
#95: FILE: fpu/softfloat.c:1383:
+int ## isz ## _t float ## fsz ## _to_int ## isz(float ## fsz a, float_status *s) \

WARNING: line over 80 characters
#186: FILE: fpu/softfloat.c:1474:
+uint ## isz ## _t float ## fsz ## _to_uint ## isz(float ## fsz a, float_status *s) \

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

total: 20 errors, 3 warnings, 1076 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/20: fpu/softfloat: re-factor int/uint to float...
Checking PATCH 18/20: fpu/softfloat: re-factor scalbn...
Checking PATCH 19/20: fpu/softfloat: re-factor minmax...
WARNING: line over 80 characters
#120: FILE: fpu/softfloat.c:1762:
+float ## sz float ## sz ## _ ## name(float ## sz a, float ## sz b, float_status *s) \

total: 0 errors, 1 warnings, 263 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 20/20: fpu/softfloat: re-factor compare...
WARNING: line over 80 characters
#91: FILE: fpu/softfloat.c:1861:
+int float ## sz ## _compare_quiet(float ## sz a, float ## sz b, float_status *s) \

total: 0 errors, 1 warnings, 154 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@freelists.org