Re: [PATCH v4 4/9] target/arm/cpu64: max cpu: Introduce sve<N> properties

[Richard Henderson]

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.

> +#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?


r~