[PATCH v2 00/17] target/arm: Convert rest of Neon 3-reg-same to decodetree

Peter Maydell posted 17 patches 3 years, 11 months ago
Test docker-mingw@fedora passed
Test checkpatch failed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200512163904.10918-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/helper.h             |   7 +-
target/arm/neon-dp.decode       | 113 +++++-
target/arm/neon_helper.c        |   7 -
target/arm/translate-neon.inc.c | 639 ++++++++++++++++++++++++++++++++
target/arm/translate.c          | 495 +------------------------
target/arm/vec_helper.c         |   7 +
target/arm/vfp_helper.c         |   4 +-
7 files changed, 767 insertions(+), 505 deletions(-)
[PATCH v2 00/17] target/arm: Convert rest of Neon 3-reg-same to decodetree
Posted by Peter Maydell 3 years, 11 months ago
This patchset is v2 of the Neon decodetree conversion. The first
half of v1 is in master already, so we're left just with patches
converting the rest of the 3-reg-same Neon dp insn group.

Based-on: <20200508152200.6547-1-richard.henderson@linaro.org>
("[PATCH v3 00/16] target/arm: partial vector cleanup")
Strictly speaking, based on that with a fixup for the VRSRA bug,
but I think patchew should be placated by that based-on tag.

Git tree available at:
 https://git.linaro.org/people/peter.maydell/qemu-arm.git neon-decodetree
with the whole patchstack including RTH's series.

Changes from v1:
 * the first 19 or so patches have been upstreamed
 * patch 1 (VQRDMLAH/VQRDMLSH) uses do_3same() now
 * patch 3 (64-bit elt 3-reg-same): shifts now use a format
   which swaps Vn and Vm in decode, so we don't need to special
   case them in the C code. We also use the gvec interface
   rather than hand-rolling a for-each-pass loop.
 * patch 4: make DO_3SAME_32() handle just one trans fn rather
   than doing both _U and _S in one macro invocation.
   Use gvec rather than hand-rolling the for-each-pass loop.
   patch 5: (vaba/vabd): new, since rth's patchset rewrote how these
   were handled and they can now just be handled via DO_3SAME_NO_SZ_3()
 * patch 7: saturating shift handling rewritten to use the gvec APIs.
 * patch 10 (vqdmulh/vqrdmulh): rewritten to use gvec
 * patch 11 (vadd/vsub): rewritten to use gvec; vabd now in an earlier
   patch with vaba
 * patch 13 (vmul/vmla/vmls): vmul uses gvec; do_3same_fp() and
   DO_3S_FP() now added in this patch as it is the first user
 * new patch 15 making recps_f32 and rsqrts_f32 easier to use with
   common gvec APIs and macros by moving the 'env' argument to the front
 * patch 16: updated VRECPS/VRSQRTS code to use gvec

NB: I have not attempted to merge VQSHL_S and VQSHL_S64 into
one pattern in patch 7 (as suggested in review of v1) -- adding
yet another DO_3SAME_FOO for the case of "64 bit and 32 bit
can be done with same trans/gen fn" didn't seem worthwhile.

thanks
-- PMM

Peter Maydell (17):
  target/arm: Convert Neon 3-reg-same VQRDMLAH/VQRDMLSH to decodetree
  target/arm: Convert Neon 3-reg-same SHA to decodetree
  target/arm: Convert Neon 64-bit element 3-reg-same insns
  target/arm: Convert Neon VHADD 3-reg-same insns
  target/arm: Convert Neon VABA/VABD 3-reg-same to decodetree
  target/arm: Convert Neon VRHADD, VHSUB 3-reg-same insns to decodetree
  target/arm: Convert Neon VQSHL, VRSHL, VQRSHL 3-reg-same insns to
    decodetree
  target/arm: Convert Neon VPMAX/VPMIN 3-reg-same insns to decodetree
  target/arm: Convert Neon VPADD 3-reg-same insns to decodetree
  target/arm: Convert Neon VQDMULH/VQRDMULH 3-reg-same to decodetree
  target/arm: Convert Neon VADD, VSUB, VABD 3-reg-same insns to
    decodetree
  target/arm: Convert Neon VPMIN/VPMAX/VPADD float 3-reg-same insns to
    decodetree
  target/arm: Convert Neon fp VMUL, VMLA, VMLS 3-reg-same insns to
    decodetree
  target/arm: Convert Neon 3-reg-same compare insns to decodetree
  target/arm: Move 'env' argument of recps_f32 and rsqrts_f32 helpers to
    usual place
  target/arm: Convert Neon fp VMAX/VMIN/VMAXNM/VMINNM/VRECPS/VRSQRTS to
    decodetree
  target/arm: Convert NEON VFMA, VFMS 3-reg-same insns to decodetree

 target/arm/helper.h             |   7 +-
 target/arm/neon-dp.decode       | 113 +++++-
 target/arm/neon_helper.c        |   7 -
 target/arm/translate-neon.inc.c | 639 ++++++++++++++++++++++++++++++++
 target/arm/translate.c          | 495 +------------------------
 target/arm/vec_helper.c         |   7 +
 target/arm/vfp_helper.c         |   4 +-
 7 files changed, 767 insertions(+), 505 deletions(-)

