On 2/12/20 12:09 AM, LIU Zhiwei wrote: > > > On 2020/2/12 0:56, Richard Henderson wrote: >> On 2/10/20 8:12 AM, LIU Zhiwei wrote: >>> static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong >>> *pc, >>> - target_ulong *cs_base, uint32_t >>> *flags) >>> + target_ulong *cs_base, uint32_t >>> *pflags) >>> { >>> + uint32_t flags = 0; >>> + uint32_t vlmax; >>> + uint8_t vl_eq_vlmax; >> bool. > OK. > > Is it clearer to use "bool" here? Or it's wrong to use "uint8_t "?
It is clearer. Using uint8_t makes me wonder what else you were going to put in that variable, but the answer from the code below is nothing. >>> + if (sew > cpu->cfg.elen) { /* only set vill bit. */ >>> + env->vext.vtype = FIELD_DP64(0, VTYPE, VILL, 1); >>> + env->vext.vl = 0; >>> + env->vext.vstart = 0; >>> + return 0; >>> + } >> You're missing checks against EDIV, VILL and the RESERVED field == 0. > This implementation does not support "Zvediv" . So I did not check it. I'm not > sure if I should check(ediv==0). > > I missed check "VILL" filed. Fix up it next patch. > > I'm not quite sure if I should set VILL if the RESERVED field != 0. The manual says # If the vtype setting is not supported by the implementation, # then the vill bit is set in vtype, the remaining bits in # vtype are set to zero, and the vl register is also set # to zero. So yes, you most certainly have to check ediv == 0. By extension, I believe the entire RESERVED field should be checked. Otherwise, we don't get the same forward compatible behaviour for the next vector extension beyond Zvediv. r~