On 3 October 2013 20:31, Alex Bennée <alex.ben...@linaro.org> wrote: > > will.new...@linaro.org writes: > > <snip> >> @@ -6756,6 +6864,13 @@ static void disas_arm_insn(CPUARMState * env, >> DisasContext *s) >> goto illegal_op; >> return; >> } >> + if ((insn & 0x0f800e50) == 0x0e000a00) { >> + /* ARMv8 VFP. */ >> + ARCH(8); >> + >> + if (disas_v8vfp_insn(env, s, insn)) >> + goto illegal_op; >> + } >> if (((insn & 0x0f30f000) == 0x0510f000) || >> ((insn & 0x0f30f010) == 0x0710f000)) { >> if ((insn & (1 << 22)) == 0) { >> @@ -8768,6 +8883,12 @@ static int disas_thumb2_insn(CPUARMState *env, >> DisasContext *s, uint16_t insn_hw >> insn = (insn & 0xe2ffffff) | ((insn & (1 << 28)) >> 4) | (1 << >> 28); >> if (disas_neon_data_insn(env, s, insn)) >> goto illegal_op; >> + } else if ((insn & 0x0f800e50) == 0x0e000a00) { >> + /* ARMv8 VFP. */ >> + ARCH(8); >> + >> + if (disas_v8vfp_insn(env, s, insn)) >> + goto illegal_op; >> } else { >> if (insn & (1 << 28)) >> goto illegal_op; > > I wonder is there a better way to deal with these masks for checking > instruction type? I only mention it because the same constant is used > twice and should it ever be changed there is a risk of one being missed > (although conceivably thumb encoding could be different?).
The v8 ARM ARM actually integrates this new instruction into the decode tables for the existing VFP insns (it's an entry in table F5-16 'three register data floating point instructions'), which makes me wonder if we should put the decode into the existing disas_vfp_insn()... -- PMM