-- 
2.20.1


Re: [PATCH v2 00/17] target/arm: Convert rest of Neon 3-reg-same to decodetree
Posted by no-reply@patchew.org 3 years, 11 months ago
Patchew URL: https://patchew.org/QEMU/20200512163904.10918-1-peter.maydell@linaro.org/



Hi,

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

Message-id: 20200512163904.10918-1-peter.maydell@linaro.org
Subject: [PATCH v2 00/17] target/arm: Convert rest of Neon 3-reg-same to decodetree
Type: series

=== 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'
baa5469 target/arm: Convert NEON VFMA, VFMS 3-reg-same insns to decodetree
5b54865 target/arm: Convert Neon fp VMAX/VMIN/VMAXNM/VMINNM/VRECPS/VRSQRTS to decodetree
d72bf92 target/arm: Move 'env' argument of recps_f32 and rsqrts_f32 helpers to usual place
87ce7b2 target/arm: Convert Neon 3-reg-same compare insns to decodetree
3613a84 target/arm: Convert Neon fp VMUL, VMLA, VMLS 3-reg-same insns to decodetree
1c588e7 target/arm: Convert Neon VPMIN/VPMAX/VPADD float 3-reg-same insns to decodetree
0cc39b7 target/arm: Convert Neon VADD, VSUB, VABD 3-reg-same insns to decodetree
b2fa781 target/arm: Convert Neon VQDMULH/VQRDMULH 3-reg-same to decodetree
3015d59 target/arm: Convert Neon VPADD 3-reg-same insns to decodetree
a4747ac target/arm: Convert Neon VPMAX/VPMIN 3-reg-same insns to decodetree
6886825 target/arm: Convert Neon VQSHL, VRSHL, VQRSHL 3-reg-same insns to decodetree
a52dbe7 target/arm: Convert Neon VRHADD, VHSUB 3-reg-same insns to decodetree
f2f6dd6 target/arm: Convert Neon VABA/VABD 3-reg-same to decodetree
d4559f0 target/arm: Convert Neon VHADD 3-reg-same insns
876d7c9 target/arm: Convert Neon 64-bit element 3-reg-same insns
acac746 target/arm: Convert Neon 3-reg-same SHA to decodetree
7c693fa target/arm: Convert Neon 3-reg-same VQRDMLAH/VQRDMLSH to decodetree

=== OUTPUT BEGIN ===
1/17 Checking commit 7c693fa75b77 (target/arm: Convert Neon 3-reg-same VQRDMLAH/VQRDMLSH to decodetree)
ERROR: spaces required around that '*' (ctx:WxV)
#37: FILE: target/arm/translate-neon.inc.c:676:
+    static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a)        \
                                                              ^

total: 1 errors, 0 warnings, 50 lines checked

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

2/17 Checking commit acac7465d7c4 (target/arm: Convert Neon 3-reg-same SHA to decodetree)
ERROR: spaces required around that '*' (ctx:WxV)
#43: FILE: target/arm/translate-neon.inc.c:690:
+static bool trans_SHA1_3s(DisasContext *s, arg_SHA1_3s *a)
                                                        ^

total: 1 errors, 0 warnings, 220 lines checked

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

3/17 Checking commit 876d7c9fec85 (target/arm: Convert Neon 64-bit element 3-reg-same insns)
4/17 Checking commit d4559f0c2349 (target/arm: Convert Neon VHADD 3-reg-same insns)
ERROR: spaces required around that '*' (ctx:WxV)
#48: FILE: target/arm/translate-neon.inc.c:866:
+    static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a)        \
                                                              ^

total: 1 errors, 0 warnings, 51 lines checked

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

