mbox series

[0/9] target/arm: Fixups for ARM_FEATURE_V8_FP16

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

Message

Richard Henderson April 25, 2018, 1:22 a.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.


r~

PS: FWIW, this was written against my tgt-arm-sve-9 tree, since I
was trying to test sve as generated by gcc.  I don't *think* there
are any dependencies on any of the sve patches, but I didn't check.

PPS: There are two more failures that might be qemu fp16 failures,
but those are SIGSEGV.  This patch set cures all of the SIGILL and
(subsequent) SIGABRT type failures within the testsuite.


Richard Henderson (9):
  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: Implement FP data-processing (2 source) for fp16
  target/arm: Implement FP data-processing (3 source) for fp16

 target/arm/helper.h        |   6 +
 target/arm/helper.c        |  87 ++++++++++-
 target/arm/translate-a64.c | 371 +++++++++++++++++++++++++++++++++++++--------
 3 files changed, 399 insertions(+), 65 deletions(-)

-- 
2.14.3

Comments

no-reply@patchew.org April 25, 2018, 1:35 a.m. UTC | #1
Hi,

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

Type: series
Message-id: 20180425012300.14698-1-richard.henderson@linaro.org
Subject: [Qemu-devel] [PATCH 0/9] 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
 t [tag update]            patchew/1513160272-15921-1-git-send-email-christian.ehrhardt@canonical.com -> patchew/1513160272-15921-1-git-send-email-christian.ehrhardt@canonical.com
 t [tag update]            patchew/20180424152405.10304-1-alex.bennee@linaro.org -> patchew/20180424152405.10304-1-alex.bennee@linaro.org
 t [tag update]            patchew/20180424160329.8089-1-alex.bennee@linaro.org -> patchew/20180424160329.8089-1-alex.bennee@linaro.org
 t [tag update]            patchew/20180424214550.32549-1-lersek@redhat.com -> patchew/20180424214550.32549-1-lersek@redhat.com
 t [tag update]            patchew/20180424222103.19946-1-f4bug@amsat.org -> patchew/20180424222103.19946-1-f4bug@amsat.org
 * [new tag]               patchew/20180425012300.14698-1-richard.henderson@linaro.org -> patchew/20180425012300.14698-1-richard.henderson@linaro.org
Switched to a new branch 'test'
84efc5c3b1 target/arm: Implement FP data-processing (3 source) for fp16
10574670d9 target/arm: Implement FP data-processing (2 source) for fp16
2d4e187869 target/arm: Implement FCVT (scalar, fixed-point) for fp16
ec350065e4 target/arm: Implement FCVT (scalar, integer) for fp16
d157aaa589 target/arm: Implement FMOV (general) for fp16
7eeb78179f target/arm: Clear SVE high bits for FMOV
a7fed5d8a6 target/arm: Fix float16 to/from int16
72239133a0 target/arm: Implement vector shifted FCVT for fp16
5cb4ff2996 target/arm: Implement vector shifted SCVF/UCVF for fp16

=== OUTPUT BEGIN ===
Checking PATCH 1/9: target/arm: Implement vector shifted SCVF/UCVF for fp16...
Checking PATCH 2/9: target/arm: Implement vector shifted FCVT for fp16...
Checking PATCH 3/9: target/arm: Fix float16 to/from int16...
ERROR: spaces required around that '*' (ctx:WxV)
#40: FILE: target/arm/helper.c:11423:
+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/9: target/arm: Clear SVE high bits for FMOV...
Checking PATCH 5/9: target/arm: Implement FMOV (general) for fp16...
Checking PATCH 6/9: target/arm: Implement FCVT (scalar, integer) for fp16...
Checking PATCH 7/9: target/arm: Implement FCVT (scalar, fixed-point) for fp16...
Checking PATCH 8/9: target/arm: Implement FP data-processing (2 source) for fp16...
Checking PATCH 9/9: target/arm: Implement FP data-processing (3 source) for fp16...
=== 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
Alex Bennée April 25, 2018, 9:14 a.m. UTC | #2
Richard Henderson <richard.henderson@linaro.org> writes:

