This patchseries provides an initial slice of the MVE
implementation. (MVE is "vector instructions for M-profile", also
known as Helium).

The series covers:
 * framework for MVE decode, including infrastructure for
   handling predication, PSR.ECI, etc
 * tail-predication forms of low-overhead-loop insns (LCTP, WLSTP, LETP)
 * basic (non-gather) loads and stores
 * pretty much all the integer 2-operand vector and scalar insns
 * most of the integer 1-operand insns
 * a handful of other insns
but is not (by a long way) complete MVE support, and this code
will remain 'dead' until the enable-MVE patch eventually lands.

Changes v1->v2:
 * Addressed code review comments
 * Where some style changes were suggested and made for patches at
   the beginning of the series I have retained the r-by tags for
   later patches which had minor changes to follow that style:
    - adding 'static const' for function pointer arrays
    - using mve_check_qreg_bank()
    - compressing the early-return-false and early-return-true
      checks in trans functions down to fewer lines
    - pass only ESIZE, not H, to macros in mve_helper.c
    - adjustments to handling of QC

Patches still in need of review are:
   04 "target/arm: Add handling for PSR.ECI/ICI"
   07 "target/arm: Implement MVE WLSTP insn"
   11 "target/arm: Implement MVE VLDR/VSTR (non-widening forms)"
   13 "target/arm: Move expand_pred_b() data to translate.c" (new patch)
   14 "target/arm: Implement MVE VCLZ"
   17 "target/arm: Implement MVE VREV16, VREV32, VREV64"
   19 "target/arm: Implement MVE VABS"
   21 "tcg: Make gen_dup_i32() public" (new patch)
   22 "target/arm: Implement MVE VDUP"
   34 "target/arm: Implement MVE VRMLALDAVH, VRMLSLDAVH"
   35 "target/arm: Implement MVE VADD (scalar)"
   45 "target/arm: Implement MVE VQSHL (vector)"
   53 "target/arm: Implement MVE VADC, VSBC"
   55 "target/arm: Implement MVE VHCADD"

Nobody seemed to object when I posted v1, so I propose to land
these via target-arm.next once they pass code review.

thanks
-- PMM

