mbox series

[v1,00/20] s390x/tcg: Implement Vector enhancements facility and switch to z14

Message ID 20200930145523.71087-1-david@redhat.com
Headers show
Series s390x/tcg: Implement Vector enhancements facility and switch to z14 | expand

Message

David Hildenbrand Sept. 30, 2020, 2:55 p.m. UTC
This series adds support for the "Vector enhancements facility" and bumps
the qemu CPU model tp to a stripped-down z14.

I yet have to find a way to get more test coverage - looks like some of
the functions aren't used anywhere yet (e.g., VECTOR FP MAXIMUM), writing
unit tests to cover all functions and cases is just nasty. But I might be
wrong - I'm planning to at least test basic functionality of all new added
instructions.

I have to make excessive use of c macros again to cover different element
sizes (32/64/128bit). Any advise to clean things up are welcome.

This is based on:
    "[PATCH v2 0/9] s390x/tcg: Implement some z14 facilities"
    "[PATCH v2 00/10] softfloat: Implement float128_muladd"

Based-on: 20200928122717.30586-1-david@redhat.com
Based-on: 20200925152047.709901-1-richard.henderson@linaro.org

David Hildenbrand (20):
  softfloat: Implement
    float128_(min|minnum|minnummag|max|maxnum|maxnummag)
  s390x/tcg: Implement VECTOR BIT PERMUTE
  s390x/tcg: Implement VECTOR MULTIPLY SUM LOGICAL
  s390x/tcg: Implement 32/128 bit for VECTOR FP ADD
  s390x/tcg: Implement 32/128 bit for VECTOR FP DIVIDE
  s390x/tcg: Implement 32/128 bit for VECTOR FP MULTIPLY
  s390x/tcg: Implement 32/128 bit for VECTOR FP SUBTRACT
  s390x/tcg: Implement 32/128 bit for VECTOR FP COMPARE (AND SIGNAL)
    SCALAR
  s390x/tcg: Implement 32/128 bit for VECTOR FP COMPARE *
  s390x/tcg: Implement 32/128 bit for VECTOR LOAD FP INTEGER
  s390x/tcg: Implement 64 bit for VECTOR FP LOAD LENGTHENED
  s390x/tcg: Implement 128 bit for VECTOR FP LOAD ROUNDED
  s390x/tcg: Implement 32/128 bit for VECTOR FP PERFORM SIGN OPERATION
  s390x/tcg: Implement 32/128 bit for VECTOR FP SQUARE ROOT
  s390x/tcg: Implement 32/128 bit for VECTOR FP TEST DATA CLASS
    IMMEDIATE
  s390x/tcg: Implement 32/128bit for VECTOR FP MULTIPLY AND
    (ADD|SUBTRACT)
  s390x/tcg: Implement VECTOR FP NEGATIVE MULTIPLY AND (ADD|SUBTRACT)
  s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)
  s390x/tcg: We support Vector enhancements facility
  s390x/cpumodel: Bump up QEMU model to a stripped-down IBM z14 GA2

 fpu/softfloat.c                 |  100 +++
 hw/s390x/s390-virtio-ccw.c      |    2 +
 include/fpu/softfloat.h         |    6 +
 target/s390x/cpu_models.c       |    4 +-
 target/s390x/gen-features.c     |   14 +-
 target/s390x/helper.h           |   72 ++
 target/s390x/insn-data.def      |   12 +
 target/s390x/translate_vx.c.inc |  625 ++++++++++++---
 target/s390x/vec_fpu_helper.c   | 1302 ++++++++++++++++++++++---------
 target/s390x/vec_helper.c       |   22 +
 10 files changed, 1681 insertions(+), 478 deletions(-)

Comments

no-reply@patchew.org Sept. 30, 2020, 3:35 p.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20200930145523.71087-1-david@redhat.com/



Hi,

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

