mbox series

[v2,00/14] target/arm: Fixups for ARM_FEATURE_V8_FP16

Message ID 20180502221552.3873-1-richard.henderson@linaro.org
Headers show
Series target/arm: Fixups for ARM_FEATURE_V8_FP16 | expand

Message

Richard Henderson May 2, 2018, 10:15 p.m. UTC 
When running the gcc testsuite with current aarch64-linux-user,
the testsuite detects the presence of the fp16 extension and
enables lots of extra tests for builtins.

Quite a few of these new tests fail because we missed implementing
some instructions.  We really should go back and verify that nothing
else is missing from this (rather large) extension.

In addition, it tests some edge conditions on data that show flaws
in the way we were performing integer<->fp conversion; particularly
with respect to scaled conversion.

Changes since v1:
  * Rebased vs master instead of tgt-arm-sve-9.
  * Alex did some additional digging through the ARM xhtml
    and came up with some additional missing instructions.
  * Everything cc'd to qemu-stable.


r~


Alex Bennée (4):
  target/arm: Implement FCMP for fp16
  target/arm: Implement FCSEL for fp16
  target/arm: Implement FMOV (immediate) for fp16
  target/arm: Fix sqrt_f16 exception raising

Richard Henderson (10):
  target/arm: Implement vector shifted SCVF/UCVF for fp16
  target/arm: Implement vector shifted FCVT for fp16
  target/arm: Fix float16 to/from int16
  target/arm: Clear SVE high bits for FMOV
  target/arm: Implement FMOV (general) for fp16
  target/arm: Implement FCVT (scalar,integer) for fp16
  target/arm: Implement FCVT (scalar,fixed-point) for fp16
  target/arm: Introduce and use read_fp_hreg
  target/arm: Implement FP data-processing (2 source) for fp16
  target/arm: Implement FP data-processing (3 source) for fp16

 target/arm/helper-a64.h    |   2 +
 target/arm/helper.h        |   6 +
 target/arm/helper-a64.c    |  10 +
 target/arm/helper.c        |  87 +++++++-
 target/arm/translate-a64.c | 535 ++++++++++++++++++++++++++++++++++++---------
 5 files changed, 532 insertions(+), 108 deletions(-)

-- 
2.14.3

Comments

no-reply@patchew.org May 2, 2018, 10:33 p.m. UTC | #1 
Hi,

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

Type: series
Message-id: 20180502221552.3873-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH v2 00/14] target/arm: Fixups for ARM_FEATURE_V8_FP16

=== 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
git config --local diff.algorithm histogram

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/20180502221552.3873-1-richard.henderson@linaro.org -> patchew/20180502221552.3873-1-richard.henderson@linaro.org
Switched to a new branch 'test'
0af6837b48 target/arm: Fix sqrt_f16 exception raising
c740b594ef target/arm: Implement FMOV (immediate) for fp16
9869941c58 target/arm: Implement FCSEL for fp16
2f9af143e5 target/arm: Implement FCMP for fp16
d7fd874757 target/arm: Implement FP data-processing (3 source) for fp16
bd693b19f4 target/arm: Implement FP data-processing (2 source) for fp16
a7c1ff7f31 target/arm: Introduce and use read_fp_hreg
5c2dfce3b3 target/arm: Implement FCVT (scalar, fixed-point) for fp16
ece533faad target/arm: Implement FCVT (scalar, integer) for fp16
1d81531581 target/arm: Implement FMOV (general) for fp16
d3e47fd86b target/arm: Clear SVE high bits for FMOV
b8530d66b3 target/arm: Fix float16 to/from int16
96451f8fef target/arm: Implement vector shifted FCVT for fp16
f3d1b8db5d target/arm: Implement vector shifted SCVF/UCVF for fp16

=== OUTPUT BEGIN ===
Checking PATCH 1/14: target/arm: Implement vector shifted SCVF/UCVF for fp16...
Checking PATCH 2/14: target/arm: Implement vector shifted FCVT for fp16...
Checking PATCH 3/14: target/arm: Fix float16 to/from int16...
ERROR: spaces required around that '*' (ctx:WxV)
#45: FILE: target/arm/helper.c:11434:
+static float16 do_postscale_fp16(float64 f, int shift, float_status *fpst)
                                                                     ^

total: 1 errors, 0 warnings, 83 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 4/14: target/arm: Clear SVE high bits for FMOV...
Checking PATCH 5/14: target/arm: Implement FMOV (general) for fp16...
Checking PATCH 6/14: target/arm: Implement FCVT (scalar, integer) for fp16...
Checking PATCH 7/14: target/arm: Implement FCVT (scalar, fixed-point) for fp16...
Checking PATCH 8/14: target/arm: Introduce and use read_fp_hreg...
Checking PATCH 9/14: target/arm: Implement FP data-processing (2 source) for fp16...
Checking PATCH 10/14: target/arm: Implement FP data-processing (3 source) for fp16...
Checking PATCH 11/14: target/arm: Implement FCMP for fp16...
Checking PATCH 12/14: target/arm: Implement FCSEL for fp16...
ERROR: space required before the open parenthesis '('
#41: FILE: target/arm/translate-a64.c:4683:
+    switch(type) {

total: 1 errors, 0 warnings, 58 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/14: target/arm: Implement FMOV (immediate) for fp16...
Checking PATCH 14/14: target/arm: Fix sqrt_f16 exception raising...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Peter Maydell May 10, 2018, 5:09 p.m. UTC | #2 
On 2 May 2018 at 23:15, Richard Henderson <richard.henderson@linaro.org> wrote:
> When running the gcc testsuite with current aarch64-linux-user,

> the testsuite detects the presence of the fp16 extension and

> enables lots of extra tests for builtins.

>

> Quite a few of these new tests fail because we missed implementing

> some instructions.  We really should go back and verify that nothing

> else is missing from this (rather large) extension.

>

> In addition, it tests some edge conditions on data that show flaws

> in the way we were performing integer<->fp conversion; particularly

> with respect to scaled conversion.

>

> Changes since v1:

>   * Rebased vs master instead of tgt-arm-sve-9.

>   * Alex did some additional digging through the ARM xhtml

>     and came up with some additional missing instructions.

>   * Everything cc'd to qemu-stable.


I'm going to take patches 1..4 into target-arm.next, just to
reduce the size of this a bit. Patch 5 is the FMOV one which
is missing some decode. Otherwise I've reviewed all the ones
which still needed review. Don't forget to fix the patch 12
patchew warning:
ERROR: space required before the open parenthesis '('
#41: FILE: target/arm/translate-a64.c:4683:
+    switch(type) {

thanks
-- PMM