> +void VECTOR_HELPER(vsetvl)(CPURISCVState *env, uint32_t rs1, uint32_t rs2, > + uint32_t rd) > +{ > + int sew, max_sew, vlmax, vl; > + > + if (rs2 == 0) { > + vector_vtype_set_ill(env); > + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC()); > + return; > + }
I don't see that vsetvl, rs2 == r0 should raise SIGILL. Is that requirement new, after the 0.7.1 specification? If so, this should happen in the translator and not here. You should *not* change cpu state (setting vill here) before raising SIGILL. As far as I can see "vsetvl rd, rs1, r0" == "vsetvli rd, rs1, e8". > + env->vfp.vtype = env->gpr[rs2]; You should pass the rs2 register by value, not by index. > + sew = 1 << vector_get_width(env) / 8; > + max_sew = sizeof(target_ulong); > + > + if (env->misa & RVD) { > + max_sew = max_sew > 8 ? max_sew : 8; > + } else if (env->misa & RVF) { > + max_sew = max_sew > 4 ? max_sew : 4; > + } > + if (sew > max_sew) { > + vector_vtype_set_ill(env); > + return; > + } > + > + vlmax = vector_get_vlmax(env); > + if (rs1 == 0) { > + vl = vlmax; > + } else if (env->gpr[rs1] <= vlmax) { > + vl = env->gpr[rs1]; > + } else if (env->gpr[rs1] < 2 * vlmax) { > + vl = ceil(env->gpr[rs1] / 2); > + } else { > + vl = vlmax; > + } You should pass rs1 register by value, not by index. The special case of rs1 == r0 can be handled by passing the value (target_ulong)-1, which will match the final case above. > + env->vfp.vl = vl; > + env->gpr[rd] = vl; > + env->vfp.vstart = 0; > + return; > +} You should return vl and have it assigned to rd by the translator code, and not assign it here. > +void VECTOR_HELPER(vsetvli)(CPURISCVState *env, uint32_t rs1, uint32_t zimm, > + uint32_t rd) You should not require a separate helper function for this. Passing the zimm constant as the value for rs2 above is the correct mapping between the two instructions. r~