qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 00/57] target/arm: First slice of MVE implementation


From: Richard Henderson
Subject: Re: [PATCH v2 00/57] target/arm: First slice of MVE implementation
Date: Mon, 14 Jun 2021 15:22:45 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1

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~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]