Peter Maydell (57):
  target/arm: Provide and use H8 and H1_8 macros
  target/arm: Enable FPSCR.QC bit for MVE
  target/arm: Handle VPR semantics in existing code
  target/arm: Add handling for PSR.ECI/ICI
  target/arm: Let vfp_access_check() handle late NOCP checks
  target/arm: Implement MVE LCTP
  target/arm: Implement MVE WLSTP insn
  target/arm: Implement MVE DLSTP
  target/arm: Implement MVE LETP insn
  target/arm: Add framework for MVE decode
  target/arm: Implement MVE VLDR/VSTR (non-widening forms)
  target/arm: Implement widening/narrowing MVE VLDR/VSTR insns
  target/arm: Move expand_pred_b() data to translate.c
  target/arm: Implement MVE VCLZ
  target/arm: Implement MVE VCLS
  bitops.h: Provide hswap32(), hswap64(), wswap64() swapping operations
  target/arm: Implement MVE VREV16, VREV32, VREV64
  target/arm: Implement MVE VMVN (register)
  target/arm: Implement MVE VABS
  target/arm: Implement MVE VNEG
  tcg: Make gen_dup_i32() public
  target/arm: Implement MVE VDUP
  target/arm: Implement MVE VAND, VBIC, VORR, VORN, VEOR
  target/arm: Implement MVE VADD, VSUB, VMUL
  target/arm: Implement MVE VMULH
  target/arm: Implement MVE VRMULH
  target/arm: Implement MVE VMAX, VMIN
  target/arm: Implement MVE VABD
  target/arm: Implement MVE VHADD, VHSUB
  target/arm: Implement MVE VMULL
  target/arm: Implement MVE VMLALDAV
  target/arm: Implement MVE VMLSLDAV
  include/qemu/int128.h: Add function to create Int128 from int64_t
  target/arm: Implement MVE VRMLALDAVH, VRMLSLDAVH
  target/arm: Implement MVE VADD (scalar)
  target/arm: Implement MVE VSUB, VMUL (scalar)
  target/arm: Implement MVE VHADD, VHSUB (scalar)
  target/arm: Implement MVE VBRSR
  target/arm: Implement MVE VPST
  target/arm: Implement MVE VQADD and VQSUB
  target/arm: Implement MVE VQDMULH and VQRDMULH (scalar)
  target/arm: Implement MVE VQDMULL scalar
  target/arm: Implement MVE VQDMULH, VQRDMULH (vector)
  target/arm: Implement MVE VQADD, VQSUB (vector)
  target/arm: Implement MVE VQSHL (vector)
  target/arm: Implement MVE VQRSHL
  target/arm: Implement MVE VSHL insn
  target/arm: Implmement MVE VRSHL
  target/arm: Implement MVE VQDMLADH and VQRDMLADH
  target/arm: Implement MVE VQDMLSDH and VQRDMLSDH
  target/arm: Implement MVE VQDMULL (vector)
  target/arm: Implement MVE VRHADD
  target/arm: Implement MVE VADC, VSBC
  target/arm: Implement MVE VCADD
  target/arm: Implement MVE VHCADD
  target/arm: Implement MVE VADDV
  target/arm: Make VMOV scalar <-> gpreg beatwise for MVE

 include/qemu/bitops.h         |   29 +
 include/qemu/int128.h         |   10 +
 include/tcg/tcg.h             |    3 +
 target/arm/helper-mve.h       |  357 ++++++++++
 target/arm/helper.h           |    2 +
 target/arm/internals.h        |   11 +
 target/arm/translate-a32.h    |    4 +
 target/arm/translate.h        |   19 +
 target/arm/vec_internal.h     |    9 +
 target/arm/mve.decode         |  260 ++++++++
 target/arm/t32.decode         |   15 +-
 target/arm/m_helper.c         |   54 +-
 target/arm/mve_helper.c       | 1175 +++++++++++++++++++++++++++++++++
 target/arm/sve_helper.c       |  381 ++++-------
 target/arm/translate-m-nocp.c |   16 +-
 target/arm/translate-mve.c    |  788 ++++++++++++++++++++++
 target/arm/translate-vfp.c    |  142 +++-
 target/arm/translate.c        |  300 ++++++++-
 target/arm/vec_helper.c       |  116 +++-
 target/arm/vfp_helper.c       |    3 +-
 tcg/tcg-op-gvec.c             |    4 +-
 target/arm/meson.build        |    3 +
 22 files changed, 3393 insertions(+), 308 deletions(-)
 create mode 100644 target/arm/helper-mve.h
 create mode 100644 target/arm/mve.decode
 create mode 100644 target/arm/mve_helper.c
 create mode 100644 target/arm/translate-mve.c

-- 
2.20.1

Patchew URL: https://patchew.org/QEMU/20210614151007.4545-1-peter.maydell@linaro.org/



Hi,

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

