Message ID | 20210614151007.4545-1-peter.maydell@linaro.org (mailing list archive) |
---|---|
Headers | show |
Series | target/arm: First slice of MVE implementation | expand |
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~