On Thu, Sep 26, 2019 at 12:07:54PM -0700, Richard Henderson wrote:
> On 9/24/19 4:31 AM, Andrew Jones wrote:
> > +static uint32_t sve_zcr_get_valid_len(ARMCPU *cpu, uint32_t start_len)
> > +{
> > + uint32_t start_vq = (start_len & 0xf) + 1;
> > +
> > + return arm_cpu_vq_map_next_smaller(cpu, start_vq + 1) - 1;
> > +}
> > +
> > /*
> > * Given that SVE is enabled, return the vector length for EL.
> > */
> > @@ -5360,13 +5367,13 @@ uint32_t sve_zcr_len_for_el(CPUARMState *env, int
> > el)
> > uint32_t zcr_len = cpu->sve_max_vq - 1;
> >
> > if (el <= 1) {
> > - zcr_len = MIN(zcr_len, 0xf & (uint32_t)env->vfp.zcr_el[1]);
> > + zcr_len = sve_zcr_get_valid_len(cpu, env->vfp.zcr_el[1]);
> > }
> > if (el <= 2 && arm_feature(env, ARM_FEATURE_EL2)) {
> > - zcr_len = MIN(zcr_len, 0xf & (uint32_t)env->vfp.zcr_el[2]);
> > + zcr_len = sve_zcr_get_valid_len(cpu, env->vfp.zcr_el[2]);
> > }
> > if (arm_feature(env, ARM_FEATURE_EL3)) {
> > - zcr_len = MIN(zcr_len, 0xf & (uint32_t)env->vfp.zcr_el[3]);
> > + zcr_len = sve_zcr_get_valid_len(cpu, env->vfp.zcr_el[3]);
>
> This has lost the MIN relation between these 3 registers.
> One possible solution is to keep these 3 if statements as-is,
> but make one call
>
> zcr_len = arm_cpu_vq_map_next_smaller(cpu, zcr_len);
>
> at the end.
Argh. I obviously didn't look closely enough at this function when I was
modifying it. I must have interpreted it as an if-else-if sequence...
I'll fix this as you suggest.
>
> > +#if __SIZEOF_LONG__ == 8
> > +#define BIT(n) (1UL << (n))
> > +#else
> > +#define BIT(n) (1ULL << (n))
> > +#endif
>
> There's no reason not to always use 1ULL is there?
Indeed ULL == UL for AArch64. I'll drop the #if... stuff.
Thanks,
drew
