| Message ID | 20210204014509.882821-1-richard.henderson@linaro.org |
|---|---|
| Headers | show |
| Series | TCI fixes and cleanups | expand |
Patchew URL: https://patchew.org/QEMU/20210204014509.882821-1-richard.henderson@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210204014509.882821-1-richard.henderson@linaro.org
Subject: [PATCH v2 00/93] TCI fixes and cleanups
=== 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
- [tag update] patchew/20210127232151.3523581-1-f4bug@amsat.org -> patchew/20210127232151.3523581-1-f4bug@amsat.org
- [tag update] patchew/20210128144125.3696119-1-f4bug@amsat.org -> patchew/20210128144125.3696119-1-f4bug@amsat.org
* [new tag] patchew/20210204014509.882821-1-richard.henderson@linaro.org -> patchew/20210204014509.882821-1-richard.henderson@linaro.org
Switched to a new branch 'test'
8b0bc01 tcg/tci: Implement add2, sub2
da10429 tcg/tci: Implement mulu2, muls2
ced4f5a tcg/tci: Implement clz, ctz, ctpop
1e4852c tcg/tci: Implement extract, sextract
f3f91cc tcg/tci: Implement andc, orc, eqv, nand, nor
87a5d9e tcg/tci: Implement movcond
9976361 tcg/tci: Implement goto_ptr
11e005d tcg/tci: Change encoding to uint32_t units
d42a563 tcg/tci: Remove tci_write_reg
231b705 tcg/tci: Emit setcond before brcond
055b38e tcg/tci: Reserve r13 for a temporary
081aea4 tcg/tci: Split out tcg_out_op_r[iI]
fff070a tcg/tci: Split out tcg_out_op_np
015559a tcg/tci: Split out tcg_out_op_v
89bee6c tcg/tci: Split out tcg_out_op_{rrm,rrrm,rrrrm}
2311b1a tcg/tci: Split out tcg_out_op_rrrrcl
b226664 tcg/tci: Split out tcg_out_op_rrrr
08b2642 tcg/tci: Split out tcg_out_op_rrrrrr
36b029b tcg/tci: Split out tcg_out_op_rrcl
a78060f tcg/tci: Split out tcg_out_op_rrrbb
80891b3 tcg/tci: Split out tcg_out_op_rrrrrc
030b2c5 tcg/tci: Split out tcg_out_op_rrrc
152d803 tcg/tci: Split out tcg_out_op_rrr
219243e tcg/tci: Split out tcg_out_op_rr
bc62fe5 tcg/tci: Split out tcg_out_op_p
ae4c05c tcg/tci: Split out tcg_out_op_l
420d1be tcg/tci: Split out tcg_out_op_rrs
d67ed68 tcg/tci: Push opcode emit into each case
5e85088 tcg/tci: Move call-return regs to end of tcg_target_reg_alloc_order
8ef0a82 tcg/tci: Improve tcg_target_call_clobber_regs
6b7a3c7 tcg/tci: Use ffi for calls
3603af3 tcg: Build ffi data structures for helpers
a3ee3dd tcg/tci: Implement the disassembler properly
938c48e tcg/tci: Remove tci_disas
102a4b7 tcg/tci: Hoist op_size checking into tci_args_*
5f4a91e tcg/tci: Split out tci_args_{rrm,rrrm,rrrrm}
4ab25b1 tcg/tci: Reduce qemu_ld/st TCGMemOpIdx operand to 32-bits
5112952 tcg/tci: Clean up deposit operations
25e378a tcg/tci: Split out tci_args_rrrr
98e9b3f tcg/tci: Split out tci_args_rrrrrr
0156bf9 tcg/tci: Reuse tci_args_l for goto_tb
0f4b492 tcg/tci: Reuse tci_args_l for exit_tb
0697d2d tcg/tci: Reuse tci_args_l for calls.
d351c13 tcg/tci: Split out tci_args_ri and tci_args_rI
699ba0f tcg/tci: Split out tci_args_rrcl and tci_args_rrrrcl
8886438 tcg/tci: Split out tci_args_rrrrrc
40f64f6 tcg/tci: Split out tci_args_l
b14402b tcg/tci: Split out tci_args_rrrc
02a8039 tcg/tci: Split out tci_args_rrr
a91c032 tcg/tci: Split out tci_args_rr
cd078fe tcg/tci: Split out tci_args_rrs
943fd28 tcg/tci: Rename tci_read_r to tci_read_rval
0d2df6a tcg/tci: Merge mov, not and neg operations
c8f29d9 tcg/tci: Merge bswap operations
93e1dd4 tcg/tci: Remove ifdefs for TCG_TARGET_HAS_ext32[us]_i64
4ba996d tcg/tci: Merge extension operations
8e5b80c tcg/tci: Merge basic arithmetic operations
21a34d1 tcg/tci: Reduce use of tci_read_r64
e98b67d tcg/tci: Remove tci_read_r32s
0b2209a tcg/tci: Remove tci_read_r16s
23f5ca8 tcg/tci: Remove tci_read_r16
b1e734e tcg/tci: Remove tci_read_r8s
4d485d6 tcg/tci: Remove tci_read_r8
8ddeee2 tcg/tci: Merge identical cases in generation
01b68d4 tcg/tci: Remove TCG_CONST
3f384f5 tcg/tci: Use bool in tcg_out_ri*
c2365d0 tcg/tci: Fix TCG_REG_R4 misusage
5b68471 tcg/tci: Restrict TCG_TARGET_NB_REGS to 16
ad391cb tcg/tci: Remove TODO as unused
1a32255 tcg/tci: Implement 64-bit division
9b566c3 tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_*
1e76758 tcg/tci: Use g_assert_not_reached
927c77f tcg/tci: Merge INDEX_op_{st_i32,st32_i64}
c794a09 tcg/tci: Move stack bounds check to compile-time
393a624 tcg/tci: Merge INDEX_op_st16_{i32,i64}
7ef8c09 tcg/tci: Merge INDEX_op_st8_{i32,i64}
10e9b92 tcg/tci: Merge INDEX_op_{ld_i32,ld32u_i64}
116f33d tcg/tci: Merge INDEX_op_ld16s_{i32,i64}
5f7ede9 tcg/tci: Merge INDEX_op_ld16u_{i32,i64}
8dcd478 tcg/tci: Merge INDEX_op_ld8s_{i32,i64}
01ef162 tcg/tci: Merge INDEX_op_ld8u_{i32,i64}
fccf4b6 tcg/tci: Inline tci_write_reg64 into 64-bit callers
c245070 tcg/tci: Inline tci_write_reg32 into all callers
84dcf2a tcg/tci: Inline tci_write_reg16 into the only caller
df64108 tcg/tci: Inline tci_write_reg8 into its callers
bde4a52 tcg/tci: Inline tci_write_reg32s into the only caller
149907b tcg/tci: Implement INDEX_op_ld8s_i64
dc1b33e tcg/tci: Implement INDEX_op_ld16s_i32
fc60c35 tcg/tci: Make tci_tb_ptr thread-local
0460433 tcg: Manage splitwx in tc_ptr_to_region_tree by hand
dcc1427 configure: Fix --enable-tcg-interpreter
88693f5 tcg: Split out tcg_raise_tb_overflow
58f1c9d gdbstub: Fix handle_query_xfer_auxv
=== OUTPUT BEGIN ===
1/93 Checking commit 58f1c9dd12ed (gdbstub: Fix handle_query_xfer_auxv)
2/93 Checking commit 88693f5b4098 (tcg: Split out tcg_raise_tb_overflow)
3/93 Checking commit dcc1427420fa (configure: Fix --enable-tcg-interpreter)
4/93 Checking commit 0460433f0b4d (tcg: Manage splitwx in tc_ptr_to_region_tree by hand)
5/93 Checking commit fc60c3528109 (tcg/tci: Make tci_tb_ptr thread-local)
6/93 Checking commit dc1b33ee7571 (tcg/tci: Implement INDEX_op_ld16s_i32)
7/93 Checking commit 149907ba9185 (tcg/tci: Implement INDEX_op_ld8s_i64)
8/93 Checking commit bde4a52322e3 (tcg/tci: Inline tci_write_reg32s into the only caller)
9/93 Checking commit df641084c79a (tcg/tci: Inline tci_write_reg8 into its callers)
10/93 Checking commit 84dcf2a64782 (tcg/tci: Inline tci_write_reg16 into the only caller)
11/93 Checking commit c24507003f2f (tcg/tci: Inline tci_write_reg32 into all callers)
12/93 Checking commit fccf4b68bfaa (tcg/tci: Inline tci_write_reg64 into 64-bit callers)
13/93 Checking commit 01ef162b41d5 (tcg/tci: Merge INDEX_op_ld8u_{i32,i64})
14/93 Checking commit 8dcd478e50c8 (tcg/tci: Merge INDEX_op_ld8s_{i32,i64})
15/93 Checking commit 5f7ede9dea7b (tcg/tci: Merge INDEX_op_ld16u_{i32,i64})
16/93 Checking commit 116f33d50d6b (tcg/tci: Merge INDEX_op_ld16s_{i32,i64})
17/93 Checking commit 10e9b929d2a4 (tcg/tci: Merge INDEX_op_{ld_i32,ld32u_i64})
18/93 Checking commit 7ef8c0931d3c (tcg/tci: Merge INDEX_op_st8_{i32,i64})
19/93 Checking commit 393a62462008 (tcg/tci: Merge INDEX_op_st16_{i32,i64})
20/93 Checking commit c794a09db8fc (tcg/tci: Move stack bounds check to compile-time)
21/93 Checking commit 927c77f44727 (tcg/tci: Merge INDEX_op_{st_i32,st32_i64})
22/93 Checking commit 1e7675830e78 (tcg/tci: Use g_assert_not_reached)
23/93 Checking commit 9b566c3c1ea4 (tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_*)
24/93 Checking commit 1a3225514775 (tcg/tci: Implement 64-bit division)
25/93 Checking commit ad391cbfd2d9 (tcg/tci: Remove TODO as unused)
26/93 Checking commit 5b684717e738 (tcg/tci: Restrict TCG_TARGET_NB_REGS to 16)
27/93 Checking commit c2365d08ae35 (tcg/tci: Fix TCG_REG_R4 misusage)
28/93 Checking commit 3f384f57facc (tcg/tci: Use bool in tcg_out_ri*)
29/93 Checking commit 01b68d4c06b1 (tcg/tci: Remove TCG_CONST)
30/93 Checking commit 8ddeee25f950 (tcg/tci: Merge identical cases in generation)
31/93 Checking commit 4d485d6d060a (tcg/tci: Remove tci_read_r8)
32/93 Checking commit b1e734ec57ec (tcg/tci: Remove tci_read_r8s)
33/93 Checking commit 23f5ca83d14c (tcg/tci: Remove tci_read_r16)
34/93 Checking commit 0b2209a73835 (tcg/tci: Remove tci_read_r16s)
35/93 Checking commit e98b67d04791 (tcg/tci: Remove tci_read_r32s)
36/93 Checking commit 21a34d1a9dee (tcg/tci: Reduce use of tci_read_r64)
37/93 Checking commit 8e5b80cd5c63 (tcg/tci: Merge basic arithmetic operations)
38/93 Checking commit 4ba996ddc4a1 (tcg/tci: Merge extension operations)
39/93 Checking commit 93e1dd4b2d1b (tcg/tci: Remove ifdefs for TCG_TARGET_HAS_ext32[us]_i64)
40/93 Checking commit c8f29d9affb2 (tcg/tci: Merge bswap operations)
41/93 Checking commit 0d2df6ae1c07 (tcg/tci: Merge mov, not and neg operations)
42/93 Checking commit 943fd281285e (tcg/tci: Rename tci_read_r to tci_read_rval)
43/93 Checking commit cd078fe19af1 (tcg/tci: Split out tci_args_rrs)
44/93 Checking commit a91c032f91a9 (tcg/tci: Split out tci_args_rr)
45/93 Checking commit 02a8039045b4 (tcg/tci: Split out tci_args_rrr)
46/93 Checking commit b14402b7c4df (tcg/tci: Split out tci_args_rrrc)
47/93 Checking commit 40f64f684fc4 (tcg/tci: Split out tci_args_l)
48/93 Checking commit 8886438314e6 (tcg/tci: Split out tci_args_rrrrrc)
49/93 Checking commit 699ba0fe82e8 (tcg/tci: Split out tci_args_rrcl and tci_args_rrrrcl)
50/93 Checking commit d351c13d441e (tcg/tci: Split out tci_args_ri and tci_args_rI)
51/93 Checking commit 0697d2ddada4 (tcg/tci: Reuse tci_args_l for calls.)
52/93 Checking commit 0f4b492e3c71 (tcg/tci: Reuse tci_args_l for exit_tb)
53/93 Checking commit 0156bf9d5970 (tcg/tci: Reuse tci_args_l for goto_tb)
54/93 Checking commit 98e9b3fa9d40 (tcg/tci: Split out tci_args_rrrrrr)
55/93 Checking commit 25e378a6a247 (tcg/tci: Split out tci_args_rrrr)
56/93 Checking commit 5112952475e2 (tcg/tci: Clean up deposit operations)
57/93 Checking commit 4ab25b103753 (tcg/tci: Reduce qemu_ld/st TCGMemOpIdx operand to 32-bits)
58/93 Checking commit 5f4a91e3ea51 (tcg/tci: Split out tci_args_{rrm,rrrm,rrrrm})
59/93 Checking commit 102a4b755857 (tcg/tci: Hoist op_size checking into tci_args_*)
60/93 Checking commit 938c48e4249b (tcg/tci: Remove tci_disas)
61/93 Checking commit a3ee3dd3a1b9 (tcg/tci: Implement the disassembler properly)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22:
deleted file mode 100644
total: 0 errors, 1 warnings, 304 lines checked
Patch 61/93 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
62/93 Checking commit 3603af302b19 (tcg: Build ffi data structures for helpers)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26:
new file mode 100644
ERROR: Macros with complex values should be enclosed in parenthesis
#41: FILE: include/exec/helper-ffi.h:11:
+#define dh_ffitype_i32 &ffi_type_uint32
ERROR: Macros with complex values should be enclosed in parenthesis
#42: FILE: include/exec/helper-ffi.h:12:
+#define dh_ffitype_s32 &ffi_type_sint32
ERROR: Macros with complex values should be enclosed in parenthesis
#44: FILE: include/exec/helper-ffi.h:14:
+#define dh_ffitype_i64 &ffi_type_uint64
ERROR: Macros with complex values should be enclosed in parenthesis
#45: FILE: include/exec/helper-ffi.h:15:
+#define dh_ffitype_s64 &ffi_type_sint64
ERROR: Macros with complex values should be enclosed in parenthesis
#46: FILE: include/exec/helper-ffi.h:16:
+#define dh_ffitype_f16 &ffi_type_uint32
ERROR: Macros with complex values should be enclosed in parenthesis
#47: FILE: include/exec/helper-ffi.h:17:
+#define dh_ffitype_f32 &ffi_type_uint32
ERROR: Macros with complex values should be enclosed in parenthesis
#48: FILE: include/exec/helper-ffi.h:18:
+#define dh_ffitype_f64 &ffi_type_uint64
ERROR: Macros with complex values should be enclosed in parenthesis
#51: FILE: include/exec/helper-ffi.h:21:
+# define dh_ffitype_tl &ffi_type_uint32
ERROR: Macros with complex values should be enclosed in parenthesis
#53: FILE: include/exec/helper-ffi.h:23:
+# define dh_ffitype_tl &ffi_type_uint64
ERROR: Macros with complex values should be enclosed in parenthesis
#56: FILE: include/exec/helper-ffi.h:26:
+#define dh_ffitype_ptr &ffi_type_pointer
ERROR: Macros with complex values should be enclosed in parenthesis
#57: FILE: include/exec/helper-ffi.h:27:
+#define dh_ffitype_cptr &ffi_type_pointer
ERROR: Macros with complex values should be enclosed in parenthesis
#60: FILE: include/exec/helper-ffi.h:30:
+#define dh_ffitype_env &ffi_type_pointer
ERROR: space required after that ',' (ctx:VxV)
#64: FILE: include/exec/helper-ffi.h:34:
+ static ffi_cif glue(cif_,NAME) = { \
^
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#68: FILE: include/exec/helper-ffi.h:38:
+#define DEF_HELPER_FLAGS_1(NAME, FLAGS, ret, t1) \
+ static ffi_type *glue(cif_args_,NAME)[1] = { dh_ffitype(t1) }; \
+ static ffi_cif glue(cif_,NAME) = { \
+ .rtype = dh_ffitype(ret), .nargs = 1, \
+ .arg_types = glue(cif_args_,NAME), \
+ };
ERROR: spaces required around that '*' (ctx:WxV)
#69: FILE: include/exec/helper-ffi.h:39:
+ static ffi_type *glue(cif_args_,NAME)[1] = { dh_ffitype(t1) }; \
^
ERROR: space required after that ',' (ctx:VxV)
#69: FILE: include/exec/helper-ffi.h:39:
+ static ffi_type *glue(cif_args_,NAME)[1] = { dh_ffitype(t1) }; \
^
ERROR: open brace '{' following function declarations go on the next line
#70: FILE: include/exec/helper-ffi.h:40:
+ static ffi_cif glue(cif_,NAME) = { \
ERROR: space required after that ',' (ctx:VxV)
#70: FILE: include/exec/helper-ffi.h:40:
+ static ffi_cif glue(cif_,NAME) = { \
^
ERROR: space required after that ',' (ctx:VxV)
#72: FILE: include/exec/helper-ffi.h:42:
+ .arg_types = glue(cif_args_,NAME), \
^
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#75: FILE: include/exec/helper-ffi.h:45:
+#define DEF_HELPER_FLAGS_2(NAME, FLAGS, ret, t1, t2) \
+ static ffi_type *glue(cif_args_,NAME)[2] = { \
+ dh_ffitype(t1), dh_ffitype(t2) \
+ }; \
+ static ffi_cif glue(cif_,NAME) = { \
+ .rtype = dh_ffitype(ret), .nargs = 2, \
+ .arg_types = glue(cif_args_,NAME), \
+ };
ERROR: spaces required around that '*' (ctx:WxV)
#76: FILE: include/exec/helper-ffi.h:46:
+ static ffi_type *glue(cif_args_,NAME)[2] = { \
^
ERROR: space required after that ',' (ctx:VxV)
#76: FILE: include/exec/helper-ffi.h:46:
+ static ffi_type *glue(cif_args_,NAME)[2] = { \
^
ERROR: open brace '{' following function declarations go on the next line
#79: FILE: include/exec/helper-ffi.h:49:
+ static ffi_cif glue(cif_,NAME) = { \
ERROR: space required after that ',' (ctx:VxV)
#79: FILE: include/exec/helper-ffi.h:49:
+ static ffi_cif glue(cif_,NAME) = { \
^
ERROR: space required after that ',' (ctx:VxV)
#81: FILE: include/exec/helper-ffi.h:51:
+ .arg_types = glue(cif_args_,NAME), \
^
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#84: FILE: include/exec/helper-ffi.h:54:
+#define DEF_HELPER_FLAGS_3(NAME, FLAGS, ret, t1, t2, t3) \
+ static ffi_type *glue(cif_args_,NAME)[3] = { \
+ dh_ffitype(t1), dh_ffitype(t2), dh_ffitype(t3) \
+ }; \
+ static ffi_cif glue(cif_,NAME) = { \
+ .rtype = dh_ffitype(ret), .nargs = 3, \
+ .arg_types = glue(cif_args_,NAME), \
+ };
ERROR: spaces required around that '*' (ctx:WxV)
#85: FILE: include/exec/helper-ffi.h:55:
+ static ffi_type *glue(cif_args_,NAME)[3] = { \
^
ERROR: space required after that ',' (ctx:VxV)
#85: FILE: include/exec/helper-ffi.h:55:
+ static ffi_type *glue(cif_args_,NAME)[3] = { \
^
ERROR: open brace '{' following function declarations go on the next line
#88: FILE: include/exec/helper-ffi.h:58:
+ static ffi_cif glue(cif_,NAME) = { \
ERROR: space required after that ',' (ctx:VxV)
#88: FILE: include/exec/helper-ffi.h:58:
+ static ffi_cif glue(cif_,NAME) = { \
^
ERROR: space required after that ',' (ctx:VxV)
#90: FILE: include/exec/helper-ffi.h:60:
+ .arg_types = glue(cif_args_,NAME), \
^
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#93: FILE: include/exec/helper-ffi.h:63:
+#define DEF_HELPER_FLAGS_4(NAME, FLAGS, ret, t1, t2, t3, t4) \
+ static ffi_type *glue(cif_args_,NAME)[4] = { \
+ dh_ffitype(t1), dh_ffitype(t2), dh_ffitype(t3), dh_ffitype(t4) \
+ }; \
+ static ffi_cif glue(cif_,NAME) = { \
+ .rtype = dh_ffitype(ret), .nargs = 4, \
+ .arg_types = glue(cif_args_,NAME), \
+ };
ERROR: spaces required around that '*' (ctx:WxV)
#94: FILE: include/exec/helper-ffi.h:64:
+ static ffi_type *glue(cif_args_,NAME)[4] = { \
^
ERROR: space required after that ',' (ctx:VxV)
#94: FILE: include/exec/helper-ffi.h:64:
+ static ffi_type *glue(cif_args_,NAME)[4] = { \
^
ERROR: open brace '{' following function declarations go on the next line
#97: FILE: include/exec/helper-ffi.h:67:
+ static ffi_cif glue(cif_,NAME) = { \
ERROR: space required after that ',' (ctx:VxV)
#97: FILE: include/exec/helper-ffi.h:67:
+ static ffi_cif glue(cif_,NAME) = { \
^
ERROR: space required after that ',' (ctx:VxV)
#99: FILE: include/exec/helper-ffi.h:69:
+ .arg_types = glue(cif_args_,NAME), \
^
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#102: FILE: include/exec/helper-ffi.h:72:
+#define DEF_HELPER_FLAGS_5(NAME, FLAGS, ret, t1, t2, t3, t4, t5) \
+ static ffi_type *glue(cif_args_,NAME)[5] = { \
+ dh_ffitype(t1), dh_ffitype(t2), dh_ffitype(t3), \
+ dh_ffitype(t4), dh_ffitype(t5) \
+ }; \
+ static ffi_cif glue(cif_,NAME) = { \
+ .rtype = dh_ffitype(ret), .nargs = 5, \
+ .arg_types = glue(cif_args_,NAME), \
+ };
ERROR: spaces required around that '*' (ctx:WxV)
#103: FILE: include/exec/helper-ffi.h:73:
+ static ffi_type *glue(cif_args_,NAME)[5] = { \
^
ERROR: space required after that ',' (ctx:VxV)
#103: FILE: include/exec/helper-ffi.h:73:
+ static ffi_type *glue(cif_args_,NAME)[5] = { \
^
ERROR: open brace '{' following function declarations go on the next line
#107: FILE: include/exec/helper-ffi.h:77:
+ static ffi_cif glue(cif_,NAME) = { \
ERROR: space required after that ',' (ctx:VxV)
#107: FILE: include/exec/helper-ffi.h:77:
+ static ffi_cif glue(cif_,NAME) = { \
^
ERROR: space required after that ',' (ctx:VxV)
#109: FILE: include/exec/helper-ffi.h:79:
+ .arg_types = glue(cif_args_,NAME), \
^
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#112: FILE: include/exec/helper-ffi.h:82:
+#define DEF_HELPER_FLAGS_6(NAME, FLAGS, ret, t1, t2, t3, t4, t5, t6) \
+ static ffi_type *glue(cif_args_,NAME)[6] = { \
+ dh_ffitype(t1), dh_ffitype(t2), dh_ffitype(t3), \
+ dh_ffitype(t4), dh_ffitype(t5), dh_ffitype(t6) \
+ }; \
+ static ffi_cif glue(cif_,NAME) = { \
+ .rtype = dh_ffitype(ret), .nargs = 6, \
+ .arg_types = glue(cif_args_,NAME), \
+ };
ERROR: spaces required around that '*' (ctx:WxV)
#113: FILE: include/exec/helper-ffi.h:83:
+ static ffi_type *glue(cif_args_,NAME)[6] = { \
^
ERROR: space required after that ',' (ctx:VxV)
#113: FILE: include/exec/helper-ffi.h:83:
+ static ffi_type *glue(cif_args_,NAME)[6] = { \
^
ERROR: open brace '{' following function declarations go on the next line
#117: FILE: include/exec/helper-ffi.h:87:
+ static ffi_cif glue(cif_,NAME) = { \
ERROR: space required after that ',' (ctx:VxV)
#117: FILE: include/exec/helper-ffi.h:87:
+ static ffi_cif glue(cif_,NAME) = { \
^
ERROR: space required after that ',' (ctx:VxV)
#119: FILE: include/exec/helper-ffi.h:89:
+ .arg_types = glue(cif_args_,NAME), \
^
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#122: FILE: include/exec/helper-ffi.h:92:
+#define DEF_HELPER_FLAGS_7(NAME, FLAGS, ret, t1, t2, t3, t4, t5, t6, t7) \
+ static ffi_type *glue(cif_args_,NAME)[7] = { \
+ dh_ffitype(t1), dh_ffitype(t2), dh_ffitype(t3), \
+ dh_ffitype(t4), dh_ffitype(t5), dh_ffitype(t6), dh_ffitype(t7) \
+ }; \
+ static ffi_cif glue(cif_,NAME) = { \
+ .rtype = dh_ffitype(ret), .nargs = 7, \
+ .arg_types = glue(cif_args_,NAME), \
+ };
ERROR: spaces required around that '*' (ctx:WxV)
#123: FILE: include/exec/helper-ffi.h:93:
+ static ffi_type *glue(cif_args_,NAME)[7] = { \
^
ERROR: space required after that ',' (ctx:VxV)
#123: FILE: include/exec/helper-ffi.h:93:
+ static ffi_type *glue(cif_args_,NAME)[7] = { \
^
ERROR: open brace '{' following function declarations go on the next line
#127: FILE: include/exec/helper-ffi.h:97:
+ static ffi_cif glue(cif_,NAME) = { \
ERROR: space required after that ',' (ctx:VxV)
#127: FILE: include/exec/helper-ffi.h:97:
+ static ffi_cif glue(cif_,NAME) = { \
^
ERROR: space required after that ',' (ctx:VxV)
#129: FILE: include/exec/helper-ffi.h:99:
+ .arg_types = glue(cif_args_,NAME), \
^
total: 55 errors, 1 warnings, 309 lines checked
Patch 62/93 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
63/93 Checking commit 6b7a3c7affe7 (tcg/tci: Use ffi for calls)
ERROR: spaces required around that '+' (ctx:VxV)
#94: FILE: tcg/tcg.c:2085:
+ bool is_64bit = sizemask & (1 << (i+1)*2);
^
ERROR: spaces required around that '*' (ctx:VxV)
#94: FILE: tcg/tcg.c:2085:
+ bool is_64bit = sizemask & (1 << (i+1)*2);
^
ERROR: suspect code indent for conditional statements (8, 11)
#123: FILE: tcg/tcg.c:2104:
+ if (TCG_TARGET_REG_BITS < 64 && is_64bit) {
+ /*
total: 3 errors, 0 warnings, 380 lines checked
Patch 63/93 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
64/93 Checking commit 8ef0a82466ec (tcg/tci: Improve tcg_target_call_clobber_regs)
65/93 Checking commit 5e850884e122 (tcg/tci: Move call-return regs to end of tcg_target_reg_alloc_order)
66/93 Checking commit d67ed6866754 (tcg/tci: Push opcode emit into each case)
67/93 Checking commit 420d1be5fd4e (tcg/tci: Split out tcg_out_op_rrs)
68/93 Checking commit ae4c05c03bdb (tcg/tci: Split out tcg_out_op_l)
69/93 Checking commit bc62fe55babd (tcg/tci: Split out tcg_out_op_p)
70/93 Checking commit 219243ed3ba9 (tcg/tci: Split out tcg_out_op_rr)
71/93 Checking commit 152d803979b5 (tcg/tci: Split out tcg_out_op_rrr)
72/93 Checking commit 030b2c50f949 (tcg/tci: Split out tcg_out_op_rrrc)
73/93 Checking commit 80891b3fba98 (tcg/tci: Split out tcg_out_op_rrrrrc)
74/93 Checking commit a78060f3a057 (tcg/tci: Split out tcg_out_op_rrrbb)
75/93 Checking commit 36b029b4973f (tcg/tci: Split out tcg_out_op_rrcl)
76/93 Checking commit 08b264232c3a (tcg/tci: Split out tcg_out_op_rrrrrr)
77/93 Checking commit b226664af838 (tcg/tci: Split out tcg_out_op_rrrr)
78/93 Checking commit 2311b1a52c71 (tcg/tci: Split out tcg_out_op_rrrrcl)
79/93 Checking commit 89bee6cf27eb (tcg/tci: Split out tcg_out_op_{rrm,rrrm,rrrrm})
80/93 Checking commit 015559a6d7b3 (tcg/tci: Split out tcg_out_op_v)
81/93 Checking commit fff070acc967 (tcg/tci: Split out tcg_out_op_np)
82/93 Checking commit 081aea4ecef3 (tcg/tci: Split out tcg_out_op_r[iI])
83/93 Checking commit 055b38e38da2 (tcg/tci: Reserve r13 for a temporary)
84/93 Checking commit 231b705d5b32 (tcg/tci: Emit setcond before brcond)
85/93 Checking commit d42a563c8817 (tcg/tci: Remove tci_write_reg)
86/93 Checking commit 11e005d652ef (tcg/tci: Change encoding to uint32_t units)
87/93 Checking commit 9976361ad295 (tcg/tci: Implement goto_ptr)
88/93 Checking commit 87a5d9e861e9 (tcg/tci: Implement movcond)
89/93 Checking commit f3f91cc7ff2a (tcg/tci: Implement andc, orc, eqv, nand, nor)
90/93 Checking commit 1e4852c279f0 (tcg/tci: Implement extract, sextract)
91/93 Checking commit ced4f5ae7859 (tcg/tci: Implement clz, ctz, ctpop)
92/93 Checking commit da10429079fe (tcg/tci: Implement mulu2, muls2)
93/93 Checking commit 8b0bc016ff68 (tcg/tci: Implement add2, sub2)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210204014509.882821-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On Thu, 4 Feb 2021 at 01:49, Richard Henderson <richard.henderson@linaro.org> wrote: > > Almost 7 years ago I detailed 5 major problems in tci[1], of > which three still remain: > > * Unaligned accesses to the bytecode stream, which means > that we immediately SIGBUS on any host requiring alignment. > * Non-portable calls to helper functions. > * Full of useless ifdefs and TODOs. > > To my mind, this means the code is unmaintained, despite what it > says in MAINTAINERS. Thus tci *should* be simply removed. > However, every time removal is suggested, someone comes out of the > woodwork and says we should keep it, because it's useful for $FOO. Not listed, but also a problem: * it's a configure-time choice, not a runtime choice (Personally I'm on the "we should just remove it" side.) thanks -- PMM
Am 04.02.21 um 10:58 schrieb Peter Maydell: > On Thu, 4 Feb 2021 at 01:49, Richard Henderson > <richard.henderson@linaro.org> wrote: >> Almost 7 years ago I detailed 5 major problems in tci[1], of >> which three still remain: >> >> * Unaligned accesses to the bytecode stream, which means >> that we immediately SIGBUS on any host requiring alignment. >> * Non-portable calls to helper functions. >> * Full of useless ifdefs and TODOs. >> >> To my mind, this means the code is unmaintained, despite what it >> says in MAINTAINERS. Thus tci *should* be simply removed. >> However, every time removal is suggested, someone comes out of the >> woodwork and says we should keep it, because it's useful for $FOO. > Not listed, but also a problem: > * it's a configure-time choice, not a runtime choice That's the feature which I also desire most. Technically it was not possible to have native and interpreted TCG in the same code some years ago when I tried to implement this, but that might have changed as TCG has evolved a lot. Having TCI as a runtime choice would not only require less CI builds but also allow new use cases for TCG testing. I also agree with most of Richard's list of problems and appreciate the efforts to fix them. I disagree on the #ifdefs which can help to understand TCG better in my opinion, so for me they have a useful side effect of being also documentation. Most of the problems which you named above were already mentioned in the README for TCI from 2011. Nevertheless I had the impression that TCI was "good enough" for those who used it, and it was sufficient to fix a newly used TCG opcode which triggered a TODO assertion from time to time. Obviously I underestimated Richard's desire to have a 100 % working TCI. I am sorry for that. Technically the patch series looks reasonable. I like especially the disassembler. Is there a Git repository which makes pulling all changes easier? It would also help if the patches which were already reviewed were already merged in qemu master. Regarding misaligned bytecode access, there exist two solutions. We could either use code which handles that correctly (I had sent a patch using memcpy two years ago and recently sent a V2 which uses the QEMU standard functions for that). Or we can align the data like it is done in Richard's patches. For me it is not obvious which one is better. While a single access is faster for aligned data, this might be different for sequential access on misaligned data which might profit from better caching of smaller bytecode. Kind regards, Stefan
On 2/4/21 10:02 AM, Stefan Weil wrote: > Is there a Git repository which makes pulling all changes easier? https://gitlab.com/rth7680/qemu/-/tree/tci-next > Regarding misaligned bytecode access, there exist two solutions. We could > either use code which handles that correctly (I had sent a patch using memcpy > two years ago and recently sent a V2 which uses the QEMU standard functions for > that). Or we can align the data like it is done in Richard's patches. For me it > is not obvious which one is better. I think it is obvious. If a host requires aligned access, a single aligned load requires only one instruction, and an unaligned access requires lots. E.g., for sparc, int foo(void *p) { int x; __builtin_memcpy(&x, p, 4); return x; } ldub [%i0], %g3 ldub [%i0+1], %g2 stb %g3, [%fp+2043] stb %g2, [%fp+2044] ldub [%i0+2], %g3 ldub [%i0+3], %g2 stb %g3, [%fp+2045] stb %g2, [%fp+2046] ldsw [%fp+2043], %i0 Such unaligned accesses are *really* slow. > While a single access is faster for aligned > data, this might be different for sequential access on misaligned data which > might profit from better caching of smaller bytecode. I believe you'll find that the rewrite encoding is smaller already. r~
On 2/4/21 10:02 AM, Stefan Weil wrote: > Am 04.02.21 um 10:58 schrieb Peter Maydell: >> Not listed, but also a problem: >> * it's a configure-time choice, not a runtime choice > > That's the feature which I also desire most. Well... that depends on how you see tci being most used. If, like John Glaubitz, you want to use tci for odd systems that don't have a native backend, then a configure-time choice is appropriate. If you're trying to use tci as a cross-check to the native tcg backend, then a command-line choice could be appropriate. With the bits done below, it could be just two extra files compiled, which I suppose is light-weight enough. If you're trying to use tci as a true interpreter for a single-use TB... then nothing about tci in its current form is appropriate. Of course, I also believe that our single-use TBs should not be single-use -- because of mttcg, we can no longer reuse the memory from them, and therefore we might as well just keep them around, properly hashed. > Technically it was not possible to > have native and interpreted TCG in the same code some years ago when I tried to > implement this, but that might have changed as TCG has evolved a lot. It is still not possible. However. There are a number of things that I would like to change about tcg that would make that more practical. In particular, and most importantly, tcg should be sufficiently partitioned from any target that it should be built once. * There are a number of constants that tcg gets from target/foo/cpu.h as defines. We could just as easily receive those constants via some structure. The code that is generated in the end should be the same. This is a prerequisite to having multiple guest cpus at the same time. The canonical example of course are the Xilinx boards that have both Arm and MicroBlaze cpus. * There are a number of constants that target/ gets from tcg, but shouldn't. These are mistakenly used by arm and tricore (probably by my hand), and represent premature optimization in the front end in both cases, IMO. (Even though I'm probably guilty of all instances.) We'll know we've got all of these fixed when touch tcg/i386/tcg-target.h rebuilds no files outside of tcg/. Having these constants in some structure is a prerequisite to having a native tcg backend live along side any kind of tci, including the pure interpreter form. > I disagree on the #ifdefs which can help to understand TCG better in my > opinion, so for me they have a useful side effect of being also documentation. Surely not. Documentation is documentation; ifdefs are clutter, to be used only when there is no alternative. I think what you actually want is the structure described above, where all of the TCG_TARGET_HAS_* knobs are data, and you can change them ad hoc -- even from within gdb. r~
On 2/4/21 10:02 AM, Stefan Weil wrote: > It would also help if the patches which were already reviewed were already > merged in qemu master. I'll queue the ones that have been reviewed to tcg-next. That'll get this lot down into the 60's. :-) r~
Almost 7 years ago I detailed 5 major problems in tci[1], of which three still remain: * Unaligned accesses to the bytecode stream, which means that we immediately SIGBUS on any host requiring alignment. * Non-portable calls to helper functions. * Full of useless ifdefs and TODOs. To my mind, this means the code is unmaintained, despite what it says in MAINTAINERS. Thus tci *should* be simply removed. However, every time removal is suggested, someone comes out of the woodwork and says we should keep it, because it's useful for $FOO. Anyway, if we're not going to remove tci, then we have to fix it. Previously, I rewrote tci all in one lump. Which was obviously a mistake, since it meant that the patch was never going to get reviewed. This time I've done the rewrite in tiny pieces. Previously, I invented a moderately complex encoding scheme that allowed any operand to encode a register or an int32_t immediate. This time the encoding is quite simple: with only 4 exceptions all operands encoded into 4-bit slots (registers & conditions). I rely on the new TEMP_CONST optimization to do a decent job of loading an immediate value into a register once (via movi), and reusing that across the TB. There is a disassembler built into tcg/tci.c, replacing the stub in disas/tci.c, which reuses the same decoding helpers that are used by the interpreter. So finally -d out_asm is useful. This is good enough to pass make check check-tcg with all of the docker cross-compilers enabled. I can boot linux with aarch64, alpha, and s390x guests. r~ Based-on: 20210203021550.375058-1-richard.henderson@linaro.org ("[PULL 00/24] tcg patch queue") [1] https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg02594.html Richard Henderson (91): gdbstub: Fix handle_query_xfer_auxv tcg: Split out tcg_raise_tb_overflow configure: Fix --enable-tcg-interpreter tcg: Manage splitwx in tc_ptr_to_region_tree by hand tcg/tci: Make tci_tb_ptr thread-local tcg/tci: Inline tci_write_reg32s into the only caller tcg/tci: Inline tci_write_reg8 into its callers tcg/tci: Inline tci_write_reg16 into the only caller tcg/tci: Inline tci_write_reg32 into all callers tcg/tci: Inline tci_write_reg64 into 64-bit callers tcg/tci: Merge INDEX_op_ld8u_{i32,i64} tcg/tci: Merge INDEX_op_ld8s_{i32,i64} tcg/tci: Merge INDEX_op_ld16u_{i32,i64} tcg/tci: Merge INDEX_op_ld16s_{i32,i64} tcg/tci: Merge INDEX_op_{ld_i32,ld32u_i64} tcg/tci: Merge INDEX_op_st8_{i32,i64} tcg/tci: Merge INDEX_op_st16_{i32,i64} tcg/tci: Move stack bounds check to compile-time tcg/tci: Merge INDEX_op_{st_i32,st32_i64} tcg/tci: Use g_assert_not_reached tcg/tci: Remove dead code for TCG_TARGET_HAS_div2_* tcg/tci: Implement 64-bit division tcg/tci: Remove TODO as unused tcg/tci: Restrict TCG_TARGET_NB_REGS to 16 tcg/tci: Fix TCG_REG_R4 misusage tcg/tci: Use bool in tcg_out_ri* tcg/tci: Remove TCG_CONST tcg/tci: Merge identical cases in generation tcg/tci: Remove tci_read_r8 tcg/tci: Remove tci_read_r8s tcg/tci: Remove tci_read_r16 tcg/tci: Remove tci_read_r16s tcg/tci: Remove tci_read_r32s tcg/tci: Reduce use of tci_read_r64 tcg/tci: Merge basic arithmetic operations tcg/tci: Merge extension operations tcg/tci: Remove ifdefs for TCG_TARGET_HAS_ext32[us]_i64 tcg/tci: Merge bswap operations tcg/tci: Merge mov, not and neg operations tcg/tci: Rename tci_read_r to tci_read_rval tcg/tci: Split out tci_args_rrs tcg/tci: Split out tci_args_rr tcg/tci: Split out tci_args_rrr tcg/tci: Split out tci_args_rrrc tcg/tci: Split out tci_args_l tcg/tci: Split out tci_args_rrrrrc tcg/tci: Split out tci_args_rrcl and tci_args_rrrrcl tcg/tci: Split out tci_args_ri and tci_args_rI tcg/tci: Reuse tci_args_l for calls. tcg/tci: Reuse tci_args_l for exit_tb tcg/tci: Reuse tci_args_l for goto_tb tcg/tci: Split out tci_args_rrrrrr tcg/tci: Split out tci_args_rrrr tcg/tci: Clean up deposit operations tcg/tci: Reduce qemu_ld/st TCGMemOpIdx operand to 32-bits tcg/tci: Split out tci_args_{rrm,rrrm,rrrrm} tcg/tci: Hoist op_size checking into tci_args_* tcg/tci: Remove tci_disas tcg/tci: Implement the disassembler properly tcg: Build ffi data structures for helpers tcg/tci: Use ffi for calls tcg/tci: Improve tcg_target_call_clobber_regs tcg/tci: Move call-return regs to end of tcg_target_reg_alloc_order tcg/tci: Push opcode emit into each case tcg/tci: Split out tcg_out_op_rrs tcg/tci: Split out tcg_out_op_l tcg/tci: Split out tcg_out_op_p tcg/tci: Split out tcg_out_op_rr tcg/tci: Split out tcg_out_op_rrr tcg/tci: Split out tcg_out_op_rrrc tcg/tci: Split out tcg_out_op_rrrrrc tcg/tci: Split out tcg_out_op_rrrbb tcg/tci: Split out tcg_out_op_rrcl tcg/tci: Split out tcg_out_op_rrrrrr tcg/tci: Split out tcg_out_op_rrrr tcg/tci: Split out tcg_out_op_rrrrcl tcg/tci: Split out tcg_out_op_{rrm,rrrm,rrrrm} tcg/tci: Split out tcg_out_op_v tcg/tci: Split out tcg_out_op_np tcg/tci: Split out tcg_out_op_r[iI] tcg/tci: Reserve r13 for a temporary tcg/tci: Emit setcond before brcond tcg/tci: Remove tci_write_reg tcg/tci: Change encoding to uint32_t units tcg/tci: Implement goto_ptr tcg/tci: Implement movcond tcg/tci: Implement andc, orc, eqv, nand, nor tcg/tci: Implement extract, sextract tcg/tci: Implement clz, ctz, ctpop tcg/tci: Implement mulu2, muls2 tcg/tci: Implement add2, sub2 Stefan Weil (2): tcg/tci: Implement INDEX_op_ld16s_i32 tcg/tci: Implement INDEX_op_ld8s_i64 configure | 5 +- meson.build | 9 +- include/exec/exec-all.h | 2 +- include/exec/helper-ffi.h | 115 ++ include/exec/helper-tcg.h | 24 +- include/tcg/tcg-opc.h | 6 +- include/tcg/tcg.h | 1 + target/hppa/helper.h | 2 + target/i386/ops_sse_header.h | 6 + target/m68k/helper.h | 1 + target/ppc/helper.h | 3 + tcg/tci/tcg-target-con-set.h | 8 +- tcg/tci/tcg-target.h | 118 +- disas/tci.c | 61 - gdbstub.c | 17 +- tcg/tcg-common.c | 4 - tcg/tcg.c | 117 +- tcg/tci.c | 1695 +++++++++++++----------- tcg/tci/tcg-target.c.inc | 989 +++++++------- tcg/tci/README | 20 +- tests/docker/dockerfiles/fedora.docker | 1 + 21 files changed, 1734 insertions(+), 1470 deletions(-) create mode 100644 include/exec/helper-ffi.h delete mode 100644 disas/tci.c -- 2.25.1