> 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.


Ouch. I thought I'd got them all from parsing the ASL. Obviously not.

> 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.

>

>

> r~

>

> PS: FWIW, this was written against my tgt-arm-sve-9 tree, since I

> was trying to test sve as generated by gcc.  I don't *think* there

> are any dependencies on any of the sve patches, but I didn't check.

>

> PPS: There are two more failures that might be qemu fp16 failures,

> but those are SIGSEGV.  This patch set cures all of the SIGILL and

> (subsequent) SIGABRT type failures within the testsuite.

>

>

> Richard Henderson (9):

>   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: Implement FP data-processing (2 source) for fp16

>   target/arm: Implement FP data-processing (3 source) for fp16

>

>  target/arm/helper.h        |   6 +

>  target/arm/helper.c        |  87 ++++++++++-

>  target/arm/translate-a64.c | 371 +++++++++++++++++++++++++++++++++++++--------

>  3 files changed, 399 insertions(+), 65 deletions(-)



--
Alex Bennée
Alex Bennée April 27, 2018, 5:22 p.m. UTC | #3
Richard Henderson <richard.henderson@linaro.org> writes:

> 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.


So this set of instructions is generated from any ASL description that
contains "half":

# Input file for risugen defining AArch64 instructions
.mode arm.aarch64
FADDP_asisdpair_only_H A64_V 0101111000110000110110 Rn:5  Rd:5
FRECPS_asimdsamefp16_only A64_V 0 Q:1 001110010 Rm:5 001111 Rn:5  Rd:5
FMAXNMP_asisdpair_only_H A64_V 0101111000110000110010 Rn:5  Rd:5
SCVTF_asimdmiscfp16_R A64_V 0 Q:1 00111001111001110110 Rn:5  Rd:5
FRSQRTE_asisdmiscfp16_R A64_V 0111111011111001110110 Rn:5  Rd:5
FCMGT_asisdmiscfp16_FZ A64_V 0101111011111000110010 Rn:5  Rd:5
FMUL_asimdsamefp16_only A64_V 0 Q:1 101110010 Rm:5 000111 Rn:5  Rd:5
FSUB_asimdsamefp16_only A64_V 0 Q:1 001110110 Rm:5 000101 Rn:5  Rd:5
FMAX_H_floatdp2 A64_V 00011110111 Rm:5 010010 Rn:5  Rd:5
FADDP_asimdsamefp16_only A64_V 0 Q:1 101110010 Rm:5 000101 Rn:5  Rd:5
FMAXNMP_asimdsamefp16_only A64_V 0 Q:1 101110010 Rm:5 000001 Rn:5  Rd:5
FMUL_asisdelem_RH_H A64_V 0101111100 L:1  M:1  Rm:4 1001 H:1 0 Rn:5  Rd:5
FCMGT_asimdsamefp16_only A64_V 0 Q:1 101110110 Rm:5 001001 Rn:5  Rd:5
FCVTAS_asisdmiscfp16_R A64_V 0101111001111001110010 Rn:5  Rd:5
FADD_asimdsamefp16_only A64_V 0 Q:1 001110010 Rm:5 000101 Rn:5  Rd:5
FCSEL_H_floatsel A64_V 00011110111 Rm:5  cond:4 11 Rn:5  Rd:5
FMAXNM_H_floatdp2 A64_V 00011110111 Rm:5 011010 Rn:5  Rd:5
FMAXNM_asimdsamefp16_only A64_V 0 Q:1 001110010 Rm:5 000001 Rn:5  Rd:5
FMINNMP_asisdpair_only_H A64_V 0101111010110000110010 Rn:5  Rd:5
FNMUL_H_floatdp2 A64_V 00011110111 Rm:5 100010 Rn:5  Rd:5
FCMPE_H_floatcmp A64_V 00011110111 Rm:5 001000 Rn:5 10000
FMUL_asimdelem_RH_H A64_V 0 Q:1 00111100 L:1  M:1  Rm:4 1001 H:1 0 Rn:5  Rd:5
FRINTN_asimdmiscfp16_R A64_V 0 Q:1 00111001111001100010 Rn:5  Rd:5
FCCMP_H_floatccmp A64_V 00011110111 Rm:5  cond:4 01 Rn:5 0 nzcv:4
FMADD_H_floatdp3 A64_V 00011111110 Rm:5 0 Ra:5  Rn:5  Rd:5
FCVTZS_asisdmiscfp16_R A64_V 0101111011111001101110 Rn:5  Rd:5
FCVTMS_asisdmiscfp16_R A64_V 0101111001111001101110 Rn:5  Rd:5
FMINP_asimdsamefp16_only A64_V 0 Q:1 101110110 Rm:5 001101 Rn:5  Rd:5
FRECPE_asimdmiscfp16_R A64_V 0 Q:1 00111011111001110110 Rn:5  Rd:5
FCMGE_asisdmiscfp16_FZ A64_V 0111111011111000110010 Rn:5  Rd:5
FNEG_asimdmiscfp16_R A64_V 0 Q:1 10111011111000111110 Rn:5  Rd:5
FCMGE_asisdsamefp16_only A64_V 01111110010 Rm:5 001001 Rn:5  Rd:5
FMULX_asisdelem_RH_H A64_V 0111111100 L:1  M:1  Rm:4 1001 H:1 0 Rn:5  Rd:5
SCVTF_asisdmiscfp16_R A64_V 0101111001111001110110 Rn:5  Rd:5
FMAXNMV_asimdall_only_H A64_V 0 Q:1 00111000110000110010 Rn:5  Rd:5
FMLA_asimdsamefp16_only A64_V 0 Q:1 001110010 Rm:5 000011 Rn:5  Rd:5
FACGT_asisdsamefp16_only A64_V 01111110110 Rm:5 001011 Rn:5  Rd:5
FRINTP_H_floatdp1 A64_V 0001111011100100110000 Rn:5  Rd:5
FRINTZ_asimdmiscfp16_R A64_V 0 Q:1 00111011111001100110 Rn:5  Rd:5
FRINTI_asimdmiscfp16_R A64_V 0 Q:1 10111011111001100110 Rn:5  Rd:5
FDIV_H_floatdp2 A64_V 00011110111 Rm:5 000110 Rn:5  Rd:5
FCVTZS_asimdmiscfp16_R A64_V 0 Q:1 00111011111001101110 Rn:5  Rd:5
FRINTP_asimdmiscfp16_R A64_V 0 Q:1 00111011111001100010 Rn:5  Rd:5
FCCMPE_H_floatccmp A64_V 00011110111 Rm:5  cond:4 01 Rn:5 1 nzcv:4
FCMGT_asisdsamefp16_only A64_V 01111110110 Rm:5 001001 Rn:5  Rd:5
FRINTM_asimdmiscfp16_R A64_V 0 Q:1 00111001111001100110 Rn:5  Rd:5
FMAXP_asimdsamefp16_only A64_V 0 Q:1 101110010 Rm:5 001101 Rn:5  Rd:5
FABD_asisdsamefp16_only A64_V 01111110110 Rm:5 000101 Rn:5  Rd:5
FCMGE_asimdmiscfp16_FZ A64_V 0 Q:1 10111011111000110010 Rn:5  Rd:5
FCMEQ_asimdsamefp16_only A64_V 0 Q:1 001110010 Rm:5 001001 Rn:5  Rd:5
FMINNM_asimdsamefp16_only A64_V 0 Q:1 001110110 Rm:5 000001 Rn:5  Rd:5
FNMADD_H_floatdp3 A64_V 00011111111 Rm:5 0 Ra:5  Rn:5  Rd:5
FRINTI_H_floatdp1 A64_V 0001111011100111110000 Rn:5  Rd:5
FMINNM_H_floatdp2 A64_V 00011110111 Rm:5 011110 Rn:5  Rd:5
FCMLT_asimdmiscfp16_FZ A64_V 0 Q:1 00111011111000111010 Rn:5  Rd:5
FCVTAU_asisdmiscfp16_R A64_V 0111111001111001110010 Rn:5  Rd:5
FCVTNU_asisdmiscfp16_R A64_V 0111111001111001101010 Rn:5  Rd:5
FCVTZU_asisdmiscfp16_R A64_V 0111111011111001101110 Rn:5  Rd:5
FABS_asimdmiscfp16_R A64_V 0 Q:1 00111011111000111110 Rn:5  Rd:5
FMLS_asisdelem_RH_H A64_V 0101111100 L:1  M:1  Rm:4 0101 H:1 0 Rn:5  Rd:5
FCMLT_asisdmiscfp16_FZ A64_V 0101111011111000111010 Rn:5  Rd:5
FMAXP_asisdpair_only_H A64_V 0101111000110000111110 Rn:5  Rd:5
FCVTPS_asisdmiscfp16_R A64_V 0101111011111001101010 Rn:5  Rd:5
FRECPE_asisdmiscfp16_R A64_V 0101111011111001110110 Rn:5  Rd:5
FMINNMV_asimdall_only_H A64_V 0 Q:1 00111010110000110010 Rn:5  Rd:5
FRSQRTE_asimdmiscfp16_R A64_V 0 Q:1 10111011111001110110 Rn:5  Rd:5
FMUL_H_floatdp2 A64_V 00011110111 Rm:5 000010 Rn:5  Rd:5
FMLS_asimdsamefp16_only A64_V 0 Q:1 001110110 Rm:5 000011 Rn:5  Rd:5
FDIV_asimdsamefp16_only A64_V 0 Q:1 101110010 Rm:5 001111 Rn:5  Rd:5
FSUB_H_floatdp2 A64_V 00011110111 Rm:5 001110 Rn:5  Rd:5
FMLS_asimdelem_RH_H A64_V 0 Q:1 00111100 L:1  M:1  Rm:4 0101 H:1 0 Rn:5  Rd:5
FMLA_asimdelem_RH_H A64_V 0 Q:1 00111100 L:1  M:1  Rm:4 0001 H:1 0 Rn:5  Rd:5
FCVTPU_asimdmiscfp16_R A64_V 0 Q:1 10111011111001101010 Rn:5  Rd:5
FCMLE_asimdmiscfp16_FZ A64_V 0 Q:1 10111011111000110110 Rn:5  Rd:5
FMLA_asisdelem_RH_H A64_V 0101111100 L:1  M:1  Rm:4 0001 H:1 0 Rn:5  Rd:5
FCVTMS_asimdmiscfp16_R A64_V 0 Q:1 00111001111001101110 Rn:5  Rd:5
FRINTM_H_floatdp1 A64_V 0001111011100101010000 Rn:5  Rd:5
FCVTMU_asimdmiscfp16_R A64_V 0 Q:1 10111001111001101110 Rn:5  Rd:5
FCMGE_asimdsamefp16_only A64_V 0 Q:1 101110010 Rm:5 001001 Rn:5  Rd:5
FRSQRTS_asisdsamefp16_only A64_V 01011110110 Rm:5 001111 Rn:5  Rd:5
FCMEQ_asisdsamefp16_only A64_V 01011110010 Rm:5 001001 Rn:5  Rd:5
FRINTZ_H_floatdp1 A64_V 0001111011100101110000 Rn:5  Rd:5
FCVTNS_asimdmiscfp16_R A64_V 0 Q:1 00111001111001101010 Rn:5  Rd:5
FRSQRTS_asimdsamefp16_only A64_V 0 Q:1 001110110 Rm:5 001111 Rn:5  Rd:5
FRINTX_H_floatdp1 A64_V 0001111011100111010000 Rn:5  Rd:5
FRINTA_H_floatdp1 A64_V 0001111011100110010000 Rn:5  Rd:5
FABD_asimdsamefp16_only A64_V 0 Q:1 101110110 Rm:5 000101 Rn:5  Rd:5
FCVTPU_asisdmiscfp16_R A64_V 0111111011111001101010 Rn:5  Rd:5
FACGE_asisdsamefp16_only A64_V 01111110010 Rm:5 001011 Rn:5  Rd:5
FACGT_asimdsamefp16_only A64_V 0 Q:1 101110110 Rm:5 001011 Rn:5  Rd:5
FCMEQ_asisdmiscfp16_FZ A64_V 0101111011111000110110 Rn:5  Rd:5
FCVTAS_asimdmiscfp16_R A64_V 0 Q:1 00111001111001110010 Rn:5  Rd:5
FMULX_asimdsamefp16_only A64_V 0 Q:1 001110010 Rm:5 000111 Rn:5  Rd:5
FMSUB_H_floatdp3 A64_V 00011111110 Rm:5 1 Ra:5  Rn:5  Rd:5
FCVTMU_asisdmiscfp16_R A64_V 0111111001111001101110 Rn:5  Rd:5
FMIN_H_floatdp2 A64_V 00011110111 Rm:5 010110 Rn:5  Rd:5
FSQRT_asimdmiscfp16_R A64_V 0 Q:1 10111011111001111110 Rn:5  Rd:5
FRINTX_asimdmiscfp16_R A64_V 0 Q:1 10111001111001100110 Rn:5  Rd:5
UCVTF_asimdmiscfp16_R A64_V 0 Q:1 10111001111001110110 Rn:5  Rd:5
FNMSUB_H_floatdp3 A64_V 00011111111 Rm:5 1 Ra:5  Rn:5  Rd:5
FCVTNU_asimdmiscfp16_R A64_V 0 Q:1 10111001111001101010 Rn:5  Rd:5
FMINNMP_asimdsamefp16_only A64_V 0 Q:1 101110110 Rm:5 000001 Rn:5  Rd:5
FRECPX_asisdmiscfp16_R A64_V 0101111011111001111110 Rn:5  Rd:5
FMULX_asisdsamefp16_only A64_V 01011110010 Rm:5 000111 Rn:5  Rd:5
FABS_H_floatdp1 A64_V 0001111011100000110000 Rn:5  Rd:5
FMAX_asimdsamefp16_only A64_V 0 Q:1 001110010 Rm:5 001101 Rn:5  Rd:5
FADD_H_floatdp2 A64_V 00011110111 Rm:5 001010 Rn:5  Rd:5
FRINTN_H_floatdp1 A64_V 0001111011100100010000 Rn:5  Rd:5
UCVTF_asisdmiscfp16_R A64_V 0111111001111001110110 Rn:5  Rd:5
FACGE_asimdsamefp16_only A64_V 0 Q:1 101110010 Rm:5 001011 Rn:5  Rd:5
FCMP_H_floatcmp A64_V 00011110111 Rm:5 001000 Rn:5 00000
FSQRT_H_floatdp1 A64_V 0001111011100001110000 Rn:5  Rd:5
FMAXV_asimdall_only_H A64_V 0 Q:1 00111000110000111110 Rn:5  Rd:5
FCVTPS_asimdmiscfp16_R A64_V 0 Q:1 00111011111001101010 Rn:5  Rd:5
FCMLE_asisdmiscfp16_FZ A64_V 0111111011111000110110 Rn:5  Rd:5
FMINV_asimdall_only_H A64_V 0 Q:1 00111010110000111110 Rn:5  Rd:5
FMINP_asisdpair_only_H A64_V 0101111010110000111110 Rn:5  Rd:5
FCMGT_asimdmiscfp16_FZ A64_V 0 Q:1 00111011111000110010 Rn:5  Rd:5
FCMP_HZ_floatcmp A64_V 00011110111 Rm:5 001000 Rn:5 01000
FRECPS_asisdsamefp16_only A64_V 01011110010 Rm:5 001111 Rn:5  Rd:5
FMOV_H_floatimm A64_V 00011110111 imm8:8 10000000 Rd:5
FCVTNS_asisdmiscfp16_R A64_V 0101111001111001101010 Rn:5  Rd:5
FMULX_asimdelem_RH_H A64_V 0 Q:1 10111100 L:1  M:1  Rm:4 1001 H:1 0 Rn:5  Rd:5
FRINTA_asimdmiscfp16_R A64_V 0 Q:1 10111001111001100010 Rn:5  Rd:5
FCVTAU_asimdmiscfp16_R A64_V 0 Q:1 10111001111001110010 Rn:5  Rd:5
FMIN_asimdsamefp16_only A64_V 0 Q:1 001110110 Rm:5 001101 Rn:5  Rd:5
FCMEQ_asimdmiscfp16_FZ A64_V 0 Q:1 00111011111000110110 Rn:5  Rd:5
FCMPE_HZ_floatcmp A64_V 00011110111 Rm:5 001000 Rn:5 11000
FCVTZU_asimdmiscfp16_R A64_V 0 Q:1 10111011111001101110 Rn:5  Rd:5
FNEG_H_floatdp1 A64_V 0001111011100001010000 Rn:5  Rd:5
FMOV_H_floatdp1 A64_V 0001111011100000010000 Rn:5  Rd:5

