On 19 December 2013 06:01, Peter Crosthwaite <peter.crosthwaite@xxxxxxxxxx> wrote: > On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote: >> + *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm, >> + r->opc0, opc1, opc2); > > You have mixed terminology here with "opc" and "op". Should they be > unionised in ARMCPRegInfo? > > union { > uint8_t op1; > uint8_t opc1; > }; That seems pretty ugly to me. The terminology mixing is kind of inevitable since AArch32 uses opc and AArch64 uses op. >> + } else { >> + *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2); >> + } > > Why the mutual exclusivity between 32 and 64 bit defs? Shouldn't it be > possible to define with one ARMCPRegInfo a register that exists in > both 32 and 64 and this code can just double add it to the hash table? > May need two flags to describe existence in either or both schemes > accordingly. Almost all the shared registers appear as 32 bit on the AArch32 side and 64 bits on the AArch64 side. This means the required fieldoffset value is different [or potentially so for bigendian hosts]. So you'd only be able to share registers which were genuinely 64 bit on both sides, which are very rare. So it didn't seem worth trying to accommodate it. -- PMM _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm