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~