And the generated test cases and traces:

  http://people.linaro.org/~alex.bennee/testcases/arm64.risu/testcases.armv8.2_half.tar.xz

Currently I'm seeing failures on:

Failed 9 tests:
testcases.armv8.2_half/insn_FCCMP_H_floatccmp__INC.risu.bin
testcases.armv8.2_half/insn_FCCMPE_H_floatccmp__INC.risu.bin
testcases.armv8.2_half/insn_FCMP_H_floatcmp__INC.risu.bin
testcases.armv8.2_half/insn_FCMP_HZ_floatcmp__INC.risu.bin
testcases.armv8.2_half/insn_FCMPE_H_floatcmp__INC.risu.bin
testcases.armv8.2_half/insn_FCMPE_HZ_floatcmp__INC.risu.bin
testcases.armv8.2_half/insn_FCSEL_H_floatsel__INC.risu.bin
testcases.armv8.2_half/insn_FMOV_H_floatimm__INC.risu.bin
testcases.armv8.2_half/insn_FSQRT_H_floatdp1__INC.risu.bin

but I haven't checked to see if that is just instructions the FVP has in
full SVE mode that aren't in the just FP16 fixes branch I was testing
against.


>

> 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.

>

>

> r~

>

> PS: FWIW, this was written against my tgt-arm-sve-9 tree, since I

