Re: [Qemu-devel] [PATCH v2 00/10] tcg vector improvements

[Mark Cave-Ayland]

On 06/02/2019 03:37, Richard Henderson wrote:

> On 2/5/19 9:29 PM, Mark Cave-Ayland wrote:
>> The only minor question I had with the patchset in its current form is 
>> whether to use
>> the new VsrD() macro for vscr_sat, or whether we don't really care enough?
> 
> Given the comment
> 
>   /* Which bit we set is completely arbitrary, but clear the rest.  */
> 
> I don't think VsrD is helpful.

Okay, I can leave that for now.

> In "target/ppc: Split out VSCR_SAT to a vector field":
> 
>       ppc_vsr_t vsr[64] QEMU_ALIGNED(16);
> +     /* Non-zero if and only if VSCR_SAT should be set.  */
> +     ppc_vsr_t vscr_sat;
> 
> Better to add the QEMU_ALIGNED(16) to vscr_sat as well.  Yes, it will already
> happen to be aligned by placement, but this is also a bit documentation.

I've added this to my latest branch.

> In "target/ppc: convert vadd*s and vsub*s to vector operations":
> 
>           if (sat) {                                                      \
> -             set_vscr_sat(env);                                          \
> +             vscr_sat->u32[0] = 1;                                       \
>           }                                                               \
> 
> Changed in error?

It looks like this was in the original patch, presumably because GEN_VXFORM_SAT
doesn't include the env parameter which was present in GEN_VXFORM_ENV. Should 
the env
parameter be added to GEN_VXFORM_SAT?

Howard has also pointed out that he's still spotted some corruption in his 
tests, so
I will do a bit more investigation and report back.


ATB,

Mark.