RE: [PATCH v4 10/30] Hexagon HVX (target/hexagon) instruction utility functions

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Friday, October 29, 2021 1:53 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: f4bug@amsat.org; ale@rev.ng; Brian Cain <bcain@quicinc.com>
> Subject: Re: [PATCH v4 10/30] Hexagon HVX (target/hexagon) instruction
> utility functions
> 
> On 10/12/21 3:10 AM, Taylor Simpson wrote:
> > +void mem_vector_scatter_init(CPUHexagonState *env, int slot,
> > +                             target_ulong base_vaddr,
> > +                             int length, int element_size) {
> > +    int i;
> > +
> > +    for (i = 0; i < sizeof(MMVector); i++) {
> > +        env->vtcm_log.data.ub[i] = 0;
> > +    }
> > +    bitmap_zero(env->vtcm_log.mask, MAX_VEC_SIZE_BYTES);
> > +
> > +    env->vtcm_pending = true;
> > +    env->vtcm_log.op = false;
> > +    env->vtcm_log.op_size = 0;
> > +    env->vtcm_log.size = sizeof(MMVector);
> 
> Init really wants size != 0 here?  Because it's not that way for gather...

The vtcm_log.size is only used during packet commit when there is a scatter.  
It's not used for gather.

Since it's always sizeof(MMVector), I will remove it and replace all the uses 
with the constant value.

Also, op and op_size are only used for scatter, so I'll remove the 
initialization from mem_vector_gather_init.

> 
> Otherwise it looks like you want
> 
>      memset(&env->vtcm_log, 0, sizeof(env->vtcm_log));

Actually, this initialization is not needed because the values will be 
overwritten by the instruction.  So, I'll remove it.

> 
> Likewise memset of vtcm_log, with a second memset for tmp_Vregs[0].

Ditto.


Thanks a ton for the reviews!  This is the last feedback to be addressed on 
this series.  I'll send v5 out tonight for your review.

Thanks,
Taylor