> was trying to test sve as generated by gcc.  I don't *think* there

> are any dependencies on any of the sve patches, but I didn't check.

>

> PPS: There are two more failures that might be qemu fp16 failures,

> but those are SIGSEGV.  This patch set cures all of the SIGILL and

> (subsequent) SIGABRT type failures within the testsuite.

>

>

> Richard Henderson (9):

>   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: Implement FP data-processing (2 source) for fp16

>   target/arm: Implement FP data-processing (3 source) for fp16

>

>  target/arm/helper.h        |   6 +

>  target/arm/helper.c        |  87 ++++++++++-

>  target/arm/translate-a64.c | 371 +++++++++++++++++++++++++++++++++++++--------

>  3 files changed, 399 insertions(+), 65 deletions(-)



--
Alex Bennée
Alex Bennée April 27, 2018, 6:55 p.m. UTC | #4
Alex Bennée <alex.bennee@linaro.org> writes:

> Richard Henderson <richard.henderson@linaro.org> writes:

>

>> 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.

>

> So this set of instructions is generated from any ASL description that

> contains "half":

<snip>
>

> Failed 9 tests:

> testcases.armv8.2_half/insn_FCCMP_H_floatccmp__INC.risu.bin

> testcases.armv8.2_half/insn_FCCMPE_H_floatccmp__INC.risu.bin