Type: series
Message-id: 20210614151007.4545-1-peter.maydell@linaro.org
Subject: [PATCH v2 00/57] target/arm: First slice of MVE implementation

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210614052623.1657103-1-f4bug@amsat.org -> patchew/20210614052623.1657103-1-f4bug@amsat.org
Switched to a new branch 'test'
114fdf4 target/arm: Make VMOV scalar <-> gpreg beatwise for MVE
3aadcfc target/arm: Implement MVE VADDV
6adb32d target/arm: Implement MVE VHCADD
2e99c0a target/arm: Implement MVE VCADD
a0493d8 target/arm: Implement MVE VADC, VSBC
977dce5 target/arm: Implement MVE VRHADD
ce1ac42 target/arm: Implement MVE VQDMULL (vector)
a5c3651 target/arm: Implement MVE VQDMLSDH and VQRDMLSDH
7a6e20d target/arm: Implement MVE VQDMLADH and VQRDMLADH
83b6df5 target/arm: Implmement MVE VRSHL
173b26f target/arm: Implement MVE VSHL insn
b48b35a target/arm: Implement MVE VQRSHL
b4e1e88 target/arm: Implement MVE VQSHL (vector)
c516319 target/arm: Implement MVE VQADD, VQSUB (vector)
bd980e5 target/arm: Implement MVE VQDMULH, VQRDMULH (vector)
eaa2cb1 target/arm: Implement MVE VQDMULL scalar
b6b168b target/arm: Implement MVE VQDMULH and VQRDMULH (scalar)
e1dbfde target/arm: Implement MVE VQADD and VQSUB
23f072f target/arm: Implement MVE VPST
9fbabfc target/arm: Implement MVE VBRSR
3ca182e target/arm: Implement MVE VHADD, VHSUB (scalar)
c0615d8 target/arm: Implement MVE VSUB, VMUL (scalar)
9646c71 target/arm: Implement MVE VADD (scalar)
e091fe1 target/arm: Implement MVE VRMLALDAVH, VRMLSLDAVH
04aee7c include/qemu/int128.h: Add function to create Int128 from int64_t
19f5544 target/arm: Implement MVE VMLSLDAV
7c74a0d target/arm: Implement MVE VMLALDAV
2eed292 target/arm: Implement MVE VMULL
71e60ea target/arm: Implement MVE VHADD, VHSUB
3a11f89 target/arm: Implement MVE VABD
3a95638 target/arm: Implement MVE VMAX, VMIN
d537c24 target/arm: Implement MVE VRMULH
35d0181 target/arm: Implement MVE VMULH
d3d5dcb target/arm: Implement MVE VADD, VSUB, VMUL
e1083b8 target/arm: Implement MVE VAND, VBIC, VORR, VORN, VEOR
af20e6a target/arm: Implement MVE VDUP
44b9fa5 tcg: Make gen_dup_i32() public
6acf6cf target/arm: Implement MVE VNEG
201f1e3 target/arm: Implement MVE VABS
ebce351 target/arm: Implement MVE VMVN (register)
2bf919a target/arm: Implement MVE VREV16, VREV32, VREV64
7b8105a0 bitops.h: Provide hswap32(), hswap64(), wswap64() swapping operations
111b6a7 target/arm: Implement MVE VCLS
8282e7f target/arm: Implement MVE VCLZ
900481d target/arm: Move expand_pred_b() data to translate.c
3c7f001 target/arm: Implement widening/narrowing MVE VLDR/VSTR insns
356b0df target/arm: Implement MVE VLDR/VSTR (non-widening forms)
5472a37 target/arm: Add framework for MVE decode
dd6a572 target/arm: Implement MVE LETP insn
d707539 target/arm: Implement MVE DLSTP
2fef29f target/arm: Implement MVE WLSTP insn
9f0aa8b target/arm: Implement MVE LCTP
4958c81 target/arm: Let vfp_access_check() handle late NOCP checks
605a2b4 target/arm: Add handling for PSR.ECI/ICI
82900b2 target/arm: Handle VPR semantics in existing code
9734264 target/arm: Enable FPSCR.QC bit for MVE
a6d9eb0 target/arm: Provide and use H8 and H1_8 macros

=== OUTPUT BEGIN ===
1/57 Checking commit a6d9eb077652 (target/arm: Provide and use H8 and H1_8 macros)
2/57 Checking commit 9734264e7ccd (target/arm: Enable FPSCR.QC bit for MVE)
3/57 Checking commit 82900b28d7cd (target/arm: Handle VPR semantics in existing code)
4/57 Checking commit 605a2b46fccc (target/arm: Add handling for PSR.ECI/ICI)
5/57 Checking commit 4958c8116393 (target/arm: Let vfp_access_check() handle late NOCP checks)
6/57 Checking commit 9f0aa8b6738e (target/arm: Implement MVE LCTP)
7/57 Checking commit 2fef29fbfced (target/arm: Implement MVE WLSTP insn)
8/57 Checking commit d707539d6307 (target/arm: Implement MVE DLSTP)
9/57 Checking commit dd6a5724a472 (target/arm: Implement MVE LETP insn)
10/57 Checking commit 5472a374b257 (target/arm: Add framework for MVE decode)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#42: 
new file mode 100644

total: 0 errors, 1 warnings, 77 lines checked

