On 10/13/2017 09:24 AM, Alex Bennée wrote: > +/* > + * do_reduction_op helper > + * > + * This mirrors the Reduce() pseudocode in the ARM ARM. It is > + * important for correct NaN propagation that we do these > + * operations in exactly the order specified by the pseudocode. > + * > + * This is a recursive function, TCG temps should be freed by the > + * calling function once it is done with the values. > + */ > +static TCGv_i32 do_reduction_op(DisasContext *s, int fpopcode, int rn, > + int esize, int size, int vmap, TCGv_ptr fpst) > +{ > + if (esize == size) { > + int element; > + TCGMemOp msize = esize == 16 ? MO_16 : MO_32; > + TCGv_i32 tcg_elem; > + > + /* We should have one register left here */ > + assert(ctpop8(vmap) == 1);
I think you should match the ctpop to the size of vmap. It's true you only need uint8_t at present, so maybe use that? At least it's self-consistent. > + tcg_hi = do_reduction_op(s, fpopcode, rn, esize, bits, vmap_hi, > fpst); > + tcg_lo = do_reduction_op(s, fpopcode, rn, esize, bits, vmap_lo, > fpst); > + tcg_res = tcg_temp_new_i32(); You can re-use one of the two inputs for the output, fwiw. > + /* Bit 1 of size field encodes min vs max and the actual size > + * depends on the encoding of the U bit. If not set (and FP16 > + * enabled) then we do half-precision float instead of single > + * precision. > */ > is_min = extract32(size, 1, 1); > is_fp = true; > - size = 2; > + if (!is_u && arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { > + size = 1; You do still need to check size[0] == 0. r~