> testcases.armv8.2_half/insn_FCMP_H_floatcmp__INC.risu.bin

> testcases.armv8.2_half/insn_FCMP_HZ_floatcmp__INC.risu.bin

> testcases.armv8.2_half/insn_FCMPE_H_floatcmp__INC.risu.bin

> testcases.armv8.2_half/insn_FCMPE_HZ_floatcmp__INC.risu.bin

> testcases.armv8.2_half/insn_FCSEL_H_floatsel__INC.risu.bin


Well that looks like a whole class of compares we are missing. I'll get
to work on that.

> testcases.armv8.2_half/insn_FMOV_H_floatimm__INC.risu.bin

> testcases.armv8.2_half/insn_FSQRT_H_floatdp1__INC.risu.bin

>

> but I haven't checked to see if that is just instructions the FVP has in

> full SVE mode that aren't in the just FP16 fixes branch I was testing

> against.

>

>

>>

>> 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.

>>

>>

>> r~

>>

>> PS: FWIW, this was written against my tgt-arm-sve-9 tree, since I

>> was trying to test sve as generated by gcc.  I don't *think* there

>> are any dependencies on any of the sve patches, but I didn't check.

>>

>> PPS: There are two more failures that might be qemu fp16 failures,

>> but those are SIGSEGV.  This patch set cures all of the SIGILL and