Patch 10/57 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/57 Checking commit 356b0df2f8db (target/arm: Implement MVE VLDR/VSTR (non-widening forms))
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#269: FILE: target/arm/mve_helper.c:134:
+        /*                                                              \

total: 0 errors, 2 warnings, 370 lines checked

Patch 11/57 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/57 Checking commit 3c7f0017f291 (target/arm: Implement widening/narrowing MVE VLDR/VSTR insns)
13/57 Checking commit 900481dd1b91 (target/arm: Move expand_pred_b() data to translate.c)
14/57 Checking commit 8282e7fba256 (target/arm: Implement MVE VCLZ)
WARNING: architecture specific defines should be avoided
#131: FILE: target/arm/mve_helper.c:241:
+#if defined(__OPTIMIZE__) && !defined(__SANITIZE_ADDRESS__)

ERROR: externs should be avoided in .c files
#132: FILE: target/arm/mve_helper.c:242:
+void unknown_mergemask_type(void *d, uint64_t r, uint16_t mask);

ERROR: spaces required around that '*' (ctx:WxV)
#188: FILE: target/arm/translate-mve.c:165:
+static bool do_1op(DisasContext *s, arg_1op *a, MVEGenOneOpFn fn)
                                             ^

ERROR: spaces required around that '*' (ctx:WxV)
#212: FILE: target/arm/translate-mve.c:189:
+    static bool trans_##INSN(DisasContext *s, arg_1op *a)       \
                                                       ^

total: 3 errors, 1 warnings, 178 lines checked

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

15/57 Checking commit 111b6a710d25 (target/arm: Implement MVE VCLS)
16/57 Checking commit 7b8105a005cd (bitops.h: Provide hswap32(), hswap64(), wswap64() swapping operations)
17/57 Checking commit 2bf919a79f1e (target/arm: Implement MVE VREV16, VREV32, VREV64)
ERROR: spaces required around that '*' (ctx:WxV)
#69: FILE: target/arm/translate-mve.c:203:
+static bool trans_VREV16(DisasContext *s, arg_1op *a)
                                                   ^

total: 1 errors, 0 warnings, 63 lines checked

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

18/57 Checking commit ebce351bd7dc (target/arm: Implement MVE VMVN (register))
19/57 Checking commit 201f1e38ed0b (target/arm: Implement MVE VABS)
20/57 Checking commit 6acf6cfb95b5 (target/arm: Implement MVE VNEG)
21/57 Checking commit 44b9fa52d918 (tcg: Make gen_dup_i32() public)
22/57 Checking commit af20e6af104e (target/arm: Implement MVE VDUP)
ERROR: spaces required around that '*' (ctx:WxV)
#93: FILE: target/arm/translate-mve.c:165:
+static bool trans_VDUP(DisasContext *s, arg_VDUP *a)
                                                  ^

total: 1 errors, 0 warnings, 82 lines checked

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

23/57 Checking commit e1083b8947f8 (target/arm: Implement MVE VAND, VBIC, VORR, VORN, VEOR)
24/57 Checking commit d3d5dcb4011e (target/arm: Implement MVE VADD, VSUB, VMUL)
25/57 Checking commit 35d01815ebb4 (target/arm: Implement MVE VMULH)
26/57 Checking commit d537c24726e2 (target/arm: Implement MVE VRMULH)
27/57 Checking commit 3a95638ded87 (target/arm: Implement MVE VMAX, VMIN)
28/57 Checking commit 3a11f89e6207 (target/arm: Implement MVE VABD)
29/57 Checking commit 71e60eace31e (target/arm: Implement MVE VHADD, VHSUB)
30/57 Checking commit 2eed2920273b (target/arm: Implement MVE VMULL)
WARNING: line over 80 characters
#72: FILE: target/arm/mve_helper.c:373:
+    void HELPER(glue(mve_, OP))(CPUARMState *env, void *vd, void *vn, void *vm) \

total: 0 errors, 1 warnings, 81 lines checked

Patch 30/57 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
31/57 Checking commit 7c74a0dbd0f1 (target/arm: Implement MVE VMLALDAV)
ERROR: spaces required around that '+=' (ctx:WxB)
#96: FILE: target/arm/mve_helper.c:533:
+DO_LDAV(vmlaldavsh, 2, int16_t, false, +=, +=)
                                            ^

ERROR: spaces required around that '+=' (ctx:WxB)
#97: FILE: target/arm/mve_helper.c:534:
+DO_LDAV(vmlaldavxsh, 2, int16_t, true, +=, +=)
                                            ^

ERROR: spaces required around that '+=' (ctx:WxB)
#98: FILE: target/arm/mve_helper.c:535:
+DO_LDAV(vmlaldavsw, 4, int32_t, false, +=, +=)
                                            ^

ERROR: spaces required around that '+=' (ctx:WxB)
#99: FILE: target/arm/mve_helper.c:536:
+DO_LDAV(vmlaldavxsw, 4, int32_t, true, +=, +=)
                                            ^

ERROR: spaces required around that '+=' (ctx:WxB)
#101: FILE: target/arm/mve_helper.c:538:
+DO_LDAV(vmlaldavuh, 2, uint16_t, false, +=, +=)
                                             ^

ERROR: spaces required around that '+=' (ctx:WxB)
#102: FILE: target/arm/mve_helper.c:539:
+DO_LDAV(vmlaldavuw, 4, uint32_t, false, +=, +=)
                                             ^

WARNING: line over 80 characters
#111: FILE: target/arm/translate-mve.c:34:
+typedef void MVEGenDualAccOpFn(TCGv_i64, TCGv_ptr, TCGv_ptr, TCGv_ptr, TCGv_i64);

ERROR: spaces required around that '*' (ctx:WxV)
#143: FILE: target/arm/translate-mve.c:386:
+static bool do_long_dual_acc(DisasContext *s, arg_vmlaldav *a,
                                                            ^

total: 7 errors, 1 warnings, 199 lines checked

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

32/57 Checking commit 19f55441f40f (target/arm: Implement MVE VMLSLDAV)
ERROR: spaces required around that '-=' (ctx:WxB)
#53: FILE: target/arm/mve_helper.c:541:
+DO_LDAV(vmlsldavsh, 2, int16_t, false, +=, -=)
                                            ^

ERROR: spaces required around that '-=' (ctx:WxB)
#54: FILE: target/arm/mve_helper.c:542:
+DO_LDAV(vmlsldavxsh, 2, int16_t, true, +=, -=)
                                            ^

ERROR: spaces required around that '-=' (ctx:WxB)
#55: FILE: target/arm/mve_helper.c:543:
+DO_LDAV(vmlsldavsw, 4, int32_t, false, +=, -=)
                                            ^

ERROR: spaces required around that '-=' (ctx:WxB)
#56: FILE: target/arm/mve_helper.c:544:
+DO_LDAV(vmlsldavxsw, 4, int32_t, true, +=, -=)
                                            ^

total: 4 errors, 0 warnings, 35 lines checked

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

33/57 Checking commit 04aee7cb7f46 (include/qemu/int128.h: Add function to create Int128 from int64_t)
34/57 Checking commit e091fe197616 (target/arm: Implement MVE VRMLALDAVH, VRMLSLDAVH)
WARNING: line over 80 characters
#101: FILE: target/arm/mve_helper.c:575:
+DO_LDAVH(vrmlaldavhsw, 4, int32_t, false, int128_add, int128_add, int128_makes64)

WARNING: line over 80 characters
#102: FILE: target/arm/mve_helper.c:576:
+DO_LDAVH(vrmlaldavhxsw, 4, int32_t, true, int128_add, int128_add, int128_makes64)

WARNING: line over 80 characters
#104: FILE: target/arm/mve_helper.c:578:
+DO_LDAVH(vrmlaldavhuw, 4, uint32_t, false, int128_add, int128_add, int128_make64)

WARNING: line over 80 characters
#106: FILE: target/arm/mve_helper.c:580:
+DO_LDAVH(vrmlsldavhsw, 4, int32_t, false, int128_add, int128_sub, int128_makes64)

WARNING: line over 80 characters
#107: FILE: target/arm/mve_helper.c:581:
+DO_LDAVH(vrmlsldavhxsw, 4, int32_t, true, int128_add, int128_sub, int128_makes64)

total: 0 errors, 5 warnings, 98 lines checked

Patch 34/57 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
35/57 Checking commit 9646c71b2673 (target/arm: Implement MVE VADD (scalar))
ERROR: spaces required around that '*' (ctx:WxV)
#112: FILE: target/arm/translate-mve.c:387:
+static bool do_2op_scalar(DisasContext *s, arg_2scalar *a,
                                                        ^

ERROR: spaces required around that '*' (ctx:WxV)
#143: FILE: target/arm/translate-mve.c:418:
+    static bool trans_##INSN(DisasContext *s, arg_2scalar *a)   \
                                                           ^

total: 2 errors, 0 warnings, 117 lines checked

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

36/57 Checking commit c0615d8a1141 (target/arm: Implement MVE VSUB, VMUL (scalar))
37/57 Checking commit 3ca182e4dc1a (target/arm: Implement MVE VHADD, VHSUB (scalar))
38/57 Checking commit 9fbabfc2828a (target/arm: Implement MVE VBRSR)
39/57 Checking commit 23f072f8ee89 (target/arm: Implement MVE VPST)
40/57 Checking commit e1dbfde6b6eb (target/arm: Implement MVE VQADD and VQSUB)
41/57 Checking commit b6b168b49ca4 (target/arm: Implement MVE VQDMULH and VQRDMULH (scalar))
WARNING: line over 80 characters
#29: FILE: target/arm/helper-mve.h:192:
+DEF_HELPER_FLAGS_4(mve_vqdmulh_scalarb, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

WARNING: line over 80 characters
#30: FILE: target/arm/helper-mve.h:193:
+DEF_HELPER_FLAGS_4(mve_vqdmulh_scalarh, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

WARNING: line over 80 characters
#31: FILE: target/arm/helper-mve.h:194:
+DEF_HELPER_FLAGS_4(mve_vqdmulh_scalarw, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

WARNING: line over 80 characters
#33: FILE: target/arm/helper-mve.h:196:
+DEF_HELPER_FLAGS_4(mve_vqrdmulh_scalarb, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

WARNING: line over 80 characters
#34: FILE: target/arm/helper-mve.h:197:
+DEF_HELPER_FLAGS_4(mve_vqrdmulh_scalarh, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

WARNING: line over 80 characters
#35: FILE: target/arm/helper-mve.h:198:
+DEF_HELPER_FLAGS_4(mve_vqrdmulh_scalarw, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

total: 0 errors, 6 warnings, 68 lines checked

Patch 41/57 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
42/57 Checking commit eaa2cb1fe6de (target/arm: Implement MVE VQDMULL scalar)
WARNING: line over 80 characters
#32: FILE: target/arm/helper-mve.h:204:
+DEF_HELPER_FLAGS_4(mve_vqdmullb_scalarh, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

WARNING: line over 80 characters
#33: FILE: target/arm/helper-mve.h:205:
+DEF_HELPER_FLAGS_4(mve_vqdmullb_scalarw, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

WARNING: line over 80 characters
#34: FILE: target/arm/helper-mve.h:206:
+DEF_HELPER_FLAGS_4(mve_vqdmullt_scalarh, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

WARNING: line over 80 characters
#35: FILE: target/arm/helper-mve.h:207:
+DEF_HELPER_FLAGS_4(mve_vqdmullt_scalarw, TCG_CALL_NO_WG, void, env, ptr, ptr, i32)

ERROR: spaces required around that '*' (ctx:WxV)
#177: FILE: target/arm/translate-mve.c:457:
+static bool trans_VQDMULLB_scalar(DisasContext *s, arg_2scalar *a)
                                                                ^

total: 1 errors, 4 warnings, 164 lines checked

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

43/57 Checking commit bd980e533eb3 (target/arm: Implement MVE VQDMULH, VQRDMULH (vector))
WARNING: line over 80 characters
#61: FILE: target/arm/mve_helper.c:389:
+    void HELPER(glue(mve_, OP))(CPUARMState *env, void *vd, void *vn, void *vm) \

total: 0 errors, 1 warnings, 70 lines checked

Patch 43/57 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
44/57 Checking commit c5163194bfe9 (target/arm: Implement MVE VQADD, VQSUB (vector))
45/57 Checking commit b4e1e889b1c0 (target/arm: Implement MVE VQSHL (vector))
46/57 Checking commit b48b35a2c541 (target/arm: Implement MVE VQRSHL)
47/57 Checking commit 173b26f91b23 (target/arm: Implement MVE VSHL insn)
48/57 Checking commit 83b6df5fd43a (target/arm: Implmement MVE VRSHL)
49/57 Checking commit 7a6e20d3dff0 (target/arm: Implement MVE VQDMLADH and VQRDMLADH)
50/57 Checking commit a5c36516cb5d (target/arm: Implement MVE VQDMLSDH and VQRDMLSDH)
51/57 Checking commit ce1ac427a479 (target/arm: Implement MVE VQDMULL (vector))
52/57 Checking commit 977dce5544b7 (target/arm: Implement MVE VRHADD)
53/57 Checking commit a0493d8edfda (target/arm: Implement MVE VADC, VSBC)
54/57 Checking commit 2e99c0a347ac (target/arm: Implement MVE VCADD)
WARNING: line over 80 characters
#74: FILE: target/arm/mve_helper.c:608:
+    void HELPER(glue(mve_, OP))(CPUARMState *env, void *vd, void *vn, void *vm) \

WARNING: Block comments use a leading /* on a separate line
#80: FILE: target/arm/mve_helper.c:614:
+        /* Calculate all results first to avoid overwriting inputs */   \