Type: series
Message-id: 20200930145523.71087-1-david@redhat.com
Subject: [PATCH v1 00/20] s390x/tcg: Implement Vector enhancements facility and switch to z14

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
855b893 s390x/cpumodel: Bump up QEMU model to a stripped-down IBM z14 GA2
9e2c5a3 s390x/tcg: We support Vector enhancements facility
91355af s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM)
22ae891 s390x/tcg: Implement VECTOR FP NEGATIVE MULTIPLY AND (ADD|SUBTRACT)
599e1d8 s390x/tcg: Implement 32/128bit for VECTOR FP MULTIPLY AND (ADD|SUBTRACT)
2d46f63 s390x/tcg: Implement 32/128 bit for VECTOR FP TEST DATA CLASS IMMEDIATE
7f46bf1 s390x/tcg: Implement 32/128 bit for VECTOR FP SQUARE ROOT
0870741 s390x/tcg: Implement 32/128 bit for VECTOR FP PERFORM SIGN OPERATION
743a4a3 s390x/tcg: Implement 128 bit for VECTOR FP LOAD ROUNDED
793d28c s390x/tcg: Implement 64 bit for VECTOR FP LOAD LENGTHENED
ff85384 s390x/tcg: Implement 32/128 bit for VECTOR LOAD FP INTEGER
de00280 s390x/tcg: Implement 32/128 bit for VECTOR FP COMPARE *
ba7aec7 s390x/tcg: Implement 32/128 bit for VECTOR FP COMPARE (AND SIGNAL) SCALAR
74af73a s390x/tcg: Implement 32/128 bit for VECTOR FP SUBTRACT
59a6f7a s390x/tcg: Implement 32/128 bit for VECTOR FP MULTIPLY
c0903b4 s390x/tcg: Implement 32/128 bit for VECTOR FP DIVIDE
8af7fca s390x/tcg: Implement 32/128 bit for VECTOR FP ADD
e103776 s390x/tcg: Implement VECTOR MULTIPLY SUM LOGICAL
092690a s390x/tcg: Implement VECTOR BIT PERMUTE
46d60ed softfloat: Implement float128_(min|minnum|minnummag|max|maxnum|maxnummag)

=== OUTPUT BEGIN ===
1/20 Checking commit 46d60ede64da (softfloat: Implement float128_(min|minnum|minnummag|max|maxnum|maxnummag))
2/20 Checking commit 092690a01e4b (s390x/tcg: Implement VECTOR BIT PERMUTE)
3/20 Checking commit e1037765c06f (s390x/tcg: Implement VECTOR MULTIPLY SUM LOGICAL)
4/20 Checking commit 8af7fcaee5ef (s390x/tcg: Implement 32/128 bit for VECTOR FP ADD)
ERROR: space prohibited between function name and open parenthesis '('
#162: FILE: target/s390x/vec_fpu_helper.c:140:
+typedef float##BITS (*vop##BITS##_3_fn)(float##BITS a, float##BITS b,          \

total: 1 errors, 0 warnings, 183 lines checked

Patch 4/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/20 Checking commit c0903b4e139a (s390x/tcg: Implement 32/128 bit for VECTOR FP DIVIDE)
6/20 Checking commit 59a6f7a3d4ca (s390x/tcg: Implement 32/128 bit for VECTOR FP MULTIPLY)
7/20 Checking commit 74af73a36417 (s390x/tcg: Implement 32/128 bit for VECTOR FP SUBTRACT)
8/20 Checking commit ba7aec76b6d7 (s390x/tcg: Implement 32/128 bit for VECTOR FP COMPARE (AND SIGNAL) SCALAR)
WARNING: Block comments use a leading /* on a separate line
#113: FILE: target/s390x/vec_fpu_helper.c:190:
+    /* only the zero-indexed elements are compared */                          \

total: 0 errors, 1 warnings, 136 lines checked