>> (subsequent) SIGABRT type failures within the testsuite.

>>

>>

>> Richard Henderson (9):

>>   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: Implement FP data-processing (2 source) for fp16

>>   target/arm: Implement FP data-processing (3 source) for fp16

>>

>>  target/arm/helper.h        |   6 +

>>  target/arm/helper.c        |  87 ++++++++++-

>>  target/arm/translate-a64.c | 371 +++++++++++++++++++++++++++++++++++++--------

>>  3 files changed, 399 insertions(+), 65 deletions(-)



--
Alex Bennée
Alex Bennée April 27, 2018, 7:50 p.m. UTC | #5
Alex Bennée <alex.bennee@linaro.org> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:

>

>> Richard Henderson <richard.henderson@linaro.org> writes:

>>

<snip>

If you take my patches from:

  https://github.com/stsquad/qemu/tree/review/rth-fp16-fixes

443a7c3d38 arm/translate-a64: fix-up FMOV FP16 immediate
be4430e9c9 arm/translate-a64: add FP16 FCSEL
7badf508ab arm/translate-a64: add FP16 FCMP operations

>> testcases.armv8.2_half/insn_FSQRT_H_floatdp1__INC.risu.bi


This is the only failure I have left. As it is approaching 2100 on a
Friday night I leave that for you to look at as you'll be up before me
on Monday ;-)

