[Top][All Lists]

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

Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 2/8] target/ppc: rework vmrg{l, h}

From: Mark Cave-Ayland
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 2/8] target/ppc: rework vmrg{l, h}{b, h, w} instructions to use Vsr* macros
Date: Wed, 30 Jan 2019 13:37:39 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0

On 30/01/2019 12:51, Richard Henderson wrote:

> Why are you over-complicating things?  I thought I'd explicitly said this
> twice, but perhaps not:
> Pass the symbol "half" directly to VMRG_DO:
> #define VMRG(suffix, element, access)          \
>     VMRG_DO(mrgl##suffix, element, access, 0)  \
>     VMRG_DO(mrgh##suffix, element, access, half)
> You do not need another conditional within VMRG_DO.

I'm sorry - I think I got confused by your original macro definitions which 
used ofs
as both the macro parameter and variable name, which is why I thought you 
wanted ofs
passed in by value. Based upon the above I now have this which appears to work:

#define VMRG_DO(name, element, access, sofs)                                 \
    void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)            \
    {                                                                        \
        ppc_avr_t result;                                                    \
        int ofs, half = ARRAY_SIZE(r->element) / 2;                          \
        int i;                                                               \
        ofs = sofs;                                                          \
        for (i = 0; i < half; i++) {                                         \
            result.access(i * 2 + 0) = a->access(i + ofs);                   \
            result.access(i * 2 + 1) = b->access(i + ofs);                   \
        }                                                                    \
        *r = result;                                                         \

#define VMRG(suffix, element, access)          \
    VMRG_DO(mrgl##suffix, element, access, half)   \
    VMRG_DO(mrgh##suffix, element, access, 0)
VMRG(b, u8, VsrB)
VMRG(h, u16, VsrH)
VMRG(w, u32, VsrW)
#undef VMRG_DO
#undef VMRG

Is this what you were intending? If this looks better then I'll resubmit a v5 
this evening.

> Also, I'm pretty sure it's the "low" merge that wants the "ofs" to be 
> non-zero,
> since all of the Vsr* symbols implement big-endian indexing.

I've just tried swapping them around as you suggested, but then all my OS X
background fills appear with corrupted colors. So to the best of my knowledge 
dumping the register contents directly, the above version with low = half and 
high =
0 still seems correct.



reply via email to

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