On 3 October 2013 23:31, Will Newton <will.new...@linaro.org> wrote: > On 3 October 2013 13:59, Peter Maydell <peter.mayd...@linaro.org> wrote: >> This isn't what I meant. If our decoding matches up with the ARM ARM >> then this instruction pattern should already fall into disas_vfp_insn(), >> and we shouldn't need an extra check and call. (If it's not correct then >> we should adjust our decode so it does.) > > I'll respin the patch pulling the calls to disas_vfp_insn up a level > which I think you alluded to in the original review. It still needs an > additional call to disas_vfp_insn in the ARM case as condition code == > 0xf is dealt with separately from the others. Let me know if this is > not what you were looking for.
Ah, that means the ARM ARM table is incorrect, because it implies that VSEL is conditional (which it definitely isn't). I need to look at where the new insns are in the T32/A32 encodings in more detail, then, which I don't have time for just at the moment. Pulling the disas_vfp_insn calls out of disas_coproc is a good idea anyway, though (it should be a separate patch to the one which adds VSEL). -- PMM