--
Alex Bennée
Alex Bennée May 1, 2018, 3:47 p.m. UTC | #6
Richard Henderson <richard.henderson@linaro.org> writes:

> 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.


I've finished reviewing this patch set.

> PS: FWIW, this was written against my tgt-arm-sve-9 tree, since I

> was trying to test sve as generated by gcc.  I don't *think* there

> are any dependencies on any of the sve patches, but I didn't check.


They aren't.

>

> PPS: There are two more failures that might be qemu fp16 failures,

> but those are SIGSEGV.  This patch set cures all of the SIGILL and

> (subsequent) SIGABRT type failures within the testsuite.


I've also pushed the fix to sqrt_f16 (passing cpu_env rather than fpst).
Are you happy rolling my fixes into your v2?

>

>

> Richard Henderson (9):

>   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: Implement FP data-processing (2 source) for fp16

>   target/arm: Implement FP data-processing (3 source) for fp16

>

>  target/arm/helper.h        |   6 +

>  target/arm/helper.c        |  87 ++++++++++-

>  target/arm/translate-a64.c | 371 +++++++++++++++++++++++++++++++++++++--------

>  3 files changed, 399 insertions(+), 65 deletions(-)



--
Alex Bennée
Richard Henderson May 1, 2018, 6:35 p.m. UTC | #7
On 05/01/2018 08:47 AM, Alex Bennée wrote:
> I've also pushed the fix to sqrt_f16 (passing cpu_env rather than fpst).


