On 3 October 2013 15:37, Peter Maydell <peter.mayd...@linaro.org> wrote:
> 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.
Yes, these are in what would be the CDP2 space in both T32
and A32. So, quick sketch of what I think we should do:
* move the disas_vfp_insn() calls outside disas_coproc_insn()
(and in the thumb decode case, to before the "if bit 28 set
then goto illegal_op" check)
(basically what you have in this patch is fine)
* add a call to disas_vfp_insn() in the unconditional code
(what you have there in this patch is fine, but remember that
QEMU coding style mandates braces; use scripts/checkpatch.pl.)
* in disas_vfp_insn(), just after the "is vfp disabled?" check, add:
if (extract32(insn, 28, 4) == 0xf) {
/* Encodings with T=1 (Thumb) or unconditional (ARM):
* only used in v8 and above
*/
return 1;
}
That all goes into patch 1 of 2, which is just doing refactoring
and makes no changes in behaviour.
* then in patch 2 of the series, actually add the VSEL
support, by replacing that 'return 1' with
'return disas_vfp_v8_insn(env, s, insn);'
and implementing that function with the VSEL support.
[It seems better to me to have this separate rather than
fully integrated into the existing logic of disas_vfp_insn
because we know that no new insn is ever going to use the
legacy/deprecated vfp vector support. And the function is
already 800 lines long...]
thanks
-- PMM
