On Fri, Jun 28, 2019 at 09:27:39AM +0200, Andrew Jones wrote: > On Thu, Jun 27, 2019 at 06:49:02PM +0200, Richard Henderson wrote: > > On 6/21/19 6:34 PM, Andrew Jones wrote: > > > + /* > > > + * In sve_vq_map each set bit is a supported vector length of > > > + * (bit-number + 1) * 16 bytes, i.e. each bit number + 1 is the > > > vector > > > + * length in quadwords. We need a map size twice the maximum > > > + * quadword length though because we use two bits for each vector > > > + * length in order to track three states: uninitialized, off, and on. > > > + */ > > > + DECLARE_BITMAP(sve_vq_map, ARM_MAX_VQ * 2); > > > > I don't see that having one continuous bitmap is more convenient than two. > > Indeed, there appear to be several places that would be clearer with two. > > > > > +static arm_vq_state arm_cpu_vq_map_get(ARMCPU *cpu, int vq) > > > +{ > > > + assert(vq <= ARM_MAX_VQ); > > > + > > > + return test_bit(vq - 1, cpu->sve_vq_map) | > > > + test_bit(vq - 1 + ARM_MAX_VQ, cpu->sve_vq_map) << 1; > > > +} > > > > Neither easier nor more complex w/ one or two bitmaps. > > > > > +static void arm_cpu_vq_map_init(ARMCPU *cpu) > > > +{ > > > + bitmap_zero(cpu->sve_vq_map, ARM_MAX_VQ * 2); > > > + bitmap_set(cpu->sve_vq_map, ARM_MAX_VQ, ARM_MAX_VQ); > > > +} > > > > Clearer with two bitmaps. > > > > bitmap_zero(cpu->sve_vq_map, ARM_MAX_VQ); > > bitmap_set(cpu->sve_vq_uninit_map, 0, ARM_MAX_VQ); > > > > > +static bool arm_cpu_vq_map_is_finalized(ARMCPU *cpu) > > > +{ > > > + DECLARE_BITMAP(map, ARM_MAX_VQ * 2); > > > + > > > + bitmap_zero(map, ARM_MAX_VQ * 2); > > > + bitmap_set(map, ARM_MAX_VQ, ARM_MAX_VQ); > > > + bitmap_and(map, map, cpu->sve_vq_map, ARM_MAX_VQ * 2); > > > + > > > + return bitmap_empty(map, ARM_MAX_VQ * 2); > > > +} > > > > Definitely clearer w/ 2 bitmaps, > > > > return bitmap_empty(cpu->sve_vq_uninit_map); > > I guess I don't have a strong opinion on one or two bitmaps. I'm not a big > fan of adding temporary variables to data structures (even if the same > amount of storage is being allocated a different way), but I can change > this for v3.
On second thought, since in this case there is storage savings (one less long), then I'd rather we keep it a single bitmap. Maybe I can just add some comments to these bitmap operations? Thanks, drew