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

[Peter Maydell]

On 6 February 2018 at 16:47, Alex Bennée <address@hidden> 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