total: 0 errors, 2 warnings, 78 lines checked

Patch 54/57 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
55/57 Checking commit 6adb32d6e8ea (target/arm: Implement MVE VHCADD)
56/57 Checking commit 3aadcfcc356e (target/arm: Implement MVE VADDV)
57/57 Checking commit 114fdf44a92f (target/arm: Make VMOV scalar <-> gpreg beatwise for MVE)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210614151007.4545-1-peter.maydell@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

On 6/14/21 8:09 AM, Peter Maydell wrote:
>      - pass only ESIZE, not H, to macros in mve_helper.c

I've been thinking about the H* macros again while reading this.

While H##ESIZE is an improvement over passing in HN, I think we can do better and force 
the adjustment to match the type -- completely avoiding bugs you've caught at least twice 
during SVE review.

The form I'm playing with today is

#ifdef HOST_WORDS_BIGENDIAN
#define HTADJ(p) (7 >> __builtin_ctz(sizeof(*(p))))
#define HBADJ(p) (7 & (7 << __builtin_ctz(sizeof(*(p)))))
#else
#define HTADJ(p) 0
#define HBADJ(p) 0
#endif

/**
  * HT: adjust Host addressing by Type
  * @p: data pointer
  * @i: array index
  *
  * Adjust p[i] for host-endian addressing of sub-quadword values.
  */
