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
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
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
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
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
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
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
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
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
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
© 2016 - 2024 Red Hat, Inc.