Patch 8/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/20 Checking commit de00280993e0 (s390x/tcg: Implement 32/128 bit for VECTOR FP COMPARE *)
WARNING: line over 80 characters
#26: FILE: target/s390x/helper.h:262:
+DEF_HELPER_FLAGS_5(gvec_vfce32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#29: FILE: target/s390x/helper.h:265:
+DEF_HELPER_FLAGS_5(gvec_vfce128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#36: FILE: target/s390x/helper.h:272:
+DEF_HELPER_FLAGS_5(gvec_vfch32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#39: FILE: target/s390x/helper.h:275:
+DEF_HELPER_FLAGS_5(gvec_vfch128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#45: FILE: target/s390x/helper.h:281:
+DEF_HELPER_FLAGS_5(gvec_vfche32, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#46: FILE: target/s390x/helper.h:282:
+DEF_HELPER_FLAGS_5(gvec_vfche32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#49: FILE: target/s390x/helper.h:285:
+DEF_HELPER_FLAGS_5(gvec_vfche128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: Block comments use a leading /* on a separate line
#250: FILE: target/s390x/vec_fpu_helper.c:250:
+        /* swap the parameters, so we can use existing functions */            \

total: 0 errors, 8 warnings, 445 lines checked

Patch 9/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/20 Checking commit ff85384a58d4 (s390x/tcg: Implement 32/128 bit for VECTOR LOAD FP INTEGER)
ERROR: space prohibited between function name and open parenthesis '('
#140: FILE: target/s390x/vec_fpu_helper.c:120:
+typedef float##BITS (*vop##BITS##_2_fn)(float##BITS a, float_status *s);       \

total: 1 errors, 0 warnings, 191 lines checked

Patch 10/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

11/20 Checking commit 793d28cefc57 (s390x/tcg: Implement 64 bit for VECTOR FP LOAD LENGTHENED)
12/20 Checking commit 743a4a3474e5 (s390x/tcg: Implement 128 bit for VECTOR FP LOAD ROUNDED)
13/20 Checking commit 08707417ca0e (s390x/tcg: Implement 32/128 bit for VECTOR FP PERFORM SIGN OPERATION)
14/20 Checking commit 7f46bf12ce3e (s390x/tcg: Implement 32/128 bit for VECTOR FP SQUARE ROOT)
15/20 Checking commit 2d46f639d6dd (s390x/tcg: Implement 32/128 bit for VECTOR FP TEST DATA CLASS IMMEDIATE)
16/20 Checking commit 599e1d8a08ff (s390x/tcg: Implement 32/128bit for VECTOR FP MULTIPLY AND (ADD|SUBTRACT))
WARNING: line over 80 characters
#20: FILE: target/s390x/helper.h:320:
+DEF_HELPER_FLAGS_6(gvec_vfma32, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#21: FILE: target/s390x/helper.h:321:
+DEF_HELPER_FLAGS_6(gvec_vfma32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#24: FILE: target/s390x/helper.h:324:
+DEF_HELPER_FLAGS_6(gvec_vfma128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#25: FILE: target/s390x/helper.h:325:
+DEF_HELPER_FLAGS_6(gvec_vfms32, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#26: FILE: target/s390x/helper.h:326:
+DEF_HELPER_FLAGS_6(gvec_vfms32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#29: FILE: target/s390x/helper.h:329:
+DEF_HELPER_FLAGS_6(gvec_vfms128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

total: 0 errors, 6 warnings, 188 lines checked

Patch 16/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/20 Checking commit 22ae891fa9a5 (s390x/tcg: Implement VECTOR FP NEGATIVE MULTIPLY AND (ADD|SUBTRACT))
WARNING: line over 80 characters
#20: FILE: target/s390x/helper.h:330:
+DEF_HELPER_FLAGS_6(gvec_vfnma32, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#21: FILE: target/s390x/helper.h:331:
+DEF_HELPER_FLAGS_6(gvec_vfnma32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#22: FILE: target/s390x/helper.h:332:
+DEF_HELPER_FLAGS_6(gvec_vfnma64, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#23: FILE: target/s390x/helper.h:333:
+DEF_HELPER_FLAGS_6(gvec_vfnma64s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#24: FILE: target/s390x/helper.h:334:
+DEF_HELPER_FLAGS_6(gvec_vfnma128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#25: FILE: target/s390x/helper.h:335:
+DEF_HELPER_FLAGS_6(gvec_vfnms32, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#26: FILE: target/s390x/helper.h:336:
+DEF_HELPER_FLAGS_6(gvec_vfnms32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#27: FILE: target/s390x/helper.h:337:
+DEF_HELPER_FLAGS_6(gvec_vfnms64, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#28: FILE: target/s390x/helper.h:338:
+DEF_HELPER_FLAGS_6(gvec_vfnms64s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#29: FILE: target/s390x/helper.h:339:
+DEF_HELPER_FLAGS_6(gvec_vfnms128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, cptr, env, i32)

total: 0 errors, 10 warnings, 109 lines checked

Patch 17/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/20 Checking commit 91355af73073 (s390x/tcg: Implement VECTOR FP (MAXIMUM|MINIMUM))
WARNING: line over 80 characters
#28: FILE: target/s390x/helper.h:320:
+DEF_HELPER_FLAGS_5(gvec_vfmax32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#29: FILE: target/s390x/helper.h:321:
+DEF_HELPER_FLAGS_5(gvec_vfmax32, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#30: FILE: target/s390x/helper.h:322:
+DEF_HELPER_FLAGS_5(gvec_vfmax64s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#31: FILE: target/s390x/helper.h:323:
+DEF_HELPER_FLAGS_5(gvec_vfmax64, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#32: FILE: target/s390x/helper.h:324:
+DEF_HELPER_FLAGS_5(gvec_vfmax128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#33: FILE: target/s390x/helper.h:325:
+DEF_HELPER_FLAGS_5(gvec_vfmin32s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#34: FILE: target/s390x/helper.h:326:
+DEF_HELPER_FLAGS_5(gvec_vfmin32, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#35: FILE: target/s390x/helper.h:327:
+DEF_HELPER_FLAGS_5(gvec_vfmin64s, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#36: FILE: target/s390x/helper.h:328:
+DEF_HELPER_FLAGS_5(gvec_vfmin64, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: line over 80 characters
#37: FILE: target/s390x/helper.h:329:
+DEF_HELPER_FLAGS_5(gvec_vfmin128, TCG_CALL_NO_WG, void, ptr, cptr, cptr, env, i32)

WARNING: Block comments use a leading /* on a separate line
#158: FILE: target/s390x/vec_fpu_helper.c:941:
+            /* fall through */                                                 \

WARNING: Block comments use a leading /* on a separate line
#211: FILE: target/s390x/vec_fpu_helper.c:994:
+    /* We can process all remaining cases using simple comparison. */          \

total: 0 errors, 12 warnings, 379 lines checked

Patch 18/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
19/20 Checking commit 9e2c5a3530bd (s390x/tcg: We support Vector enhancements facility)
20/20 Checking commit 855b893d0a4d (s390x/cpumodel: Bump up QEMU model to a stripped-down IBM z14 GA2)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200930145523.71087-1-david@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Richard Henderson Oct. 1, 2020, 3:07 p.m. UTC | #2
On 9/30/20 9:55 AM, David Hildenbrand wrote:
> This series adds support for the "Vector enhancements facility" and bumps
> the qemu CPU model tp to a stripped-down z14.
> 
> I yet have to find a way to get more test coverage - looks like some of
> the functions aren't used anywhere yet (e.g., VECTOR FP MAXIMUM), writing
> unit tests to cover all functions and cases is just nasty. But I might be
> wrong - I'm planning to at least test basic functionality of all new added
> instructions.

This is where RISU can be helpful.  Auto-generate 100k random variations,
record known good results on hardware, verify that replay on qemu produces the
same results.


r~
David Hildenbrand Oct. 7, 2020, 1:09 p.m. UTC | #3
On 01.10.20 17:07, Richard Henderson wrote:
> On 9/30/20 9:55 AM, David Hildenbrand wrote:
>> This series adds support for the "Vector enhancements facility" and bumps
>> the qemu CPU model tp to a stripped-down z14.
>>
>> I yet have to find a way to get more test coverage - looks like some of
>> the functions aren't used anywhere yet (e.g., VECTOR FP MAXIMUM), writing
>> unit tests to cover all functions and cases is just nasty. But I might be
>> wrong - I'm planning to at least test basic functionality of all new added
>> instructions.
> 
> This is where RISU can be helpful.  Auto-generate 100k random variations,
> record known good results on hardware, verify that replay on qemu produces the
> same results.

Makes sense, however, some corner cases (e.g., MAX(+0, -O)) still might
have to be handled manually.

It might take me a while to address the feedback (I'm  fairly busy as
usual ...). Thanks!