On 02/23/2018 07:44 AM, Peter Maydell wrote: >> +/* Similar to the ARM LastActiveElement pseudocode function, except the >> + result is multiplied by the element size. This includes the not found >> + indication; e.g. not found for esz=3 is -8. */ >> +int32_t HELPER(sve_last_active_element)(void *vg, uint32_t pred_desc) >> +{ >> + intptr_t oprsz = extract32(pred_desc, 0, SIMD_OPRSZ_BITS) + 2; >> + intptr_t esz = extract32(pred_desc, SIMD_DATA_SHIFT, 2); > > pred_desc is obviously an encoding of some stuff, so the comment would > be a good place to mention what it is.
Yeah, and I've also just noticed I'm not totally consistent about it. I probably want to re-think how some of this is done. >> +/* Compute CLAST for a scalar. */ >> +static void do_clast_scalar(DisasContext *s, int esz, int pg, int rm, >> + bool before, TCGv_i64 reg_val) >> +{ >> + TCGv_i32 last = tcg_temp_new_i32(); >> + TCGv_i64 ele, cmp, zero; >> + >> + find_last_active(s, last, esz, pg); >> + >> + /* Extend the original value of last prior to incrementing. */ >> + cmp = tcg_temp_new_i64(); >> + tcg_gen_ext_i32_i64(cmp, last); >> + >> + if (!before) { >> + incr_last_active(s, last, esz); >> + } >> + >> + /* The conceit here is that while last < 0 indicates not found, after >> + adjusting for cpu_env->vfp.zregs[rm], it is still a valid address >> + from which we can load garbage. We then discard the garbage with >> + a conditional move. */ > > That's a bit ugly. Can we at least do a compile time assert that the > worst case (which I guess is offset of zregs[0] minus largest-element-size) > is still positive ? That way if for some reason we reshuffle fields > in CPUARMState we'll notice if it's going to fall off the beginning > of the struct. I suppose so. Though as commented above find_last_active, the minimal value is -8. I feel fairly confident that zregs[0] will never be shuffled to the absolute start of the structure. r~