#define HT(p, i)  ((p)[(i) ^ HADJ(p)])

/**
  * HB: adjust Host addressing by Bype
  * @p: data pointer
  * @i: byte offset
  *
  * Adjust p[i / sizeof(*p)] for host-endian addressing
  * of sub-quadword values.  Unlike HT, @i is not an array
  * index but a byte offset.
  */
#define HB(p, i) (*(__typeof(p))((uintptr_t)(p) + ((i) ^ H1ADJ(p))))

void bt(unsigned char  *x, long i) { H(x, i) = 0; }
void ht(unsigned short *x, long i) { H(x, i) = 0; }
void wt(unsigned int   *x, long i) { H(x, i) = 0; }
void qt(unsigned long  *x, long i) { H(x, i) = 0; }

void bb(unsigned char  *x, long i) { H1(x, i) = 0; }
void hb(unsigned short *x, long i) { H1(x, i) = 0; }
void wb(unsigned int   *x, long i) { H1(x, i) = 0; }
void qb(unsigned long  *x, long i) { H1(x, i) = 0; }

This gives us

#define DO_1OP(OP, TYPE, FN)                                            \
     void HELPER(mve_##OP)(CPUARMState *env, void *vd, void *vm)         \
     {                                                                   \
         TYPE *d = vd, *m = vm;                                          \
         uint16_t mask = mve_element_mask(env);                          \
         unsigned e;                                                     \
         unsigned const esize = sizeof(TYPE);                            \
         for (e = 0; e < 16 / esize; e++, mask >>= esize) {              \
             mergemask(&HT(d, e), FN(HT(m, e)]), mask);                  \
         }                                                               \
         mve_advance_vpt(env);                                           \
     }

