| Message ID | CAFEAcA_SLYcPAAoP-dTdMJ4r+LkOg4M3nJj_bjfG+emzAQivhg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Jan 5, 2014 at 5:58 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 2 January 2014 01:51, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: >> On Mon, Dec 23, 2013 at 8:49 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> +#define ARM_CP_STATE_AA32 0 >>> +#define ARM_CP_STATE_AA64 1 >>> +#define ARM_CP_STATE_BOTH 2 >> >> You iterator below depends on this specific encoding ordering, so >> maybe this should be enumified. > > Delta from this to my fixed version: > > ===begin=== > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index b082bca..9430464 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -672,10 +672,14 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) > * If the reginfo is declared to be visible in both states then a second > * reginfo is synthesised for the AArch32 view of the AArch64 register, > * such that the AArch32 view is the lower 32 bits of the AArch64 one. > + * Note that we rely on the values of these enums as we iterate through > + * the various states in some places. > */ > -#define ARM_CP_STATE_AA32 0 > -#define ARM_CP_STATE_AA64 1 > -#define ARM_CP_STATE_BOTH 2 > +enum { > + ARM_CP_STATE_AA32 = 0, > + ARM_CP_STATE_AA64 = 1, > + ARM_CP_STATE_BOTH = 2, > +}; > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > /* Return true if cptype is a valid type field. This is used to try to > * catch errors where the sentinel has been accidentally left off the end > ===endit=== > > Personally I think the change to enum is less useful than the > comment saying "the values matter", but I don't particularly > object to enums. > > thanks > -- PMM >
===begin=== diff --git a/target-arm/cpu.h b/target-arm/cpu.h index b082bca..9430464 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -672,10 +672,14 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) * If the reginfo is declared to be visible in both states then a second * reginfo is synthesised for the AArch32 view of the AArch64 register, * such that the AArch32 view is the lower 32 bits of the AArch64 one. + * Note that we rely on the values of these enums as we iterate through + * the various states in some places. */ -#define ARM_CP_STATE_AA32 0 -#define ARM_CP_STATE_AA64 1 -#define ARM_CP_STATE_BOTH 2 +enum { + ARM_CP_STATE_AA32 = 0, + ARM_CP_STATE_AA64 = 1, + ARM_CP_STATE_BOTH = 2, +}; /* Return true if cptype is a valid type field. This is used to try to * catch errors where the sentinel has been accidentally left off the end