Re: [PATCH 15/23] i386: Destructive vector helpers for AVX

[Richard Henderson]

On 8/26/22 16:11, Paolo Bonzini wrote:
> From: Paul Brook <paul@nowt.org>
>
> These helpers need to take special care to avoid overwriting source values
> before the wole result has been calculated.  Currently they use a dummy
> Reg typed variable to store the result then assign the whole register.
> This will cause 128 bit operations to corrupt the upper half of the register,
> so replace it with explicit temporaries and element assignments.
>
> Signed-off-by: Paul Brook <paul@nowt.org>
> Message-Id: <20220424220204.2493824-14-paul@nowt.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/ops_sse.h | 582 +++++++++++++++++++++---------------------
>   1 file changed, 289 insertions(+), 293 deletions(-)
>
> diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
> index 9ea763cad2..09dabfcbd5 100644
> --- a/target/i386/ops_sse.h
> +++ b/target/i386/ops_sse.h
> @@ -40,6 +40,8 @@
>   #define SUFFIX _xmm
>   #endif
>   
> +#define PACK_WIDTH (4 << SHIFT)

Incorrect for AVX, it was correct with the ifdef in v1.
Perhaps just (SHIFT ? 8 : 4)?

I think this should be parameterized on the larger of
the two types in the insn.  Because right now you get
some weird arithmetic in e.g. punpck*dq.


r~