mbox series

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

Message ID 20200512163904.10918-1-peter.maydell@linaro.org
Headers show
Series target/arm: Convert rest of Neon 3-reg-same to decodetree | expand

Message

Peter Maydell May 12, 2020, 4:38 p.m. UTC 
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

Comments

no-reply@patchew.org May 12, 2020, 11:44 p.m. UTC | #1 
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