5/17 Checking commit f2f6dd6b5d9c (target/arm: Convert Neon VABA/VABD 3-reg-same to decodetree)
6/17 Checking commit a52dbe7662e7 (target/arm: Convert Neon VRHADD, VHSUB 3-reg-same insns to decodetree)
7/17 Checking commit 6886825aa2b5 (target/arm: Convert Neon VQSHL, VRSHL, VQRSHL 3-reg-same insns to decodetree)
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#103: FILE: target/arm/translate-neon.inc.c:890:
+#define DO_3SAME_32_ENV(INSN, FUNC)                                     \
+    WRAP_ENV_FN(gen_##INSN##_tramp8, gen_helper_neon_##FUNC##8);        \
+    WRAP_ENV_FN(gen_##INSN##_tramp16, gen_helper_neon_##FUNC##16);      \
+    WRAP_ENV_FN(gen_##INSN##_tramp32, gen_helper_neon_##FUNC##32);      \
+    static void gen_##INSN##_3s(unsigned vece, uint32_t rd_ofs,         \
+                                uint32_t rn_ofs, uint32_t rm_ofs,       \
+                                uint32_t oprsz, uint32_t maxsz)         \
+    {                                                                   \
+        static const GVecGen3 ops[4] = {                                \
+            { .fni4 = gen_##INSN##_tramp8 },                            \
+            { .fni4 = gen_##INSN##_tramp16 },                           \
+            { .fni4 = gen_##INSN##_tramp32 },                           \
+            { 0 },                                                      \
+        };                                                              \
+        tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, oprsz, maxsz, &ops[vece]); \
+    }                                                                   \
+    static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a)        \
+    {                                                                   \
+        if (a->size > 2) {                                              \
+            return false;                                               \
+        }                                                               \
+        return do_3same(s, a, gen_##INSN##_3s);                         \
+    }

ERROR: spaces required around that '*' (ctx:WxV)
#119: FILE: target/arm/translate-neon.inc.c:906:
+    static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a)        \
                                                              ^

total: 2 errors, 0 warnings, 154 lines checked

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

8/17 Checking commit a4747ac6caf6 (target/arm: Convert Neon VPMAX/VPMIN 3-reg-same insns to decodetree)
ERROR: spaces required around that '*' (ctx:WxV)
#51: FILE: target/arm/translate-neon.inc.c:928:
+static bool do_3same_pair(DisasContext *s, arg_3same *a, NeonGenTwoOpFn *fn)
                                                      ^

ERROR: spaces required around that '*' (ctx:WxV)
#98: FILE: target/arm/translate-neon.inc.c:975:
+    static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a)        \
                                                              ^

total: 2 errors, 0 warnings, 136 lines checked

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

9/17 Checking commit 3015d59a7178 (target/arm: Convert Neon VPADD 3-reg-same insns to decodetree)
10/17 Checking commit b2fa781c9e60 (target/arm: Convert Neon VQDMULH/VQRDMULH 3-reg-same to decodetree)
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#38: FILE: target/arm/translate-neon.inc.c:1001:
+#define DO_3SAME_VQDMULH(INSN, FUNC)                                    \
+    WRAP_ENV_FN(gen_##INSN##_tramp16, gen_helper_neon_##FUNC##_s16);    \
+    WRAP_ENV_FN(gen_##INSN##_tramp32, gen_helper_neon_##FUNC##_s32);    \
+    static void gen_##INSN##_3s(unsigned vece, uint32_t rd_ofs,         \
+                                uint32_t rn_ofs, uint32_t rm_ofs,       \
+                                uint32_t oprsz, uint32_t maxsz)         \
+    {                                                                   \
+        static const GVecGen3 ops[2] = {                                \
+            { .fni4 = gen_##INSN##_tramp16 },                           \
+            { .fni4 = gen_##INSN##_tramp32 },                           \
+        };                                                              \
+        tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, oprsz, maxsz, &ops[vece - 1]); \
+    }                                                                   \
+    static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a)        \
+    {                                                                   \
+        if (a->size != 1 && a->size != 2) {                             \
+            return false;                                               \
+        }                                                               \
+        return do_3same(s, a, gen_##INSN##_3s);                         \
+    }

ERROR: spaces required around that '*' (ctx:WxV)
#51: FILE: target/arm/translate-neon.inc.c:1014:
+    static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a)        \
                                                              ^

total: 2 errors, 0 warnings, 72 lines checked

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

11/17 Checking commit 0cc39b799ad7 (target/arm: Convert Neon VADD, VSUB, VABD 3-reg-same insns to decodetree)
ERROR: space required after that ',' (ctx:VxV)
#90: FILE: target/arm/translate-neon.inc.c:1029:
+#define DO_3S_FP_GVEC(INSN,FUNC)                                        \
                           ^

ERROR: spaces required around that '*' (ctx:WxV)
#100: FILE: target/arm/translate-neon.inc.c:1039:
+    static bool trans_##INSN##_fp_3s(DisasContext *s, arg_3same *a)     \
                                                                 ^

WARNING: Block comments use a leading /* on a separate line
#103: FILE: target/arm/translate-neon.inc.c:1042:
+            /* TODO fp16 support */                                     \

total: 2 errors, 1 warnings, 120 lines checked

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

12/17 Checking commit 1c588e7110e6 (target/arm: Convert Neon VPMIN/VPMAX/VPADD float 3-reg-same insns to decodetree)
ERROR: spaces required around that '*' (ctx:WxV)
#47: FILE: target/arm/translate-neon.inc.c:1053:
+static bool do_3same_fp_pair(DisasContext *s, arg_3same *a, VFPGen3OpSPFn *fn)
                                                         ^

ERROR: space required after that ',' (ctx:VxV)
#96: FILE: target/arm/translate-neon.inc.c:1102:
+#define DO_3S_FP_PAIR(INSN,FUNC)                                    \
                           ^

ERROR: spaces required around that '*' (ctx:WxV)
#97: FILE: target/arm/translate-neon.inc.c:1103:
+    static bool trans_##INSN##_fp_3s(DisasContext *s, arg_3same *a) \
                                                                 ^

WARNING: Block comments use a leading /* on a separate line
#100: FILE: target/arm/translate-neon.inc.c:1106:
+            /* TODO fp16 support */                                 \

ERROR: suspect code indent for conditional statements (8, 8)
#158: FILE: target/arm/translate.c:5476:
         for (pass = 0; pass < (q ? 4 : 2); pass++) {
[...]
+        /* Elementwise.  */

total: 4 errors, 1 warnings, 181 lines checked

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

13/17 Checking commit 3613a840e48e (target/arm: Convert Neon fp VMUL, VMLA, VMLS 3-reg-same insns to decodetree)
ERROR: spaces required around that '*' (ctx:WxV)
#55: FILE: target/arm/translate-neon.inc.c:1025:
+static bool do_3same_fp(DisasContext *s, arg_3same *a, VFPGen3OpSPFn *fn,
                                                    ^

ERROR: space required after that ',' (ctx:VxV)
#117: FILE: target/arm/translate-neon.inc.c:1107:
+#define DO_3S_FP(INSN,FUNC,READS_VD)                                \
                      ^

ERROR: space required after that ',' (ctx:VxV)
#117: FILE: target/arm/translate-neon.inc.c:1107:
+#define DO_3S_FP(INSN,FUNC,READS_VD)                                \
                           ^

ERROR: spaces required around that '*' (ctx:WxV)
#118: FILE: target/arm/translate-neon.inc.c:1108:
+    static bool trans_##INSN##_fp_3s(DisasContext *s, arg_3same *a) \
                                                                 ^

WARNING: Block comments use a leading /* on a separate line
#121: FILE: target/arm/translate-neon.inc.c:1111:
+            /* TODO fp16 support */                                 \

total: 4 errors, 1 warnings, 130 lines checked

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

14/17 Checking commit 87ce7b2990a3 (target/arm: Convert Neon 3-reg-same compare insns to decodetree)
15/17 Checking commit d72bf92f9fba (target/arm: Move 'env' argument of recps_f32 and rsqrts_f32 helpers to usual place)
16/17 Checking commit 5b54865d4a45 (target/arm: Convert Neon fp VMAX/VMIN/VMAXNM/VMINNM/VRECPS/VRSQRTS to decodetree)
ERROR: spaces required around that '*' (ctx:WxV)
#48: FILE: target/arm/translate-neon.inc.c:1142:
+static bool trans_VMAXNM_fp_3s(DisasContext *s, arg_3same *a)
                                                           ^

total: 1 errors, 0 warnings, 153 lines checked

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

17/17 Checking commit baa5469d3e6d (target/arm: Convert NEON VFMA, VFMS 3-reg-same insns to decodetree)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200512163904.10918-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