[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce s
From: |
Andrew Jones |
Subject: |
Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties |
Date: |
Fri, 28 Jun 2019 10:31:01 +0200 |
User-agent: |
NeoMutt/20180716 |
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
- Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties, (continued)
Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties, Auger Eric, 2019/06/26
- Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties, Andrew Jones, 2019/06/27
- Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties, Auger Eric, 2019/06/27
- Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties, Andrew Jones, 2019/06/27
- Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties, Dave Martin, 2019/06/27
- Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties, Richard Henderson, 2019/06/27
Re: [Qemu-devel] [PATCH v2 07/14] target/arm/cpu64: max cpu: Introduce sve<vl-bits> properties, Richard Henderson, 2019/06/27
[Qemu-devel] [PATCH v2 06/14] target/arm: Allow SVE to be disabled via a CPU property, Andrew Jones, 2019/06/21
[Qemu-devel] [PATCH v2 08/14] target/arm/kvm64: Fix error returns, Andrew Jones, 2019/06/21