Thoughts?  Especially on the naming of the macros?
If the idea appeals, I'll do a pass over the existing code.


r~

On Mon, 14 Jun 2021 at 23:22, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/14/21 8:09 AM, Peter Maydell wrote:
> >      - pass only ESIZE, not H, to macros in mve_helper.c
>
> I've been thinking about the H* macros again while reading this.
>
> While H##ESIZE is an improvement over passing in HN, I think we can do better and force
> the adjustment to match the type -- completely avoiding bugs you've caught at least twice
> during SVE review.
>
> The form I'm playing with today is
>
> #ifdef HOST_WORDS_BIGENDIAN
> #define HTADJ(p) (7 >> __builtin_ctz(sizeof(*(p))))
> #define HBADJ(p) (7 & (7 << __builtin_ctz(sizeof(*(p)))))
> #else
> #define HTADJ(p) 0
> #define HBADJ(p) 0
> #endif
>
> /**
>   * HT: adjust Host addressing by Type
>   * @p: data pointer
>   * @i: array index
>   *
>   * Adjust p[i] for host-endian addressing of sub-quadword values.
>   */
> #define HT(p, i)  ((p)[(i) ^ HADJ(p)])
>
> /**
>   * HB: adjust Host addressing by Bype
>   * @p: data pointer
>   * @i: byte offset
>   *
>   * Adjust p[i / sizeof(*p)] for host-endian addressing
>   * of sub-quadword values.  Unlike HT, @i is not an array
>   * index but a byte offset.
>   */
> #define HB(p, i) (*(__typeof(p))((uintptr_t)(p) + ((i) ^ H1ADJ(p))))
>
> void bt(unsigned char  *x, long i) { H(x, i) = 0; }
> void ht(unsigned short *x, long i) { H(x, i) = 0; }
> void wt(unsigned int   *x, long i) { H(x, i) = 0; }
> void qt(unsigned long  *x, long i) { H(x, i) = 0; }
>
> void bb(unsigned char  *x, long i) { H1(x, i) = 0; }
> void hb(unsigned short *x, long i) { H1(x, i) = 0; }
> void wb(unsigned int   *x, long i) { H1(x, i) = 0; }
> void qb(unsigned long  *x, long i) { H1(x, i) = 0; }