Oops.

> Are you happy rolling my fixes into your v2?


Yes, I can do that.


r~
Richard Henderson May 11, 2018, 2:17 a.m. UTC | #8
On 04/27/2018 10:22 AM, Alex Bennée wrote:
> 

> Richard Henderson <richard.henderson@linaro.org> writes:

> 

>> 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.

> 

> So this set of instructions is generated from any ASL description that

> contains "half":


This still isn't all of them.  At least the insns from fmov_float_gen.html are
missing.  The four insns could be handled with

FMOV_H_general  A64_V sf:1 00 11110 11 100 11 o:1 000000 Rn:5 Rd:5

Peter had pointed out that I didn't handle sf=1 in fmov during review.


r~
Alex Bennée May 11, 2018, 9:13 p.m. UTC | #9
Richard Henderson <richard.henderson@linaro.org> writes:

> On 04/27/2018 10:22 AM, Alex Bennée wrote:

>>

>> Richard Henderson <richard.henderson@linaro.org> writes:

>>

>>> 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.

>>

>> So this set of instructions is generated from any ASL description that

>> contains "half":

>

> This still isn't all of them.  At least the insns from fmov_float_gen.html are

> missing.  The four insns could be handled with

>

> FMOV_H_general  A64_V sf:1 00 11110 11 100 11 o:1 000000 Rn:5 Rd:5

>

> Peter had pointed out that I didn't handle sf=1 in fmov during review.


It describes itself as:

   <docvars>
       <docvar key="convert-type" value="32-to-half" />
       <docvar key="instr-class" value="float" />
       <docvar key="isa" value="A64" />
       <docvar key="mnemonic" value="FMOV" />
   </docvars>

I found I had to make the --desc matching a lot more liberal to catch
stuff from the Aarch32 XML, so now:

22:11:47 [alex@zen:~/l/q/risu.git] add-asl-support-v2(+1/-1) + ./contrib/armasl2risu.py --only-desc "half" xml/ISA_v83A_A64_xml_00bet6/*.xml | grep FMOV
FMOV_32H_float2int A64_V 0001111011100110000000 Rn:5  Rd:5
FMOV_H32_float2int A64_V 0001111011100111000000 Rn:5  Rd:5
FMOV_asimdimm_H_h A64_V 0 Q:1 00111100000 a:1  b:1  c:1 111111 d:1  e:1  f:1  g:1  h:1  Rd:5
FMOV_H64_float2int A64_V 1001111011100111000000 Rn:5  Rd:5
FMOV_64H_float2int A64_V 1001111011100110000000 Rn:5  Rd:5
FMOV_H_floatimm A64_V 00011110111 imm8:8 10000000 Rd:5
FMOV_H_floatdp1 A64_V 0001111011100000010000 Rn:5  Rd:5

Whereas I had:

22:11:58 [alex@zen:~/l/q/risu.git] add-asl-support-v2(+1/-1) + grep FMOV all_v82_half.risu
FMOV_H_floatimm A64_V 00011110111 imm8:8 10000000 Rd:5
FMOV_H_floatdp1 A64_V 0001111011100000010000 Rn:5  Rd:5

I'll regenerate a set.

--
Alex Bennée