On Mon, 14 Jun 2021 at 23:22, Richard Henderson <richard.hender...@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