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

Alex Bennée posted 22 patches 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180206164815.10084-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
fpu/softfloat-macros.h          |   48 +
fpu/softfloat-specialize.h      |  109 +-
fpu/softfloat.c                 | 4545 ++++++++++++++++-----------------------
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         |    2 +-
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, 2199 insertions(+), 2941 deletions(-)
create mode 100644 include/fpu/softfloat-types.h
[Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions
Posted by Alex Bennée 6 years, 1 month ago
Hi,

The main change is applying the __attribute__((flatten)) to some of
the public functions that show up in Emilio's dbt-benchmark. This
seems to be a cleaner solution that squashing inlines higher up the
chain and still leaves the chance for re-use for the less widely used
functions. The results are an improvement over v3 by some margin:

                         NBench score; higher is better

    5 +-+-----------+-------------+------------+-------------+-----------+-+
      |                     ****### %%%%  +++                              |
  4.5 +-+...................*..*..#.%..%..****##..%%%%+ system-2.5       +-+
      |                     *  *  # %  %  *  * #  %  %      master         |
    4 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-v3       +-+
  3.5 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-%%%%.....+-+
      |                     *  *  # %  %  *  * #  %  %  * *  #  %  %       |
    3 +-+...................*..*..#.%..%..*..*.#..%..%..*.*..#..%..%.....+-+
      |                     *  *  #+%  %  *  * #$$$  %  * *  #  %  %       |
  2.5 +-+........####.......*..*..#$$..%..*..*.#..$..%..*.*..#..%..%.....+-+
      |       ****  #  %%%  *  *  # $  %  *  * #  $  %  * *  #$$$  %       |
    2 +-+.....*..*..#..%.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
      |       *  *  #  % %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
  1.5 +-+.....*..*..#$$$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
    1 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
      |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
  0.5 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
      |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
    0 +-+-----****###$$$%%--****###$$%%%--****##$$$%%%--***###$$$%%%-----+-+
                 FOURIER     NEURAL NETLU DECOMPOSITION    gmean

Slightly easier to read PNG:

    https://i.imgur.com/XEeL0bC.png

I think it's pretty ready for a merge. Shall I submit a pull myself or
does it make sense going via someone else? According to MAINTAINERS
Peter and Aurelien are responsible for this code...

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          |   48 +
 fpu/softfloat-specialize.h      |  109 +-
 fpu/softfloat.c                 | 4545 ++++++++++++++++-----------------------
 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         |    2 +-
 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, 2199 insertions(+), 2941 deletions(-)
 create mode 100644 include/fpu/softfloat-types.h

-- 
2.15.1


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

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

Type: series
Message-id: 20180206164815.10084-1-alex.bennee@linaro.org
Subject: [Qemu-devel] [PATCH v4 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
Switched to a new branch 'test'
1e781206be fpu/softfloat: re-factor sqrt
e685ea8dc4 fpu/softfloat: re-factor compare
63b87bcb69 fpu/softfloat: re-factor minmax
669591c216 fpu/softfloat: re-factor scalbn
2ed6c7c81d fpu/softfloat: re-factor int/uint to float
d1bd89624f fpu/softfloat: re-factor float to int/uint
bc1a994506 fpu/softfloat: re-factor round_to_int
6485285f24 fpu/softfloat: re-factor muladd
e385a48184 fpu/softfloat: re-factor div
92464a80c3 fpu/softfloat: re-factor mul
ffd17dee64 fpu/softfloat: re-factor add/sub
7815017f4b fpu/softfloat: define decompose structures
30d6d92cb4 fpu/softfloat: move the extract functions to the top of the file
51361e8312 fpu/softfloat: improve comments on ARM NaN propagation
c585bf8e1d include/fpu/softfloat: add some float16 constants
e5feca0066 include/fpu/softfloat: implement float16_set_sign helper
28e817f796 include/fpu/softfloat: implement float16_chs helper
fecbf8dcc9 include/fpu/softfloat: implement float16_abs helper
294fc6dd01 target/*/cpu.h: remove softfloat.h
b712e8fbb6 fpu/softfloat-types: new header to prevent excessive re-builds
694133665c include/fpu/softfloat: remove USE_SOFTFLOAT_STRUCT_TYPES
cf60e4d868 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 '('
#532: FILE: fpu/softfloat.c:2568:
+    aSign = extractFloat32Sign( a );

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

total: 18 errors, 0 warnings, 996 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
#118: FILE: fpu/softfloat.c:1261:
+                inc = ((a.frac & roundeven_mask) != frac_lsbm1 ? frac_lsbm1 : 0);

total: 0 errors, 1 warnings, 353 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 open parenthesis '('
#716: FILE: fpu/softfloat.c:3420:
+    if (    ( ( extractFloat32Exp( a ) == 0xFF ) && extractFloat32Frac( a ) )

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

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

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

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

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

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

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

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

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

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

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

ERROR: trailing statements should be on next line
#742: FILE: fpu/softfloat.c:3433:
+    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:3433:
+    if ( aSign != bSign ) return aSign && ( (uint32_t) ( ( av | bv )<<1 ) != 0 );
[...]

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

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

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

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

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

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

total: 19 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
Re: [Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions
Posted by Peter Maydell 6 years, 1 month ago
On 6 February 2018 at 16:47, Alex Bennée <alex.bennee@linaro.org> wrote:
> Hi,
>
> The main change is applying the __attribute__((flatten)) to some of
> the public functions that show up in Emilio's dbt-benchmark. This
> seems to be a cleaner solution that squashing inlines higher up the
> chain and still leaves the chance for re-use for the less widely used
> functions. The results are an improvement over v3 by some margin:
>
>                          NBench score; higher is better
>
>     5 +-+-----------+-------------+------------+-------------+-----------+-+
>       |                     ****### %%%%  +++                              |
>   4.5 +-+...................*..*..#.%..%..****##..%%%%+ system-2.5       +-+
>       |                     *  *  # %  %  *  * #  %  %      master         |
>     4 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-v3       +-+
>   3.5 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-%%%%.....+-+
>       |                     *  *  # %  %  *  * #  %  %  * *  #  %  %       |
>     3 +-+...................*..*..#.%..%..*..*.#..%..%..*.*..#..%..%.....+-+
>       |                     *  *  #+%  %  *  * #$$$  %  * *  #  %  %       |
>   2.5 +-+........####.......*..*..#$$..%..*..*.#..$..%..*.*..#..%..%.....+-+
>       |       ****  #  %%%  *  *  # $  %  *  * #  $  %  * *  #$$$  %       |
>     2 +-+.....*..*..#..%.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>       |       *  *  #  % %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>   1.5 +-+.....*..*..#$$$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>     1 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>   0.5 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>     0 +-+-----****###$$$%%--****###$$%%%--****##$$$%%%--***###$$$%%%-----+-+
>                  FOURIER     NEURAL NETLU DECOMPOSITION    gmean
>
> Slightly easier to read PNG:
>
>     https://i.imgur.com/XEeL0bC.png
>
> I think it's pretty ready for a merge. Shall I submit a pull myself or
> does it make sense going via someone else? According to MAINTAINERS
> Peter and Aurelien are responsible for this code...

I had some nits but I think the best thing to do is if you fix those
and then just send a pull request for this.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions
Posted by Laurent Vivier 6 years, 1 month ago
Le 13/02/2018 à 16:51, Peter Maydell a écrit :
> On 6 February 2018 at 16:47, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Hi,
>>
>> The main change is applying the __attribute__((flatten)) to some of
>> the public functions that show up in Emilio's dbt-benchmark. This
>> seems to be a cleaner solution that squashing inlines higher up the
>> chain and still leaves the chance for re-use for the less widely used
>> functions. The results are an improvement over v3 by some margin:
>>
>>                          NBench score; higher is better
>>
>>     5 +-+-----------+-------------+------------+-------------+-----------+-+
>>       |                     ****### %%%%  +++                              |
>>   4.5 +-+...................*..*..#.%..%..****##..%%%%+ system-2.5       +-+
>>       |                     *  *  # %  %  *  * #  %  %      master         |
>>     4 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-v3       +-+
>>   3.5 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-%%%%.....+-+
>>       |                     *  *  # %  %  *  * #  %  %  * *  #  %  %       |
>>     3 +-+...................*..*..#.%..%..*..*.#..%..%..*.*..#..%..%.....+-+
>>       |                     *  *  #+%  %  *  * #$$$  %  * *  #  %  %       |
>>   2.5 +-+........####.......*..*..#$$..%..*..*.#..$..%..*.*..#..%..%.....+-+
>>       |       ****  #  %%%  *  *  # $  %  *  * #  $  %  * *  #$$$  %       |
>>     2 +-+.....*..*..#..%.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>       |       *  *  #  % %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>   1.5 +-+.....*..*..#$$$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>     1 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>   0.5 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>     0 +-+-----****###$$$%%--****###$$%%%--****##$$$%%%--***###$$$%%%-----+-+
>>                  FOURIER     NEURAL NETLU DECOMPOSITION    gmean
>>
>> Slightly easier to read PNG:
>>
>>     https://i.imgur.com/XEeL0bC.png
>>
>> I think it's pretty ready for a merge. Shall I submit a pull myself or
>> does it make sense going via someone else? According to MAINTAINERS
>> Peter and Aurelien are responsible for this code...
> 
> I had some nits but I think the best thing to do is if you fix those
> and then just send a pull request for this.

Just to be sure no one has missed that:

https://bellard.org/libbf/

I'm wondering if it can help for this work.

Thanks,
Laurent



Re: [Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions
Posted by Alex Bennée 6 years, 1 month ago
Laurent Vivier <laurent@vivier.eu> writes:

> Le 13/02/2018 à 16:51, Peter Maydell a écrit:
>> On 6 February 2018 at 16:47, Alex Bennée <alex.bennee@linaro.org> wrote:
>>> Hi,
>>>
>>> The main change is applying the __attribute__((flatten)) to some of
>>> the public functions that show up in Emilio's dbt-benchmark. This
>>> seems to be a cleaner solution that squashing inlines higher up the
>>> chain and still leaves the chance for re-use for the less widely used
>>> functions. The results are an improvement over v3 by some margin:
>>>
>>>                          NBench score; higher is better
>>>
>>>     5 +-+-----------+-------------+------------+-------------+-----------+-+
>>>       |                     ****### %%%%  +++                              |
>>>   4.5 +-+...................*..*..#.%..%..****##..%%%%+ system-2.5       +-+
>>>       |                     *  *  # %  %  *  * #  %  %      master         |
>>>     4 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-v3       +-+
>>>   3.5 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-%%%%.....+-+
>>>       |                     *  *  # %  %  *  * #  %  %  * *  #  %  %       |
>>>     3 +-+...................*..*..#.%..%..*..*.#..%..%..*.*..#..%..%.....+-+
>>>       |                     *  *  #+%  %  *  * #$$$  %  * *  #  %  %       |
>>>   2.5 +-+........####.......*..*..#$$..%..*..*.#..$..%..*.*..#..%..%.....+-+
>>>       |       ****  #  %%%  *  *  # $  %  *  * #  $  %  * *  #$$$  %       |
>>>     2 +-+.....*..*..#..%.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>>       |       *  *  #  % %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>>   1.5 +-+.....*..*..#$$$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>>     1 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>>   0.5 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>>     0 +-+-----****###$$$%%--****###$$%%%--****##$$$%%%--***###$$$%%%-----+-+
>>>                  FOURIER     NEURAL NETLU DECOMPOSITION    gmean
>>>
>>> Slightly easier to read PNG:
>>>
>>>     https://i.imgur.com/XEeL0bC.png
>>>
>>> I think it's pretty ready for a merge. Shall I submit a pull myself or
>>> does it make sense going via someone else? According to MAINTAINERS
>>> Peter and Aurelien are responsible for this code...
>>
>> I had some nits but I think the best thing to do is if you fix those
>> and then just send a pull request for this.
>
> Just to be sure no one has missed that:
>
> https://bellard.org/libbf/
>
> I'm wondering if it can help for this work.

I did have a brief look through to get a sense of how it works. The
first thing it is missing however is half-precision. It only seems to
deal in 32 and 64 bit floats. The code is also fairly sparse in its
commenting.

The main approach seems to be somewhere between rth's glibc macro fest
and what we have now. It makes extensive use of every QEMU developers
favourite glue macro to instantiate code from a "template". This allows
some better usage of size appropriate types in each instantiation where
we just do most things at the highest precision.

However I think it also suffers the same problem as SoftFloat3 as in
it is not an upstream project so it is just another lump of code to
import into out code base. Based on that I favour our re-factor more as
I think it is easier to follow and hopefully will be easier to maintain.

I think we can address the inefficiencies in our mul/div code by passing
FloatFmt in and letting the compiler deal with it in each flattened
implementation. I prototyped mul:

  http://ix.io/MYw

However unless we are super worried about these inefficiencies I'm
proposing we merge what we have and deal with these in a later round.

>
> Thanks,
> Laurent


--
Alex Bennée

Re: [Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions
Posted by Peter Maydell 6 years, 1 month ago
On 6 February 2018 at 16:47, Alex Bennée <alex.bennee@linaro.org> wrote:
> Hi,
>
> The main change is applying the __attribute__((flatten)) to some of
> the public functions that show up in Emilio's dbt-benchmark. This
> seems to be a cleaner solution that squashing inlines higher up the
> chain and still leaves the chance for re-use for the less widely used
> functions. The results are an improvement over v3 by some margin:
>
>                          NBench score; higher is better
>
>     5 +-+-----------+-------------+------------+-------------+-----------+-+
>       |                     ****### %%%%  +++                              |
>   4.5 +-+...................*..*..#.%..%..****##..%%%%+ system-2.5       +-+
>       |                     *  *  # %  %  *  * #  %  %      master         |
>     4 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-v3       +-+
>   3.5 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-%%%%.....+-+
>       |                     *  *  # %  %  *  * #  %  %  * *  #  %  %       |
>     3 +-+...................*..*..#.%..%..*..*.#..%..%..*.*..#..%..%.....+-+
>       |                     *  *  #+%  %  *  * #$$$  %  * *  #  %  %       |
>   2.5 +-+........####.......*..*..#$$..%..*..*.#..$..%..*.*..#..%..%.....+-+
>       |       ****  #  %%%  *  *  # $  %  *  * #  $  %  * *  #$$$  %       |
>     2 +-+.....*..*..#..%.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>       |       *  *  #  % %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>   1.5 +-+.....*..*..#$$$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>     1 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>   0.5 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>     0 +-+-----****###$$$%%--****###$$%%%--****##$$$%%%--***###$$$%%%-----+-+
>                  FOURIER     NEURAL NETLU DECOMPOSITION    gmean
>
> Slightly easier to read PNG:
>
>     https://i.imgur.com/XEeL0bC.png
>
> I think it's pretty ready for a merge. Shall I submit a pull myself or
> does it make sense going via someone else? According to MAINTAINERS
> Peter and Aurelien are responsible for this code...
>
> 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

If you persuade git to use the --minimal, --patience or --histogram
git diff option when generating these patches you'll find that it
doesn't produce unreadable patches that provoke all the checkpatch
warnings. That in turn will let you find the genuine warning that
got lost in all the spurious ones:

Checking PATCH 16/22: fpu/softfloat: re-factor round_to_int...
WARNING: line over 80 characters
#127: FILE: fpu/softfloat.c:1261:
+                inc = ((a.frac & roundeven_mask) != frac_lsbm1 ?
frac_lsbm1 : 0);


As far as I can tell from a quick search, the 'histogram'
algorithm is reckoned to be about as fast as the default but
much less likely to produce terrible diffs.

  git config --global diff.algorithm histogram

should set it up as the default for all diff-producing purposes
including generating patches for email.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions
Posted by Alex Bennée 6 years, 1 month ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On 6 February 2018 at 16:47, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Hi,
>>
>> The main change is applying the __attribute__((flatten)) to some of
>> the public functions that show up in Emilio's dbt-benchmark. This
>> seems to be a cleaner solution that squashing inlines higher up the
>> chain and still leaves the chance for re-use for the less widely used
>> functions. The results are an improvement over v3 by some margin:
>>
>>                          NBench score; higher is better
>>
>>     5 +-+-----------+-------------+------------+-------------+-----------+-+
>>       |                     ****### %%%%  +++                              |
>>   4.5 +-+...................*..*..#.%..%..****##..%%%%+ system-2.5       +-+
>>       |                     *  *  # %  %  *  * #  %  %      master         |
>>     4 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-v3       +-+
>>   3.5 +-+...................*..*..#.%..%..*..*.#..%..%softfloat-%%%%.....+-+
>>       |                     *  *  # %  %  *  * #  %  %  * *  #  %  %       |
>>     3 +-+...................*..*..#.%..%..*..*.#..%..%..*.*..#..%..%.....+-+
>>       |                     *  *  #+%  %  *  * #$$$  %  * *  #  %  %       |
>>   2.5 +-+........####.......*..*..#$$..%..*..*.#..$..%..*.*..#..%..%.....+-+
>>       |       ****  #  %%%  *  *  # $  %  *  * #  $  %  * *  #$$$  %       |
>>     2 +-+.....*..*..#..%.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>       |       *  *  #  % %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>   1.5 +-+.....*..*..#$$$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>     1 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>   0.5 +-+.....*..*..#..$.%..*..*..#.$..%..*..*.#..$..%..*.*..#..$..%.....+-+
>>       |       *  *  #  $ %  *  *  # $  %  *  * #  $  %  * *  #  $  %       |
>>     0 +-+-----****###$$$%%--****###$$%%%--****##$$$%%%--***###$$$%%%-----+-+
>>                  FOURIER     NEURAL NETLU DECOMPOSITION    gmean
>>
>> Slightly easier to read PNG:
>>
>>     https://i.imgur.com/XEeL0bC.png
>>
>> I think it's pretty ready for a merge. Shall I submit a pull myself or
>> does it make sense going via someone else? According to MAINTAINERS
>> Peter and Aurelien are responsible for this code...
>>
>> 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
>
> If you persuade git to use the --minimal, --patience or --histogram
> git diff option when generating these patches you'll find that it
> doesn't produce unreadable patches that provoke all the checkpatch
> warnings.

I think this is patchew getting confused as I generated the patches
with:

  [diff]
        algorithm = minimal

In my qemu.git/.git/config

> That in turn will let you find the genuine warning that
> got lost in all the spurious ones:
>
> Checking PATCH 16/22: fpu/softfloat: re-factor round_to_int...
> WARNING: line over 80 characters
> #127: FILE: fpu/softfloat.c:1261:
> +                inc = ((a.frac & roundeven_mask) != frac_lsbm1 ?
> frac_lsbm1 : 0);

Yeah that was in the release but the one character over is the ; and it
seemed nicer keeping all the logic on the same line.

> As far as I can tell from a quick search, the 'histogram'
> algorithm is reckoned to be about as fast as the default but
> much less likely to produce terrible diffs.
>
>   git config --global diff.algorithm histogram
>
> should set it up as the default for all diff-producing purposes
> including generating patches for email.
>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions
Posted by Peter Maydell 6 years, 1 month ago
On 17 February 2018 at 13:23, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> If you persuade git to use the --minimal, --patience or --histogram
>> git diff option when generating these patches you'll find that it
>> doesn't produce unreadable patches that provoke all the checkpatch
>> warnings.
>
> I think this is patchew getting confused

Oh yes, sorry. Fam, can we update patchew's git config to use
  git config --global diff.algorithm histogram
(equivalently, algorithm = histogram in the .gitconfig) -- that
way the patches it runs checkpatch on are more likely to be
formatted so that they minimise spurious checkpatch output, I think.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions
Posted by Fam Zheng 6 years, 1 month ago
On Mon, 02/19 13:56, Peter Maydell wrote:
> On 17 February 2018 at 13:23, Alex Bennée <alex.bennee@linaro.org> wrote:
> > Peter Maydell <peter.maydell@linaro.org> writes:
> >> If you persuade git to use the --minimal, --patience or --histogram
> >> git diff option when generating these patches you'll find that it
> >> doesn't produce unreadable patches that provoke all the checkpatch
> >> warnings.
> >
> > I think this is patchew getting confused
> 
> Oh yes, sorry. Fam, can we update patchew's git config to use
>   git config --global diff.algorithm histogram
> (equivalently, algorithm = histogram in the .gitconfig) -- that
> way the patches it runs checkpatch on are more likely to be
> formatted so that they minimise spurious checkpatch output, I think.

Updated. (With s/--global/--local/ along with other diff options, in the testing
command list:

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

)

Fam