All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
@ 2018-01-24 21:49 Howard Spoelstra
  2018-01-25  0:09 ` Richard Henderson
  2018-01-25 12:59 ` Alex Bennée
  0 siblings, 2 replies; 8+ messages in thread
From: Howard Spoelstra @ 2018-01-24 21:49 UTC (permalink / raw)
  To: Alex Bennée; +Cc: qemu-devel qemu-devel

Hi,

I built qemu-system-ppc for OSX and Windows from
https://github.com/stsquad/qemu/tree/softfloat-refactor-and-fp16-v3
and noticed a considerable drop in floating point performance on both
hosts.
Running Mac OS 9.2 in OSX, using MacBench 3.0, the score for the
floating point performance dropped from ~60 to ~42.

Recent tcg optimisations had improved processor and floating point
performance considerably, but that gain seems to be more than lost for
the floating point performance.

Any idea what is causing this?

Best regards,
Howard

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
  2018-01-24 21:49 [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions Howard Spoelstra
@ 2018-01-25  0:09 ` Richard Henderson
  2018-01-25  0:15   ` Howard Spoelstra
  2018-01-25 12:59 ` Alex Bennée
  1 sibling, 1 reply; 8+ messages in thread
From: Richard Henderson @ 2018-01-25  0:09 UTC (permalink / raw)
  To: Howard Spoelstra, Alex Bennée; +Cc: qemu-devel qemu-devel

On 01/24/2018 01:49 PM, Howard Spoelstra wrote:
> Hi,
> 
> I built qemu-system-ppc for OSX and Windows from
> https://github.com/stsquad/qemu/tree/softfloat-refactor-and-fp16-v3
> and noticed a considerable drop in floating point performance on both
> hosts.
> Running Mac OS 9.2 in OSX, using MacBench 3.0, the score for the
> floating point performance dropped from ~60 to ~42.
> 
> Recent tcg optimisations had improved processor and floating point
> performance considerably, but that gain seems to be more than lost for
> the floating point performance.
> 
> Any idea what is causing this?

Is this a 64-bit or a 32-bit build?

We made a conscious choice to share code and use all 64-bit operations, under
the assumption that x86_64 is the most common host.

Otherwise we have not really done extensive benchmarking.


r~

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
  2018-01-25  0:09 ` Richard Henderson
@ 2018-01-25  0:15   ` Howard Spoelstra
  0 siblings, 0 replies; 8+ messages in thread
From: Howard Spoelstra @ 2018-01-25  0:15 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alex Bennée, qemu-devel qemu-devel

On Thu, Jan 25, 2018 at 1:09 AM, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 01/24/2018 01:49 PM, Howard Spoelstra wrote:
>> Hi,
>>
>> I built qemu-system-ppc for OSX and Windows from
>> https://github.com/stsquad/qemu/tree/softfloat-refactor-and-fp16-v3
>> and noticed a considerable drop in floating point performance on both
>> hosts.
>> Running Mac OS 9.2 in OSX, using MacBench 3.0, the score for the
>> floating point performance dropped from ~60 to ~42.
>>
>> Recent tcg optimisations had improved processor and floating point
>> performance considerably, but that gain seems to be more than lost for
>> the floating point performance.
>>
>> Any idea what is causing this?
>
> Is this a 64-bit or a 32-bit build?
>
> We made a conscious choice to share code and use all 64-bit operations, under
> the assumption that x86_64 is the most common host.
>
> Otherwise we have not really done extensive benchmarking.
>
>
> r~

Hi,

Both OSX and Windows builds are 64-bit.

Best,
Howard

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
  2018-01-24 21:49 [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions Howard Spoelstra
  2018-01-25  0:09 ` Richard Henderson
@ 2018-01-25 12:59 ` Alex Bennée
  2018-01-28 20:41   ` Emilio G. Cota
  1 sibling, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2018-01-25 12:59 UTC (permalink / raw)
  To: Howard Spoelstra; +Cc: qemu-devel qemu-devel


Howard Spoelstra <hsp.cat7@gmail.com> writes:

> Hi,
>
> I built qemu-system-ppc for OSX and Windows from
> https://github.com/stsquad/qemu/tree/softfloat-refactor-and-fp16-v3
> and noticed a considerable drop in floating point performance on both
> hosts.
> Running Mac OS 9.2 in OSX, using MacBench 3.0, the score for the
> floating point performance dropped from ~60 to ~42.
>
> Recent tcg optimisations had improved processor and floating point
> performance considerably, but that gain seems to be more than lost for
> the floating point performance.
>
> Any idea what is causing this?

Well we expected a little degradation but it's a bit more than I
expected.

Re-factor

12:32:18 [alex@zen:~/m/t/aarch64] master(+0/-0) ± ~/lsrc/qemu/qemu.git/aarch64-linux-user/qemu-aarch64 ./vector-benchmark -b float64-add
float64-add         : test took 1743387 msec
                      67108864 ops, ~25978 nsec/kop
12:32:26 [alex@zen:~/m/t/aarch64] master(+0/-0) ± ~/lsrc/qemu/qemu.git/aarch64-linux-user/qemu-aarch64 ./vector-benchmark -b float64-add
float64-add         : test took 1735526 msec
                      67108864 ops, ~25861 nsec/kop
12:32:32 [alex@zen:~/m/t/aarch64] master(+0/-0) ± ~/lsrc/qemu/qemu.git/aarch64-linux-user/qemu-aarch64 ./vector-benchmark -b float64-add
float64-add         : test took 1742030 msec
                      67108864 ops, ~25958 nsec/kop

Original

12:32:35 [alex@zen:~/m/t/aarch64] master(+0/-0) ± ~/lsrc/qemu/qemu-builddirs/arm-targets.build/aarch64-linux-user/qemu-aarch64 ./vector-benchmark -b float64-add
float64-add         : test took 1255007 msec
                      67108864 ops, ~18701 nsec/kop
12:32:44 [alex@zen:~/m/t/aarch64] master(+0/-0) ± ~/lsrc/qemu/qemu-builddirs/arm-targets.build/aarch64-linux-user/qemu-aarch64 ./vector-benchmark -b float64-add
float64-add         : test took 1243866 msec
                      67108864 ops, ~18535 nsec/kop
12:32:46 [alex@zen:~/m/t/aarch64] master(+0/-0) ± ~/lsrc/qemu/qemu-builddirs/arm-targets.build/aarch64-linux-user/qemu-aarch64 ./vector-benchmark -b float64-add
float64-add         : test took 1278100 msec
                      67108864 ops, ~19045 nsec/kop

The main difference is all the code has a common path now and works on a
canonicalized representation of the floating point number rather than
direct bit-fiddling of the register representation. This is obviously
going to have some cost but it looks like we should spend a bit more
time seeing if we can claw some of that back.

That said any floating point on QEMU is going to be slow by the very
nature of the implementation. Because we don't translate guest<->host
instructions directly we will be several orders of magnitude slower for
floating point instructions compared to normal integer and bitwise
operations. If we want to bring on FPU performance we need to look at
how we can safely use host instructions to get the exact same results as
we currently do with softfloat.

I'm still keen on the re-factor as it does achieve the other goals of
improving readability of the code and making it much easier to reason
with. It also reduces the amount of effective copy and paste but with
slightly different constants which the old code had.

>
> Best regards,
> Howard


--
Alex Bennée

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
  2018-01-25 12:59 ` Alex Bennée
@ 2018-01-28 20:41   ` Emilio G. Cota
  2018-01-29 19:14     ` Richard Henderson
  0 siblings, 1 reply; 8+ messages in thread
From: Emilio G. Cota @ 2018-01-28 20:41 UTC (permalink / raw)
  To: Alex Bennée; +Cc: Howard Spoelstra, qemu-devel qemu-devel

On Thu, Jan 25, 2018 at 12:59:56 +0000, Alex Bennée wrote:
> Howard Spoelstra <hsp.cat7@gmail.com> writes:
(snip)
> > Recent tcg optimisations had improved processor and floating point
> > performance considerably, but that gain seems to be more than lost for
> > the floating point performance.
> >
> > Any idea what is causing this?
> 
> Well we expected a little degradation but it's a bit more than I
> expected.

It's pretty bad (almost a 2X mean slowdown) for the three FP workloads
in dbt-bench:

[FWIW before is 52483b067cce, after is 00fc0c00ca9fa26]

                        NBench score; higher is better                         
                Host: Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz                 
                                                                               
  6 +-+-----------+-------------+--------------+-------------+-----------+-+   
    |                       *****                                          |   
    |                       *   *                 softfloat-before         |   
  5 +-+.....................*...*.........******...softfloat-after.......+-+   
    |                       *   *         *    *        ******             |   
    |                       *   *         *    *        *    *             |   
  4 +-+.....................*...*.........*....*........*....*...........+-+   
    |                       *   *         *    *        *    *             |   
  3 +-+......******.........*...*.........*....*........*....*...........+-+   
    |        *    *         *   *#####    *    *####    *    *             |   
    |        *    *         *   *    #    *    *   #    *    *#####        |   
  2 +-+......*....*.........*...*....#....*....*...#....*....*....#......+-+   
    |        *    *         *   *    #    *    *   #    *    *    #        |   
    |        *    *#####    *   *    #    *    *   #    *    *    #        |   
  1 +-+......*....*....#....*...*....#....*....*...#....*....*....#......+-+   
    |        *    *    #    *   *    #    *    *   #    *    *    #        |   
    |        *    *    #    *   *    #    *    *   #    *    *    #        |   
  0 +-+------******#####----*****#####----******####----******#####------+-+   
               FOURIER     NEURAL NET  LU DECOMPOSITION    gmean               


You can obtain the above on your machine (assuming /tmp/softfloat-$branch
corresponds to the before/after x86_64-linux-user binaries) with:

$ time for branch in before after; do cd $DBT_BENCH/nbench && \
	taskset -c 0 /tmp/softfloat-$branch \
	$DBT_BENCH/nbench/nbench -V -C$DBT_BENCH/nbench/FP.DAT \
	| tee $DBT_BENCH/softfloat-$branch.nbench || break && cd -; \
	done

And then plot the results with:
$ ./breakdown.pl --suite=fp --barchart softfloat-before.nbench softfloat-after.nbench \
	| $BARCHART/barchart.pl --extra-gnuplot='set term dumb 80' \
	--extra-gnuplot='set title "NBench score; higher is better\nHost: Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz"' \
	--extra-gnuplot="set yrange [0:]" --extra="=norotate" | gnuplot

You'll need both dbt-bench and barchart:
  https://github.com/cota/dbt-bench (clone with --recursive and build nbench first.
    You can also use CROSS_COMPILE there to benchmark other linux-user targets)
  https://github.com/cota/barchart

Hope that helps,

		Emilio

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
  2018-01-28 20:41   ` Emilio G. Cota
@ 2018-01-29 19:14     ` Richard Henderson
  0 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2018-01-29 19:14 UTC (permalink / raw)
  To: Emilio G. Cota, Alex Bennée; +Cc: qemu-devel qemu-devel, Howard Spoelstra

On 01/28/2018 12:41 PM, Emilio G. Cota wrote:
> On Thu, Jan 25, 2018 at 12:59:56 +0000, Alex Bennée wrote:
>> Howard Spoelstra <hsp.cat7@gmail.com> writes:
> (snip)
>>> Recent tcg optimisations had improved processor and floating point
>>> performance considerably, but that gain seems to be more than lost for
>>> the floating point performance.
>>>
>>> Any idea what is causing this?
>>
>> Well we expected a little degradation but it's a bit more than I
>> expected.
> 
> It's pretty bad (almost a 2X mean slowdown) for the three FP workloads
> in dbt-bench:

I have not yet tried dbt-bench, but on another test case I was able to recover
all of the performance by marking all of the helpers inline.  I'm somewhat
disappointed by the compiler here.


r~

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
  2018-01-24 13:12 Alex Bennée
@ 2018-01-24 13:42 ` no-reply
  0 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2018-01-24 13:42 UTC (permalink / raw)
  To: alex.bennee
  Cc: famz, richard.henderson, peter.maydell, laurent, bharata, andrew,
	qemu-devel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions
@ 2018-01-24 13:12 Alex Bennée
  2018-01-24 13:42 ` no-reply
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2018-01-24 13:12 UTC (permalink / raw)
  To: richard.henderson, peter.maydell, laurent, bharata, andrew
  Cc: qemu-devel, Alex Bennée

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-01-29 19:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-24 21:49 [Qemu-devel] [PATCH v3 00/22] re-factor softfloat and add fp16 functions Howard Spoelstra
2018-01-25  0:09 ` Richard Henderson
2018-01-25  0:15   ` Howard Spoelstra
2018-01-25 12:59 ` Alex Bennée
2018-01-28 20:41   ` Emilio G. Cota
2018-01-29 19:14     ` Richard Henderson
  -- strict thread matches above, loose matches on Subject: below --
2018-01-24 13:12 Alex Bennée
2018-01-24 13:42 ` no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.