What are these functions for ?

> This gives us
>
> #define DO_1OP(OP, TYPE, FN)                                            \
>      void HELPER(mve_##OP)(CPUARMState *env, void *vd, void *vm)         \
>      {                                                                   \
>          TYPE *d = vd, *m = vm;                                          \
>          uint16_t mask = mve_element_mask(env);                          \
>          unsigned e;                                                     \
>          unsigned const esize = sizeof(TYPE);                            \
>          for (e = 0; e < 16 / esize; e++, mask >>= esize) {              \
>              mergemask(&HT(d, e), FN(HT(m, e)]), mask);                  \
>          }                                                               \
>          mve_advance_vpt(env);                                           \
>      }
>
> Thoughts?  Especially on the naming of the macros?
> If the idea appeals, I'll do a pass over the existing code.

Getting rid of the need to keep matches between H macros and
the types certainly sounds like a good idea. I don't have a strong
view on the macro names -- they're always going to be a bit opaque
because we want to give them short names.

-- PMM

On 6/21/21 9:37 AM, Peter Maydell wrote:
>> void bt(unsigned char  *x, long i) { H(x, i) = 0; }
>> void ht(unsigned short *x, long i) { H(x, i) = 0; }
>> void wt(unsigned int   *x, long i) { H(x, i) = 0; }
>> void qt(unsigned long  *x, long i) { H(x, i) = 0; }
>>
>> void bb(unsigned char  *x, long i) { H1(x, i) = 0; }
>> void hb(unsigned short *x, long i) { H1(x, i) = 0; }
>> void wb(unsigned int   *x, long i) { H1(x, i) = 0; }
>> void qb(unsigned long  *x, long i) { H1(x, i) = 0; }
> 
> What are these functions for ?

Ah, sorry, testing.  Just looking at the -S output.


r~